On Sat Aug 31 16:30:43 2024 +0000, Elizabeth Figura wrote:
> Sorry it's taken me so long to get to this, but 3/4 seems suspicious.
> I'd think BroadcastDeviceNotification() [or SendMessageNotify() or
> whatever it calls internally] should be making a copy of the message
> data itself. Are we not doing that? Should we be doing that instead?
Yeah, I might be wrong here. A quick inspection of the code does suggest that we do make copies in `(un)pack_message`. MSDN's documentation for `SendNotifyMessageW`\[1\] warned that messages containing pointers could not be sent as they would result in access to a dangling reference, which is what prompted me to write 3/4. However, `DEV_BROADCAST_DEVICEINTERFACE` messages don't contain any pointers, so this doesn't apply here.
\[1\] https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-send…
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/6315#note_80621
> I don't think adding custom structs is justified at this point, especially as it requires rewriting large parts of the code in multiple modules at the same time.
`wine/dbt.h` doesn't really _add_ a new struct. It merely makes `struct device_notification_details` a common public declaration. The struct is currently [a private def in `dlls/sechost/service.c`](https://gitlab.winehq.org/wine/wine/-/blob/055bddab4f14a1f73e887b88b86408d654382c2b/dlls/sechost/service.c?page=2#L1976), and needs to be defined again for any component that utilizes `I_ScRegisterDeviceNotification`. [`dlls/user32/input.c` seems to be the only place where it gets used ATM](https://gitlab.winehq.org/wine/wine/-/blob/055bddab4f14a1f73e887b88b86…, and the motivation behind shifting the definition to a common header was to preempt the possibility of the struct's definition accidentally diverging in the future.
> However, passing process-private handles around ~~to other processes~~ and later getting them back looks wrong. What happens if you register for notification on a handle then close it right away? Does the notification keep firing with the old handle value? It seem surprising and it would be a nice thing to test for instance.
Yeah, this is something I've been curious about as well. I suppose one way to test this would be to also add an implementation for`IoReportTargetDeviceChange` (thanks @zfigura!) to this MR, and test `RegisterDeviceNotification`'s behavior in `dlls/ntoskrnl.exe/tests/driver_pnp.c`.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/6315#note_80620
The MR adds implementations (+ a few tests) for the following methods in `bluetoothapis.h`:
* `BluetoothSdpEnumAttributes`
* `BluetoothSdpGetContainerElementData`
* `BluetoothSdpGetElementData`
* `BluetoothSdpGetAttributeValue`
--
v12: bluetoothapis/tests: Add tests for BluetoothSdpEnumAttributes and BluetoothSdpGetAttributeValue.
bluetoothapis: Implement BluetoothSdpGetAttributeValue.
bluetoothapis/tests: Add unit tests for BluetoothSdpGetContainerElementData and BluetoothSdpGetElementData.
bluetoothapis: Implement BluetoothSdpEnumAttributes.
bluetoothapis: Implement BluetoothSdpGetContainerElementData.
bluetoothapis: Implement BluetoothSdpGetElementData.
https://gitlab.winehq.org/wine/wine/-/merge_requests/6402