[PATCH v3 0/8] MR871: ntdll: Fill the IOSB only inside the if (alerted) block in multiple functions.
-- v3: ntdll: Combine the "if (alerted)" blocks in sock_transmit(). ntdll: Fill the IOSB in sock_transmit() only inside the "if (alerted)" block. ntdll: Combine the "if (alerted)" blocks in sock_send(). ntdll: Fill the IOSB in sock_send() only inside the "if (alerted)" block. ntdll: Combine the "if (alerted)" blocks in sock_recv(). ntdll: Fill the IOSB in sock_recv() only inside the "if (alerted)" block. ntdll: The async handle passed to set_async_direct_result() cannot be NULL. ws2_32/tests: Add more tests for iosb contents while a recv is pending. https://gitlab.winehq.org/wine/wine/-/merge_requests/871
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ws2_32/tests/afd.c | 7 ++++--- dlls/ws2_32/tests/sock.c | 4 ++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/dlls/ws2_32/tests/afd.c b/dlls/ws2_32/tests/afd.c index 21e7a50aef3..3215ddaef62 100644 --- a/dlls/ws2_32/tests/afd.c +++ b/dlls/ws2_32/tests/afd.c @@ -1466,13 +1466,14 @@ static void test_recv(void) IOCTL_AFD_RECV, ¶ms, sizeof(params) - 1, NULL, 0); ok(ret == STATUS_INVALID_PARAMETER, "got %#x\n", ret); - memset(&io, 0, sizeof(io)); + io.Status = 0xdeadbeef; + io.Information = 0xdeadbeef; memset(buffer, 0xcc, sizeof(buffer)); ret = NtDeviceIoControlFile((HANDLE)client, event, NULL, NULL, &io, IOCTL_AFD_RECV, ¶ms, sizeof(params), NULL, 0); ok(ret == STATUS_PENDING, "got %#x\n", ret); - ok(!io.Status, "got status %#lx\n", io.Status); - ok(!io.Information, "got information %#Ix\n", io.Information); + ok(io.Status == 0xdeadbeef, "got status %#lx\n", io.Status); + ok(io.Information == 0xdeadbeef, "got information %#Ix\n", io.Information); /* These structures need not remain valid. */ memset(¶ms, 0xcc, sizeof(params)); diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c index d4f430a29d2..adf16012951 100644 --- a/dlls/ws2_32/tests/sock.c +++ b/dlls/ws2_32/tests/sock.c @@ -12208,11 +12208,15 @@ static void test_nonblocking_async_recv(void) memset(buffer, 0, sizeof(buffer)); WSASetLastError(0xdeadbeef); + overlapped.Internal = 0xdeadbeef; + overlapped.InternalHigh = 0xdeadbeef; ret = WSARecv(client, &wsabuf, 1, NULL, &flags, &overlapped, NULL); ok(ret == -1, "got %d\n", ret); ok(WSAGetLastError() == ERROR_IO_PENDING, "got error %u\n", WSAGetLastError()); ret = WaitForSingleObject((HANDLE)client, 0); ok(ret == WAIT_TIMEOUT, "expected timeout\n"); + ok(overlapped.Internal == STATUS_PENDING, "got status %#lx\n", (NTSTATUS)overlapped.Internal); + ok(overlapped.InternalHigh == 0xdeadbeef, "got size %Iu\n", overlapped.InternalHigh); ret = send(server, "data", 4, 0); ok(ret == 4, "got %d\n", ret); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/871
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/ntdll/unix/sync.c | 9 +++++---- dlls/ntdll/unix/unix_private.h | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/dlls/ntdll/unix/sync.c b/dlls/ntdll/unix/sync.c index 1194ee514b5..821a1f08311 100644 --- a/dlls/ntdll/unix/sync.c +++ b/dlls/ntdll/unix/sync.c @@ -2533,21 +2533,22 @@ NTSTATUS WINAPI NtWaitForAlertByThreadId( const void *address, const LARGE_INTEG /* Notify direct completion of async and close the wait handle if it is no longer needed. * This function is a no-op (returns status as-is) if the supplied handle is NULL. */ -void set_async_direct_result( HANDLE *optional_handle, NTSTATUS status, ULONG_PTR information, BOOL mark_pending ) +void set_async_direct_result( HANDLE *async_handle, NTSTATUS status, ULONG_PTR information, BOOL mark_pending ) { NTSTATUS ret; - if (!*optional_handle) return; + /* if we got STATUS_ALERTED, we must have a valid async handle */ + assert( *async_handle ); SERVER_START_REQ( set_async_direct_result ) { - req->handle = wine_server_obj_handle( *optional_handle ); + req->handle = wine_server_obj_handle( *async_handle ); req->status = status; req->information = information; req->mark_pending = mark_pending; ret = wine_server_call( req ); if (ret == STATUS_SUCCESS) - *optional_handle = wine_server_ptr_handle( reply->handle ); + *async_handle = wine_server_ptr_handle( reply->handle ); } SERVER_END_REQ; diff --git a/dlls/ntdll/unix/unix_private.h b/dlls/ntdll/unix/unix_private.h index 47f0f9c56a9..73b9ed76de0 100644 --- a/dlls/ntdll/unix/unix_private.h +++ b/dlls/ntdll/unix/unix_private.h @@ -278,7 +278,7 @@ extern NTSTATUS get_device_info( int fd, struct _FILE_FS_DEVICE_INFORMATION *inf extern void init_files(void) DECLSPEC_HIDDEN; extern void init_cpu_info(void) DECLSPEC_HIDDEN; extern void add_completion( HANDLE handle, ULONG_PTR value, NTSTATUS status, ULONG info, BOOL async ) DECLSPEC_HIDDEN; -extern void set_async_direct_result( HANDLE *optional_handle, NTSTATUS status, ULONG_PTR information, BOOL mark_pending ); +extern void set_async_direct_result( HANDLE *async_handle, NTSTATUS status, ULONG_PTR information, BOOL mark_pending ) DECLSPEC_HIDDEN; extern void dbg_init(void) DECLSPEC_HIDDEN; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/871
From: Zebediah Figura <zfigura(a)codeweavers.com> We can only get a successful status that way. This avoids an uninitialized variable warning with gcc 12.2. --- dlls/ntdll/unix/socket.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index f8ed9f6f854..8ff58f7566e 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -23,6 +23,7 @@ #endif #include "config.h" +#include <assert.h> #include <errno.h> #include <sys/types.h> #include <sys/socket.h> @@ -879,24 +880,25 @@ static NTSTATUS sock_recv( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi } SERVER_END_REQ; + /* the server currently will never succeed immediately */ + assert(status == STATUS_ALERTED || status == STATUS_PENDING || NT_ERROR(status)); + alerted = status == STATUS_ALERTED; if (alerted) { status = try_recv( fd, async, &information ); if (status == STATUS_DEVICE_NOT_READY && (force_async || !nonblocking)) status = STATUS_PENDING; - } - - if (status != STATUS_PENDING) - { - if (!NT_ERROR(status) || (wait_handle && !alerted)) + if (!NT_ERROR(status) && status != STATUS_PENDING) { io->Status = status; io->Information = information; } - release_fileio( &async->io ); } + if (status != STATUS_PENDING) + release_fileio( &async->io ); + if (alerted) set_async_direct_result( &wait_handle, status, information, FALSE ); if (wait_handle) status = wait_async( wait_handle, options & FILE_SYNCHRONOUS_IO_ALERT ); return status; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/871
From: Zebediah Figura <zfigura(a)codeweavers.com> There is no need to release the async_fileio structure before calling set_async_direct_result(). --- dlls/ntdll/unix/socket.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index 8ff58f7566e..44694f8c404 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -852,9 +852,8 @@ static BOOL is_icmp_over_dgram( int fd ) static NTSTATUS sock_recv( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc_user, IO_STATUS_BLOCK *io, int fd, struct async_recv_ioctl *async, int force_async ) { - BOOL nonblocking, alerted; - ULONG_PTR information; HANDLE wait_handle; + BOOL nonblocking; NTSTATUS status; unsigned int i; ULONG options; @@ -883,9 +882,10 @@ static NTSTATUS sock_recv( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi /* the server currently will never succeed immediately */ assert(status == STATUS_ALERTED || status == STATUS_PENDING || NT_ERROR(status)); - alerted = status == STATUS_ALERTED; - if (alerted) + if (status == STATUS_ALERTED) { + ULONG_PTR information; + status = try_recv( fd, async, &information ); if (status == STATUS_DEVICE_NOT_READY && (force_async || !nonblocking)) status = STATUS_PENDING; @@ -894,12 +894,12 @@ static NTSTATUS sock_recv( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi io->Status = status; io->Information = information; } + set_async_direct_result( &wait_handle, status, information, FALSE ); } if (status != STATUS_PENDING) release_fileio( &async->io ); - if (alerted) set_async_direct_result( &wait_handle, status, information, FALSE ); if (wait_handle) status = wait_async( wait_handle, options & FILE_SYNCHRONOUS_IO_ALERT ); return status; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/871
From: Zebediah Figura <zfigura(a)codeweavers.com> We can only get a successful status that way. This matches sock_recv(). --- dlls/ntdll/unix/socket.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index 44694f8c404..cc3c898d464 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1116,6 +1116,9 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi } SERVER_END_REQ; + /* the server currently will never succeed immediately */ + assert(status == STATUS_ALERTED || status == STATUS_PENDING || NT_ERROR(status)); + if (!NT_ERROR(status) && is_icmp_over_dgram( fd )) sock_save_icmp_id( async ); @@ -1132,19 +1135,17 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi * and returns EWOULDBLOCK, but we have no way of doing that. */ if (status == STATUS_DEVICE_NOT_READY && async->sent_len) status = STATUS_SUCCESS; - } - if (status != STATUS_PENDING) - { information = async->sent_len; - if (!NT_ERROR(status) || (wait_handle && !alerted)) + if (!NT_ERROR(status) && status != STATUS_PENDING) { io->Status = status; io->Information = information; } - release_fileio( &async->io ); } - else information = 0; + + if (status != STATUS_PENDING) + release_fileio( &async->io ); if (alerted) set_async_direct_result( &wait_handle, status, information, FALSE ); if (wait_handle) status = wait_async( wait_handle, options & FILE_SYNCHRONOUS_IO_ALERT ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/871
From: Zebediah Figura <zfigura(a)codeweavers.com> There is no need to release the async_fileio structure before calling set_async_direct_result(). --- dlls/ntdll/unix/socket.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index cc3c898d464..e53ad4b76be 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1099,9 +1099,8 @@ static void sock_save_icmp_id( struct async_send_ioctl *async ) static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, void *apc_user, IO_STATUS_BLOCK *io, int fd, struct async_send_ioctl *async, int force_async ) { - BOOL nonblocking, alerted; - ULONG_PTR information; HANDLE wait_handle; + BOOL nonblocking; NTSTATUS status; ULONG options; @@ -1122,9 +1121,10 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi if (!NT_ERROR(status) && is_icmp_over_dgram( fd )) sock_save_icmp_id( async ); - alerted = status == STATUS_ALERTED; - if (alerted) + if (status == STATUS_ALERTED) { + ULONG_PTR information; + status = try_send( fd, async ); if (status == STATUS_DEVICE_NOT_READY && (force_async || !nonblocking)) status = STATUS_PENDING; @@ -1142,12 +1142,13 @@ static NTSTATUS sock_send( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, voi io->Status = status; io->Information = information; } + + set_async_direct_result( &wait_handle, status, information, FALSE ); } if (status != STATUS_PENDING) release_fileio( &async->io ); - if (alerted) set_async_direct_result( &wait_handle, status, information, FALSE ); if (wait_handle) status = wait_async( wait_handle, options & FILE_SYNCHRONOUS_IO_ALERT ); return status; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/871
From: Zebediah Figura <zfigura(a)codeweavers.com> We can only get a successful status that way. This matches sock_recv(). --- dlls/ntdll/unix/socket.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index e53ad4b76be..351b341f84f 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1389,25 +1389,26 @@ static NTSTATUS sock_transmit( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, } SERVER_END_REQ; + /* the server currently will never succeed immediately */ + assert(status == STATUS_ALERTED || status == STATUS_PENDING || NT_ERROR(status)); + alerted = status == STATUS_ALERTED; if (alerted) { status = try_transmit( fd, file_fd, async ); if (status == STATUS_DEVICE_NOT_READY) status = STATUS_PENDING; - } - if (status != STATUS_PENDING) - { information = async->head_cursor + async->file_cursor + async->tail_cursor; - if (!NT_ERROR(status) || wait_handle) + if (!NT_ERROR(status) && status != STATUS_PENDING) { io->Status = status; io->Information = information; } - release_fileio( &async->io ); } - else information = 0; + + if (status != STATUS_PENDING) + release_fileio( &async->io ); if (alerted) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/871
From: Zebediah Figura <zfigura(a)codeweavers.com> There is no need to release the async_fileio structure before calling set_async_direct_result(). --- dlls/ntdll/unix/socket.c | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c index 351b341f84f..b502d3a03cb 100644 --- a/dlls/ntdll/unix/socket.c +++ b/dlls/ntdll/unix/socket.c @@ -1335,9 +1335,7 @@ static NTSTATUS sock_transmit( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, socklen_t addr_len; HANDLE wait_handle; NTSTATUS status; - ULONG_PTR information; ULONG options; - BOOL alerted; addr_len = sizeof(addr); if (getpeername( fd, &addr.addr, &addr_len ) != 0) @@ -1392,9 +1390,10 @@ static NTSTATUS sock_transmit( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, /* the server currently will never succeed immediately */ assert(status == STATUS_ALERTED || status == STATUS_PENDING || NT_ERROR(status)); - alerted = status == STATUS_ALERTED; - if (alerted) + if (status == STATUS_ALERTED) { + ULONG_PTR information; + status = try_transmit( fd, file_fd, async ); if (status == STATUS_DEVICE_NOT_READY) status = STATUS_PENDING; @@ -1405,22 +1404,21 @@ static NTSTATUS sock_transmit( HANDLE handle, HANDLE event, PIO_APC_ROUTINE apc, io->Status = status; io->Information = information; } + + set_async_direct_result( &wait_handle, status, information, TRUE ); } if (status != STATUS_PENDING) release_fileio( &async->io ); - if (alerted) + if (!status && !(options & (FILE_SYNCHRONOUS_IO_ALERT | FILE_SYNCHRONOUS_IO_NONALERT))) { - set_async_direct_result( &wait_handle, status, information, TRUE ); - if (!(options & (FILE_SYNCHRONOUS_IO_ALERT | FILE_SYNCHRONOUS_IO_NONALERT))) - { - /* Pretend we always do async I/O. The client can always retrieve - * the actual I/O status via the IO_STATUS_BLOCK. - */ - status = STATUS_PENDING; - } + /* Pretend we always do async I/O. The client can always retrieve + * the actual I/O status via the IO_STATUS_BLOCK. + */ + status = STATUS_PENDING; } + if (wait_handle) status = wait_async( wait_handle, options & FILE_SYNCHRONOUS_IO_ALERT ); return status; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/871
participants (2)
-
Zebediah Figura -
Zebediah Figura (＠zfigura)