[PATCH 0/1] MR1367: ws2_32: Return success for setting SO_ERROR.

[Paul Gofman (＠gofman)]

As analysis performed in [1] suggests, Microsoft Flight Simulator depends on that.

1. https://github.com/ValveSoftware/Proton/issues/4134#issuecomment-1314432224

[PATCH 1/1] ws2_32: Return success for setting SO_ERROR.

[Paul Gofman]

From: Paul Gofman

---
 dlls/ws2_32/socket.c     | 6 +++---
 dlls/ws2_32/tests/sock.c | 1 -
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/dlls/ws2_32/socket.c b/dlls/ws2_32/socket.c
index 9e447f29d8c..6a03ec176d5 100644
--- a/dlls/ws2_32/socket.c
+++ b/dlls/ws2_32/socket.c
@@ -3173,9 +3173,9 @@ int WINAPI setsockopt( SOCKET s, int level, int optname, const char *optval, int
         }

case SO_ERROR:
-            FIXME( "SO_ERROR, stub!\n" );
-            SetLastError( WSAENOPROTOOPT );
-            return -1;
+            FIXME( "SO_ERROR, stub.\n" );
+            SetLastError( ERROR_SUCCESS );
+            return 0;

case SO_KEEPALIVE:
             if (optlen <= 0 || !optval)
diff --git a/dlls/ws2_32/tests/sock.c b/dlls/ws2_32/tests/sock.c
index d4d92b58c41..bcccaef9bc6 100644
--- a/dlls/ws2_32/tests/sock.c
+++ b/dlls/ws2_32/tests/sock.c
@@ -1431,7 +1431,6 @@ static void test_set_getsockopt(void)
     SetLastError(0xdeadbeef);
     i = 1234;
     err = setsockopt(s, SOL_SOCKET, SO_ERROR, (char *) &i, size);
-    todo_wine
     ok( !err && !WSAGetLastError(),
         "got %d with %d (expected 0 with 0)\n",
         err, WSAGetLastError());

[PATCH 1/1] ws2_32: Return success for setting SO_ERROR.

[Marvin]

Hi,

It looks like your patch introduced the new failures shown below.
Please investigate and fix them before resubmitting your patch.
If they are not new, fixing them anyway would help a lot. Otherwise
please ask for the known failures list to be updated.

The tests also ran into some preexisting test failures. If you know how
to fix them that would be helpful. See the TestBot job for the details:

The full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=126162

Your paranoid android.

=== debian11 (32 bit report) ===

crypt32:
cert.c:4191: Test failed: success
cert.c:4192: Test failed: got 00000000
cert.c:4193: Test failed: got 00000000

wmvcore:
wmvcore.c:1617: Test failed: Stream 0: Format 4: Got hr 0x8007000e.
Unhandled exception: page fault on read access to 0x00000000 in 32-bit code (0x00410d09).

[Alex Henrie (＠alexhenrie)]

I sent a patch a year ago to support setting SO_ERROR (and not just fake success), but it was not accepted, presumably because I didn't have a compelling enough use case: https://www.winehq.org/mailman3/hyperkitty/list/[e-mail]/thread...

[Paul Gofman]

Yes, I think it is possible to really support setting the last error on 
socket, somehow between the lines of your patch. But that seems rather 
convoluted and if the only known case depends on setting SO_ERROR to 
succeed I personally am not motivated to pursue finalizing such an 
implementation.

Do you know anything actually depending on that to fully work?

On 11/14/22 16:53, Alex Henrie (@alexhenrie) wrote:
> I sent a patch a year ago to support setting SO_ERROR (and not just fake success), but it was not accepted, presumably because I didn't have a compelling enough use case: https://www.winehq.org/mailman3/hyperkitty/list/[e-mail]/thread...

[Alex Henrie (＠alexhenrie)]

On Mon Nov 14 23:01:49 2022 +0000, **** wrote:
> Paul Gofman replied on the mailing list:
> Yes, I think it is possible to really support setting the last error on 
> socket, somehow between the lines of your patch. But that seems rather 
> convoluted and if the only known case depends on setting SO_ERROR to 
> succeed I personally am not motivated to pursue finalizing such an 
> implementation.
> Do you know anything actually depending on that to fully work?
> On 11/14/22 16:53, Alex Henrie (@alexhenrie) wrote:

I don't know of any "real-world" application that needs to set SO_ERROR and then read it back, but we do have a test for it, and implementing it properly would avoid a FIXME. It's fine if all you want is a stub, I just wanted to let you know that code to actually implement the option is there for the taking.

[PATCH 0/1] MR1367: ws2_32: Return success for setting SO_ERROR. - approved

[Zebediah Figura (＠zfigura)]

This merge request was approved by Zebediah Figura.

[Zebediah Figura (＠zfigura)]

On Mon Nov 14 23:09:22 2022 +0000, Alex Henrie wrote:
> I don't know of any "real-world" application that needs to set SO_ERROR
> and then read it back, but we do have a test for it, and implementing it
> properly would avoid a FIXME. It's fine if all you want is a stub, I
> just wanted to let you know that code to actually implement the option
> is there for the taking.

I recall replying in private, but I should have said so on the mailing list, so my apologies for that. In any case, I personally think actually implementing it is unnecessarily adding nontrivial code for no clear benefit.