- wine-gitlab - winehq.org

[PATCH v3 0/4] MR1153: winex11.drv: Correctly handle clipping for fullscreen windows.
by Gabriel Ivăncescu (＠insn) 16 Jun '23

16 Jun '23

When we are clipping (due to fullscreen), certain operations are bugged (such as creating another window on top, grabbing the window during a size-move operation, changing mouse capture, etc) so we must temporarily unclip first. Seems unclipping it temporarily during such operation is enough to fix this. This probably wasn't very obvious in practice because most times "fullscreen" means a single non-resizeable window with no menus and so on (e.g. fullscreen games), but that's not always the case. -- v3: win32u: Don't clip while capturing mouse pointer. winex11: Ungrab clipping window when capturing mouse pointer. winex11: Don't clip while doing move/resize dragging. winex11: Don't clip while creating X Window. https://gitlab.winehq.org/wine/wine/-/merge_requests/1153

3 13

Re: [PATCH v25 0/7] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) 16 Jun '23

16 Jun '23

Zebediah Figura (@zfigura) commented about dlls/ntdll/unix/socket.c: > } > #endif > > + case AF_UNIX: > + { > + struct WS_sockaddr_un win = {0}; > + > + if (wsaddrlen < sizeof(win)) return -1; > + win.sun_family = WS_AF_UNIX; > + memcpy( win.sun_path, uaddr->un.sun_path, sizeof(win.sun_path) ); > + memcpy( wsaddr, &win, sizeof(win) ); > + return sizeof(win); Similarly this isn't going to work without converting from Unix to win32 paths. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_35928

1 0

Re: [PATCH v25 0/7] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) 16 Jun '23

16 Jun '23

Zebediah Figura (@zfigura) commented about dlls/ws2_32/tests/sock.c: > + ok(!memcmp((char[108]){0}, outAddr.sun_path, sizeof(addr.sun_path)), > + "getsockname returned incorrect path '%s' for provided path '%s'\n", > + outAddr.sun_path, > + addr.sun_path); > + > + memset(&outAddr, 0, sizeof(outAddr)); > + ret = getsockname(server, (SOCKADDR *)&outAddr, &(int){sizeof(outAddr)}); > + ok(!ret, "Could not get info on Unix socket: %lu\n", GetLastError()); > + ok(!memcmp(addr.sun_path, outAddr.sun_path, sizeof(addr.sun_path)), > + "getsockname returned incorrect path '%s' for provided path '%s'\n", > + outAddr.sun_path, > + addr.sun_path); > + > + memset(&outAddr, 0, sizeof(outAddr)); > + ret = getpeername(listener, (SOCKADDR *)&outAddr, &(int){sizeof(outAddr)}); > + ok(ret == -1, "Got info on Unix socket: %lu\n", GetLastError()); If we're testing that something returns an error we should test the value of GetLastError() as well. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_35927

1 0

Re: [PATCH v25 0/7] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) 16 Jun '23

16 Jun '23

Zebediah Figura (@zfigura) commented about dlls/ws2_32/tests/sock.c: > + > + 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()); > + server = accept(listener, NULL, NULL); > + ok(server != INVALID_SOCKET, "Could not accept Unix socket connection: %lu\n", > + GetLastError()); > + > + WaitForSingleObject(clientThread, 1000); > + > + memset(&outAddr, 0, sizeof(outAddr)); > + ret = getsockname(listener, (SOCKADDR *)&outAddr, &(int){sizeof(outAddr)}); Here and elsewhere, we should actually test the returned length. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_35926

1 0

Re: [PATCH v25 0/7] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) 16 Jun '23

16 Jun '23

Zebediah Figura (@zfigura) commented about dlls/ws2_32/tests/sock.c: > + ok(listener != INVALID_SOCKET, "Could not create Unix socket: %lu\n", > + GetLastError()); > + > + ret = bind(listener, (SOCKADDR *)&addr, sizeof(SOCKADDR_UN)); > + ok(!ret, "Could not bind Unix socket: %lu\n", GetLastError()); > + > + ret = listen(listener, 1); > + ok(!ret, "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()); Do we need to bother with the client thread here? (Could we potentially make the client socket nonblocking instead?) -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_35924

1 0

Re: [PATCH v25 0/7] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) 16 Jun '23

16 Jun '23

Zebediah Figura (@zfigura) commented about dlls/ws2_32/tests/sock.c: > + > + ret = listen(listener, 1); > + ok(!ret, "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()); > + server = accept(listener, NULL, NULL); > + ok(server != INVALID_SOCKET, "Could not accept Unix socket connection: %lu\n", > + GetLastError()); > + > + WaitForSingleObject(clientThread, 1000); If you're going to use a thread you should check this return value; if it fails then that means we have a bug in the implementation. We're also missing a CloseHandle(). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_35925

1 0

Re: [PATCH v25 0/7] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) 16 Jun '23

16 Jun '23

Zebediah Figura (@zfigura) commented about server/sock.c: > { > const struct afd_connect_params *params = get_req_data(); > const struct WS_sockaddr *addr; > - union unix_sockaddr unix_addr; > + union unix_sockaddr unix_addr, win_addr; This win_addr variable looks unused? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_35922

1 0

Re: [PATCH v25 0/7] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) 16 Jun '23

16 Jun '23

Zebediah Figura (@zfigura) commented about server/sock.c: > } > unix_len = sizeof(unix_addr); > if (!getsockname( acceptfd, &unix_addr.addr, &unix_len )) > - acceptsock->addr_len = sockaddr_from_unix( &unix_addr, &acceptsock->addr.addr, sizeof(acceptsock->addr) ); > + { > + unix_len = sizeof(unix_addr); This second assignment to "unix_len" looks unused? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_35923

1 0

Re: [PATCH v25 0/7] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) 16 Jun '23

16 Jun '23

Zebediah Figura (@zfigura) commented about server/sock.c: > if (unix_addr.addr.sa_family == AF_INET && !memcmp( &unix_addr.in.sin_addr, magic_loopback_addr, 4 )) > unix_addr.in.sin_addr.s_addr = htonl( INADDR_LOOPBACK ); > > + if (sock->family == WS_AF_UNIX && *((char *)(params + 1) + params->addr_len)) > + { > + char *unix_path = (char *)(params + 1) + params->addr_len; > + char unix_path_copy[PATH_MAX]; > + strcpy(unix_path_copy, unix_path); > + if (chdir( dirname( unix_path_copy ) ) == -1) > + { > + set_error( sock_get_ntstatus( errno ) ); > + return; > + } > + memcpy( unix_addr.un.sun_path, basename( unix_path ), sizeof(unix_addr.un.sun_path) ); We shouldn't need this anymore now that we're pssing the real unix path, right? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_35921

1 0

Re: [PATCH v25 0/7] MR2786: Add support for AF_UNIX sockets
by Zebediah Figura (＠zfigura) 16 Jun '23

16 Jun '23

Zebediah Figura (@zfigura) commented about dlls/ws2_32/socket.c: > > if (!(sync_event = get_sync_event())) return -1; > > - params = malloc( sizeof(int) + len ); > - ret_addr = malloc( len ); > + if (addr->sa_family == AF_UNIX && *addr->sa_data) Accessing sa_data in this way is convenient, but looks wrong. I'd rather just see the cast to sockaddr_un written out anyway. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2786#note_35919

1 0

2025

2024

2023

2022

wine-gitlab