May 2023 - Wine-GitLab - list.winehq.org

Re: [PATCH v8 0/6] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) May 18, 2023

May 18, 2023

Zebediah Figura (@zfigura) commented about server/fd.c: > } > > + /* open(3) throws ENXIO when `path` is one of: > + * 1. a FIFO, and open(3) is called with O_WRONLY | O_NONBLOCK, > + * and no other process is currently attempting to read from `path`. > + * 2. a special device file, and the device it corresponds to does not exist. > + * 3. a UNIX socket. > + * We can confirm the third case to allow for unlinking a socket. > + */ > + if (errno == ENXIO && !stat( name, &st ) && S_ISSOCK(st.st_mode)) > + { > + if ((access & DELETE)) > + unlink( name ); > + else > + { > + set_error( ENXIO ); That's... not really correct, ENXIO isn't an NTSTATUS. Better just to let this hit the same file_set_error() path. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_33116

1 0

Re: [PATCH v8 0/6] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) May 18, 2023

May 18, 2023

Zebediah Figura (@zfigura) commented about dlls/ws2_32/tests/sock.c: > +{ > + int sockfd; > + SOCKADDR_UN addr; > +}; > + > +static DWORD WINAPI test_afunix_client_connect_thread(void *param) > +{ > + struct afunix_thread_param *in = param; > + ok(!connect(in->sockfd, (SOCKADDR *)&in->addr, sizeof(in->addr)), "Could not connect to Unix socket: %lu\n", > + GetLastError()); > + return 0; > +} > + > +static void test_afunix(void) > +{ > + SOCKET server, client, conn; This naming is understandable, but a bit confusing, since much of the rest of the file uses the convention "listener, server, client", where "server" is returned from accept(). For the sake of clarity to someone reading the test I'd request that it be changed to match that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_33115

1 0

Re: [PATCH v8 0/6] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) May 18, 2023

May 18, 2023

Zebediah Figura (@zfigura) commented about dlls/ws2_32/tests/sock.c: > + ok(ret == sizeof(serverMsg), > + ret >= 0 > + ? "Incorrect amount of bytes written to Unix socket: %lu\n" > + : "Could not send data over Unix socket: %lu\n", > + ret >= 0 ? ret : GetLastError()); > + > + ret = recv(client, clientBuf, sizeof(serverMsg), 0); > + ok(ret == sizeof(serverMsg), > + ret >= 0 > + ? "Incorrect amount of bytes read from Unix socket: %lu\n" > + : "Could not receive data over Unix socket: %lu\n", > + ret >= 0 ? ret : GetLastError()); > + > + ok(!memcmp(serverMsg, clientBuf, sizeof(serverMsg)), "Data mismatch over Unix socket\n"); > + > + DeleteFileA("test_afunix.sock"); This isn't very interesting if you don't test the return value. Judging from patch 4/8, I'm guessing real software does this. "Analogous to unbinding" is an interesting statement, though. Do real world programs expect the socket to be able to be bound to a different address afterwards? We'll need tests for that, and I don't think it's currently possible in Wine. Note that this can't possibly be a substitute for actually closing the socket handle, which this test should do for all of its sockets regardless. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_33114

1 0

Re: [PATCH v8 0/6] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) May 18, 2023

May 18, 2023

Zebediah Figura (@zfigura) commented about dlls/ws2_32/tests/sock.c: > + ok(client != INVALID_SOCKET, "Failed to create second Unix socket: %lu\n", > + GetLastError()); > + > + clientThread = CreateThread(NULL, 0, test_afunix_client_connect_thread, > + &(struct afunix_thread_param){ client, addr }, 0, NULL); > + ok(clientThread != NULL, "CreateThread failed unexpectedly: %ld\n", GetLastError()); > + conn = accept(server, NULL, NULL); > + ok(conn != INVALID_SOCKET, "Could not accept Unix socket connection: %lu\n", > + GetLastError()); > + > + ret = send(conn, serverMsg, sizeof(serverMsg), 0); > + ok(ret == sizeof(serverMsg), > + ret >= 0 > + ? "Incorrect amount of bytes written to Unix socket: %lu\n" > + : "Could not send data over Unix socket: %lu\n", > + ret >= 0 ? ret : GetLastError()); This is more complex than it needs to be; just write "Got return value %d", and either leave out the GetLastError() or add a second unconditional ok() for it. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_33113

