On Wed, May 20, 2020 at 09:01:21AM -0500, Zebediah Figura wrote:
On 5/20/20 7:14 AM, Zhiyi Zhang wrote:
Hi Arkadiusz,
Checking NULL is usually fine. But I think you should add the tests to sechost. There are also https://github.com/ValveSoftware/wine/commit/bbcd2686d599adf6c5cc8e8466ade0b... https://github.com/ValveSoftware/wine/commit/2d5d0a50652e13254fef3d13df14d3b...
I haven't look at if I_ScUnregisterDeviceNotification() is already fully implemented. But if it is not, it would be more useful to get it upstream rather than simply checking NULL. If checking NULL is enough to fix a real world application and you think upstreamming the work is too much for you then a simple NULL fix is also fine.
Personally, I think the test makes sense where it is.
Hey,
Sorry for late reply, I subscribe to about a dozen of mailing lists and I am used that people in the discussion are explicitly CCed. I'll adjust my filtering for wine-devel.
I am still wrapping my head around some of the things here. As far as I undersand I_ScUnregisterDeviceNotification() is internal and is not exposed by any header. UnregisterDeviceNotification() it the call that a lot of programs are usining through the winuser.h, and therefore should have conformance tests on it's own.
I_ScUnregisterDeviceNotification() may be called by some .exes though anyway, so it should have it's own set of conformance tests too.
Is this correct?
On the other hand, it'd be nice to mention in the commit message what application is fixed by this.
It's the Glorious Model O control software. It won't work but at least this fixes the crash.
I'll resend my patch with that mentioned + Roman's bug linked and CC you. Thanks!