[PATCH 0/1] MR5574: explorer: Reset last error after loading drivers.

[Aida Jonikienė (＠DodoGTA)]

I'm not sure if this is the best approach for this problem (resetting the last error right before loading a program's entrypoint could be better).

[PATCH 1/1] explorer: Reset last error after loading drivers.

[Aida Jonikienė]

From: Aida Jonikienė

The mac graphics driver is loaded first (which on Linux obviously
fails and changes the last error which could cause issues in some
tests when compared to Windows).

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=50553
---
 programs/explorer/desktop.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/programs/explorer/desktop.c b/programs/explorer/desktop.c
index fb59258ad36..6f68218ce25 100644
--- a/programs/explorer/desktop.c
+++ b/programs/explorer/desktop.c
@@ -1015,6 +1015,7 @@ static void load_graphics_driver( const WCHAR *driver, GUID *guid )
         }
         name = next;
     }
+    SetLastError( ERROR_SUCCESS );

TRACE( "display %s driver %s\n", debugstr_guid(guid), debugstr_w(libname) );

[PATCH 1/1] explorer: Reset last error after loading drivers.

[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=145366

Your paranoid android.

=== debian11b (64 bit WoW report) ===

wmvcore:
wmvcore.c:2824: Test succeeded inside todo block: Got pts 2780000.
wmvcore.c:2831: Test failed: Got pts 2860000.
wmvcore.c:2850: Test failed: Got pts 2860000.
wmvcore.c:2869: Test succeeded inside todo block: Got pts 2860000.

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about programs/explorer/desktop.c:
>      }
>      name = next;
>  }
> - SetLastError( ERROR_SUCCESS );

How does this fix the bug? The bug is about SetEnvironmentVariableW. It's not about a program that indirectly uses it (explorer).

If I changed this line to `SetLastError( -1 );` or `SetLastError( -GetLastError() );`, does it cause anything? I see nothing that relies on the last error value from this point onward.

Again, I don't see how this fixes bug #50553 or anything related.

[Aida Jonikienė (＠DodoGTA)]

On Fri May  3 15:23:33 2024 +0000, Jinoh Kang wrote:
> How does this fix the bug? The bug is about SetEnvironmentVariableW.
> It's not about a program that indirectly uses it (explorer).
> If I changed this line to `SetLastError( -1 );` or `SetLastError(
> -GetLastError() );`, does it cause anything? I see nothing that relies
> on the last error value from this point onward.
> Again, I don't see how this fixes bug #50553 or anything related.

The first test case in that bug 50553 test has a wrong last error value when compared to Windows (Windows has ERROR_SUCCESS while Wine has ERROR_MOD_NOT_FOUND which comes from the LoadLibraryW() failure for winemac.drv which is a library that doesn't exist on Linux)

[Jinoh Kang (＠iamahuman)]

On Fri May  3 15:35:41 2024 +0000, Aida Jonikienė wrote:
> The first test case in that bug 50553 test has a wrong last error value
> when compared to Windows (Windows has ERROR_SUCCESS while Wine has
> ERROR_MOD_NOT_FOUND which comes from the LoadLibraryW() failure for
> winemac.drv which is a library that doesn't exist on Linux)

Bug 50553 is mainly about fixing Git for Windows and other apps that use SetEnvironmentVariableW (it's literally in the bug summary). Is this MR known to fix any app?

[Aida Jonikienė (＠DodoGTA)]

On Fri May  3 21:44:55 2024 +0000, Jinoh Kang wrote:
> Bug 50553 is mainly about fixing Git for Windows and other apps that use
> SetEnvironmentVariableW (it's literally in the bug summary). Is this MR
> known to fix any app?

I don't know of a real-world case that depends on this last error behavior (as I said before it's meant to fix that test program)

[Alexandre Julliard (＠julliard)]

explorer is a separate process, so this cannot possibly change last error in the application.

[PATCH 0/1] MR5574: explorer: Reset last error after loading drivers. - closed

[Alexandre Julliard (＠julliard)]

This merge request was closed by Alexandre Julliard.

[Aida Jonikienė (＠DodoGTA)]

After some relay debugging I can see the last error in the test application is being set by `libgcc_s_dw2-1.dll` not being present (so I guess my potential fix was in the wrong place then)