1 0

Re: [PATCH v8 0/6] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) May 18, 2023

May 18, 2023

Zebediah Figura (@zfigura) commented about dlls/ws2_32/tests/sock.c: > +{ > + SOCKET server, client, conn; > + const SOCKADDR_UN addr = { AF_UNIX, "test_afunix.sock" }; > + HANDLE clientThread; > + const char serverMsg[] = "ws2_32/AF_UNIX socket test"; > + char clientBuf[sizeof(serverMsg)] = { 0 }; > + int ret; > + > + server = socket(AF_UNIX, SOCK_STREAM, 0); > + ok(server != INVALID_SOCKET, "Could not create Unix socket: %lu\n", > + GetLastError()); > + > + ok(!bind(server, (SOCKADDR *)&addr, sizeof(SOCKADDR_UN)), "Could not bind Unix socket: %lu\n", > + GetLastError()); > + ok(!listen(server, 1), "Could not listen on Unix socket: %lu\n", > + GetLastError()); This kind of thing should generally be avoided, because GetLastError() won't necessarily run after the bind call. Just use two separate lines. (There may be precedent for the bad code earlier in the file, but this file is pretty much full of bad code.) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_33112

1 0

Re: [PATCH v8 0/6] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) May 18, 2023

May 18, 2023

Zebediah Figura (@zfigura) commented about dlls/ws2_32/tests/sock.c: > + server = socket(AF_UNIX, SOCK_STREAM, 0); > + ok(server != INVALID_SOCKET, "Could not create Unix socket: %lu\n", > + GetLastError()); > + > + ok(!bind(server, (SOCKADDR *)&addr, sizeof(SOCKADDR_UN)), "Could not bind Unix socket: %lu\n", > + GetLastError()); > + ok(!listen(server, 1), "Could not listen on Unix socket: %lu\n", > + GetLastError()); > + > + client = socket(AF_UNIX, SOCK_STREAM, 0); > + ok(client != INVALID_SOCKET, "Failed to create second Unix socket: %lu\n", > + GetLastError()); > + > + clientThread = CreateThread(NULL, 0, test_afunix_client_connect_thread, > + &(struct afunix_thread_param){ client, addr }, 0, NULL); > + ok(clientThread != NULL, "CreateThread failed unexpectedly: %ld\n", GetLastError()); The thread should be unnecessary, you can just use nonblocking I/O. It doesn't make a big difference, but it's probably easier to read without the thread. If you do use the thread, you need to clean it up. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_33111

1 0

[PATCH v8 0/3] MR2808: Draft: SNI tray implementation
by Cherser-s (＠Cherser-s) May 18, 2023

May 18, 2023

Adds the tray icons implementation based on org.kde.StatusNotifierItem interface usage. Does allow restarting StatusNotifierWatcher object, but will fallback to XEMBED or internal tray, if wine gets initialized when there is no StatusNotifierWatcher object registered. -- v8: configure: fix winesni.drv not getting disabled https://gitlab.winehq.org/wine/wine/-/merge_requests/2808

3 4

Re: [PATCH v8 0/6] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) May 18, 2023

May 18, 2023

> Yes, Windows 10 v1803 ("April 2018 Update") was the first stable release of Windows to include this feature. `w1064v1507` fails, while `w1064v1809` passes. We'll need to account for that in the tests, by failing with win_skip() when we can't create an AF_UNIX socket. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_33107

1 0

Re: [PATCH v8 0/6] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) May 18, 2023

May 18, 2023

> @zfigura Can you elaborate on this? I don't see a clean way of doing this in ntdll without modifying `struct afd_bind_params`, `struct afd_connect_params`, etc. to add a field for the Unix path, since none of the other `sock_ioctl` cases add on varargs. afd_bind_params et al. should already take a variable sized sockaddr, and the struct definitions in include/wine/afd.h mention that specifically. Am I missing something? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_33106

1 0

[PATCH v2 0/1] MR2847: gdiplus: Improve performance of gdip_transform_points.
by Bartosz Kosiorek (＠gang65) May 18, 2023

May 18, 2023

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=53947 -- v2: gdiplus: Improve performance of gdip_transform_points. https://gitlab.winehq.org/wine/wine/-/merge_requests/2847

4 6