-
Notifications
You must be signed in to change notification settings - Fork 17.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
x/sys/windows: add support for SOCKADDR_BTH structure #53929
Comments
Can you provide more details about why you want to define your own |
I tried to implement SOCKADDR_BTH for Windows. There is an example implementation (I'm not a big expert in Windows programming at all, and I only know about the existence of these 1 week ago). But I think this is something which is missing from the x/sys. Also the Bluetooth handling is pretty weak as well. It works with Linux perfectly, it has examples as well. But Windows is tricky and MacOS implementation is not fitting into the x/sys I think. On the otherhand I think it make sense to open this interface, even if the SOCKADDR_BTH will be added. Because anytime a new protocol/anything comes in, the developers needs to open a similar issue and waiting until it will be released. Is there anything why it shouldn't be more open what I don't see right now? |
If we export the method, then we have to have documentation and tests and ongoing maintenance for user defined socket types. Since new socket types are rare, that doesn't seem like a good use of maintainer time. We should just add support for |
Well, that's a solution as well. How it would look like? Because I'm open for contribution, but I didn't have a working example yet. The repository what I linked has issues what I didn't fix yet. So I'm heavily working on it. |
Update on that field, I have a Proof of Concept implementation. The good news that the only thing which missing is the SOCKADDR_BTH struct and it's method. So I could use the built-in functions from the x/sys/windows package. (Side-note I copy pasted the Connect function for investigation purposes.) So what I did:
This is working, so it proves, that the current functions can handle this feature. The only leftover is to figure out a struct and it's method which can create a similar byte array after the connect calls the |
After a massive investigation I printed out the raw memory what the C implementation sends. I got invalid parameters error so I checked the differences between the same struct what I had in Go against C. I found out that the Go implementation doesn't respect the data alignment. I guess it's because missing the attribute's packed. Otherwise I compared how the data I send should look like.
So I created a raw struct based on that and implemented the type SockaddrBth struct {
BtAddr [6]byte
ServiceClassId windows.GUID
Port uint32
raw RawSockaddrBth
}
type RawSockaddrBth struct {
Family [2]byte
Addr [8]byte
ServiceClassId [16]byte
Port [4]byte
}
func (sa *SockaddrBth) sockaddr() (unsafe.Pointer, int32, error) {
family := windows.AF_BTH
sa.raw = RawSockaddrBth{
Family: *(*[2]byte)(unsafe.Pointer(&family)),
Addr: *(*[8]byte)(unsafe.Pointer(&sa.BtAddr)),
Port: *(*[4]byte)(unsafe.Pointer(&sa.Port)),
ServiceClassId: *(*[16]byte)(unsafe.Pointer(&sa.ServiceClassId)),
}
return unsafe.Pointer(&sa.raw), int32(unsafe.Sizeof(sa.raw)), nil
} This is working for me properly. Let me know your thoughts. If you agree with it, I can send a PR or similar to Gerrit. |
I haven't looked at the details but that looks like the right general idea. Thanks. |
Cool, Then I will open a PR. I have seen that the RawSockaddrAny should support it, as well. Can it be a separate PR later? |
Change https://go.dev/cl/419394 mentions this issue: |
Problem
There are several interfaces in the x/sys like these:
These all contain unexported method descriptors, so there is no way to add own implementation. We run into the issue, that we needed a windows extension and even if we can implement that, we can't use it with this solution.
Solution
Make the method descriptors exported.
The text was updated successfully, but these errors were encountered: