[PATCH 0/3] MR871: ntdll: Fill the IOSB only inside the if (alerted) block in multiple functions.

[Zebediah Figura (＠zfigura)]

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/871

[PATCH 1/3] ntdll: Fill the IOSB in sock_recv() only inside the "if (alerted)" block.

[Zebediah Figura]

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..e718b1a7570 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))
         {
             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

[PATCH 2/3] ntdll: Fill the IOSB in sock_send() only inside the "if (alerted)" block.

[Zebediah Figura]

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 | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c
index e718b1a7570..2b6d5f0f556 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))
         {
             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

[PATCH 3/3] ntdll: Fill the IOSB in sock_transmit() only inside the "if (alerted)" block.

[Zebediah Figura]

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 | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/dlls/ntdll/unix/socket.c b/dlls/ntdll/unix/socket.c
index 2b6d5f0f556..bfc7489d347 100644
--- a/dlls/ntdll/unix/socket.c
+++ b/dlls/ntdll/unix/socket.c
@@ -1388,25 +1388,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)
         {
             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

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about dlls/ntdll/unix/socket.c:
> * 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))

`alerted` is always true here.

```suggestion:-0+0
        if (!NT_ERROR(status))
```

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/871#note_8691

[Zebediah Figura]

On 9/16/22 07:49, Jinoh Kang (@iamahuman) wrote:
> Jinoh Kang (@iamahuman) commented about dlls/ntdll/unix/socket.c:
> > * 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))
>
> `alerted` is always true here.
>
> ```suggestion:-0+0
>          if (!NT_ERROR(status))
> ```

Thanks, I meant to change that too but missed it.