On Wed Aug 21 14:09:17 2024 +0000, Vibhav Pant wrote:
> I have renamed it to `device_notification_details_with_notify`, is that better?
Eh. I don't like "notify" as a noun, it's still kind of misleading. I guess "handle" is also ambiguous for that matter.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/6315#note_79507
On Wed Aug 21 14:12:47 2024 +0000, Vibhav Pant wrote:
> Yup, moved it. However, I haven't inlined it, as I wanted to also add a
> log entry whenever `device_notify_proc` matches an incoming event
> against a registered notification filter, just before the `struct
> device_broadcast` object is dispatched to the corresponding callback.
> Would that be too noisy? If yes, I'll go ahead and inline it.
I can only speak for myself, but I don't imagine I would find a TRACE like that too noisy.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/6315#note_79506
On Wed Aug 21 13:01:35 2024 +0000, Vibhav Pant wrote:
> > Note that if you're using DWORD (and you could just as easily use
> "unsigned int" instead) you probably want "%lu" rather than casting to int.
> Yeah, I was a little confused if there's a format specification
> convention used by Wine for unsigned type variables used for storing
> size. Except for `%#Ix` used to `SIZE_T`, I could only see `ULONG`s
> being cast to int and then being printed with `%d`, so that's what I
> went with to be safe. But yeah, `%lu` makes more sense.
I think %u/%d vs %#x is mostly a matter of "best judgement", but in general you should use the right signed variant, and in PE code you shouldn't need to cast to get the right length specifier (though it may require a bit of extra knowledge: %d for int, %ld for long [which is always 32-bit on Windows], %Id for anything pointer-size, %I64d for 64-bit types [note that 'long long' is always 64-bit, but can't be used.])
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/6315#note_79505
On Wed Aug 21 14:08:25 2024 +0000, Vibhav Pant wrote:
> changed this line in [version 15 of the diff](/wine/wine/-/merge_requests/6315/diffs?diff_id=127699&start_sha=200a214bfb66f40ec311b78086abe9c3b95ba36a#c24d5a128c8e2e7df00647f30fabed6de6e42a52_2_0)
Hmm indeed. I'm not sure there's any reason we shouldn't do something like the attached patch in general, but that's for another merge request.
[scratch.diff](/uploads/36b496a0f6da14780df7bf853560835a/scratch.diff)
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/6315#note_79504