[PATCH 0/1] MR1367: ws2_32: Return success for setting SO_ERROR.
As analysis performed in [1] suggests, Microsoft Flight Simulator depends on that. 1. https://github.com/ValveSoftware/Proton/issues/4134#issuecomment-1314432224 -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1367
From: Paul Gofman <pgofman(a)codeweavers.com> --- 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()); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1367
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).
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/wine-devel(a)winehq.org/thre... -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1367#note_15803
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/wine-devel(a)winehq.org/thre...
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1367#note_15805
This merge request was approved by Zebediah Figura. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1367
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1367#note_15809
participants (6)
-
Alex Henrie (@alexhenrie) -
Marvin -
Paul Gofman -
Paul Gofman -
Paul Gofman (@gofman) -
Zebediah Figura (@zfigura)