Today, Wine fails to build with musl libc and Linux kernel 5.14 uAPI
header files.
musl libc doesn't supply any definitions for IPX, such as the SOL_IPX
macro. However, it still provides linux/ipx.h from Linux uAPI header
files if it exists.
Linux kernel wouldn't drop linux/ipx.h from uAPI headers until 5.15,
although IPX support has already been marked obsolete since 2018.
Fix this by not defining HAS_IPX if linux/ipx.h has been included but
nothing defines the SOL_IPX macro.
Status of IPX support from other libcs are noted below:
- bionic: netipx/ipx.h does not exist. linux/ipx.h may or may not
exist. Note that sys/socket.h defines SOL_IPX even if linux/ipx.h is
missing.
- glibc: netipx/ipx.h exists. In this case, Wine assumes IPX support
even if the operating system does not support it in runtime.
- BSD variants: netipx/ipx.h may or may not exist. linux/ipx.h does not
exist. Some BSDs supply SO_DEFAULT_HEADERS instead of SOL_IPX.
--
v2: server: Avoid relying on linux/ipx.h to define SOL_IPX.
ws2_32: Avoid relying on linux/ipx.h to define SOL_IPX.
ntdll: Avoid relying on linux/ipx.h to define SOL_IPX.
https://gitlab.winehq.org/wine/wine/-/merge_requests/3428
> We could maybe grab the Queue id in the cs, 0 it, and use the local copy for unlocking. Does that sound better?
That does seem like an improvement; I can't say I see a better way to solve the problem after looking, at least. We should probably mention in a comment why we're releasing outside of the lock; it is not obvious at all from the documentation that MFUnlockWorkQueue() can block.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3491#note_41430
On Fri Aug 4 20:16:23 2023 +0000, Zebediah Figura wrote:
> > It doesn't matter much if the handle is invalid, as the PutWorkItem
> call will just fail.
> But the handle could in theory be reallocated for a different queue.
> Also, if queues are supposed to be refcounted, it looks quite wrong that
> a thread which doesn't have a reference to a handle is accessing it;
> that's the sort of thing that generally causes memory errors.
> It might be the case that our current implementation is preventing any
> of this from being a problem in practice, but it's fragile and
> unidiomatic, and it is rather confusing to anyone reading the code; it
> looks like an error.
We could maybe grab the Queue id in the cs, 0 it, and use the local copy for unlocking. Does that sound better?
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3491#note_41426
> It doesn't matter much if the handle is invalid, as the PutWorkItem call will just fail.
But the handle could in theory be reallocated for a different queue. Also, if queues are supposed to be refcounted, it looks quite wrong that a thread which doesn't have a reference to a handle is accessing it; that's the sort of thing that generally causes memory errors.
It might be the case that our current implementation is preventing any of this from being a problem in practice, but it's fragile and unidiomatic, and it is rather confusing to anyone reading the code; it looks like an error.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3491#note_41425
On Fri Aug 4 19:58:51 2023 +0000, Zebediah Figura wrote:
> Not sure why I wasn't added as reviewer, but this looks wrong, and I
> think this doesn't fully solve the problem, either.
> As far as I can tell, if there's something else [what?] that pushes to
> the queue executing concurrently with Shutdown(), it'll end up pushing
> to an invalid queue handle after Shutdown() returns. This was already
> the case, but now it's worse, because you can't just fix it by setting
> source->async_commands_queue to NULL.
1. You were added as reviewer.
2. It doesn't matter much if the handle is invalid, as the PutWorkItem call will just fail.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3491#note_41424
The only important field at this point is cbSize, but if it's zero, the
message will not be processed.
---
The other option would be to zero the buffer and then do the equivalent of `((COMBOBOXINFO *)buffer)->cbSize = sizeof(COMBOBOXINFO)`, rather than copying the incoming lparam. That doesn't seem better to me, but I'm no expert.
--
v2: win32u: Copy an incoming COMBOBOXINFO when packing CB_GETCOMBOBOXINFO for clients.
https://gitlab.winehq.org/wine/wine/-/merge_requests/3507
Not sure why I wasn't added as reviewer, but this looks wrong, and I think this doesn't fully solve the problem, either.
As far as I can tell, if there's something else [what?] that pushes to the queue executing concurrently with Shutdown(), it'll end up pushing to an invalid queue handle after Shutdown() returns. This was already the case, but now it's worse, because you can't just fix it by setting source->async_commands_queue to NULL.
--
https://gitlab.winehq.org/wine/wine/-/merge_requests/3491#note_41413