[PATCH v4 0/1] MR4343: setupapi: Add stub for DriverStoreFindDriverPackageW

[Fabian Maurer (＠DarkShadow44)]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45455

--
  v4: setupapi: Add stub for DriverStoreFindDriverPackageW

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

[PATCH v4 1/1] setupapi: Add stub for DriverStoreFindDriverPackageW

[Fabian Maurer]

From: Fabian Maurer

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=45455
---
 dlls/setupapi/query.c       | 10 ++++++
 dlls/setupapi/setupapi.spec |  1 +
 dlls/setupapi/tests/query.c | 67 +++++++++++++++++++++++++++++++++++++
 3 files changed, 78 insertions(+)

diff --git a/dlls/setupapi/query.c b/dlls/setupapi/query.c
index 88efea17473..ae06871670b 100644
--- a/dlls/setupapi/query.c
+++ b/dlls/setupapi/query.c
@@ -732,6 +732,16 @@ BOOL WINAPI SetupGetInfDriverStoreLocationW(
     return FALSE;
 }

+HRESULT WINAPI DriverStoreFindDriverPackageW(const WCHAR *path_in, void *unk2, void *unk3, DWORD flags, void *unk5, WCHAR *path_out, DWORD *path_size)
+{
+    FIXME("%s, %p, %p, %lu, %p, %p, %p, %lu stub!\n", debugstr_w(path_in), unk2, unk3, flags, unk5, path_out, path_size, path_size ? *path_size : 0);
+    if (!path_in || !path_out || !path_size || *path_size < 260)
+        return E_INVALIDARG;
+
+    wcscpy(path_out, path_in);
+    return S_OK;
+}
+
 BOOL WINAPI SetupQueryInfVersionInformationA(SP_INF_INFORMATION *info, UINT index, const char *key, char *buff,
     DWORD size, DWORD *req_size)
 {
diff --git a/dlls/setupapi/setupapi.spec b/dlls/setupapi/setupapi.spec
index 7578fb25c9c..f80d389daac 100644
--- a/dlls/setupapi/setupapi.spec
+++ b/dlls/setupapi/setupapi.spec
@@ -208,6 +208,7 @@
 @ stub DelimStringToMultiSz
 @ stub DestroyTextFileReadBuffer
 @ stdcall DoesUserHavePrivilege(wstr)
+@ stdcall DriverStoreFindDriverPackageW(ptr ptr ptr long ptr ptr ptr)
 @ stdcall DuplicateString(wstr)
 @ stdcall EnablePrivilege(wstr long)
 @ stub ExtensionPropSheetPageProc
diff --git a/dlls/setupapi/tests/query.c b/dlls/setupapi/tests/query.c
index f7aeba41153..f00622900da 100644
--- a/dlls/setupapi/tests/query.c
+++ b/dlls/setupapi/tests/query.c
@@ -513,6 +513,72 @@ static void test_SetupGetTargetPath(void)
     DeleteFileA(inf_filename);
 }

+static void test_DriverStoreFindDriverPackageW(void)
+{
+    HMODULE library;
+    HRESULT result;
+    WCHAR buffer[500];
+    DWORD len;
+    HRESULT (WINAPI *pDriverStoreFindDriverPackageW)(const WCHAR*, void*, void*, DWORD, void*, WCHAR*, DWORD*);
+
+    library = LoadLibraryA("setupapi.dll");
+    ok(library != NULL, "Failed to load setupapi.dll\n");
+    if (!library) return;
+
+    pDriverStoreFindDriverPackageW = (void *)GetProcAddress(library, "DriverStoreFindDriverPackageW");
+    if (!pDriverStoreFindDriverPackageW)
+    {
+        win_skip("Can't find DriverStoreFindDriverPackageW\n");
+        return;
+    }
+
+    len = ARRAY_SIZE(buffer);
+
+    /* No invalid parameters, with flags */
+    result = pDriverStoreFindDriverPackageW(L"c:\nonexistant.inf", 0, 0, 9, 0, buffer, &len);
+    todo_wine
+    ok(result == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), "Got %lx\n", result);
+
+    /* No invalid parameters, no flags */
+    result = pDriverStoreFindDriverPackageW(L"c:\nonexistant.inf", 0, 0, 0, 0, buffer, &len);
+    if (sizeof(void *) == 4)
+        todo_wine
+        ok(result == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), "Got %lx\n", result);
+    else
+        todo_wine
+        ok(result == E_INVALIDARG, "Got %lx\n", result); /* Win64 needs flags 0x9, or it gives invalid parameter */
+
+    /* Invalid parameter tests */
+
+    result = pDriverStoreFindDriverPackageW(L"c:\nonexistant.inf", 0, 0, 9, 0, 0, &len);
+    ok(result == E_INVALIDARG, "Got %lx\n", result);
+
+    result = pDriverStoreFindDriverPackageW(0, 0, 0, 9, 0, buffer, &len);
+    ok(result == E_INVALIDARG, "Got %lx\n", result);
+
+    result = pDriverStoreFindDriverPackageW(L"", 0, 0, 9, 0, buffer, &len);
+    todo_wine
+    ok(result == HRESULT_FROM_WIN32(ERROR_INVALID_NAME) || result == E_INVALIDARG, "Got %lx\n", result); /* Win7 and Win10 have different results */
+
+    result = pDriverStoreFindDriverPackageW(L"c:\nonexistant.inf", 0, 0, 9, 0, buffer, 0);
+    ok(result == E_INVALIDARG, "Got %lx\n", result);
+
+    /* Tests with different length parameter */
+
+    len = 0;
+    result = pDriverStoreFindDriverPackageW(L"c:\nonexistant.inf", 0, 0, 9, 0, buffer, &len);
+    ok(result == E_INVALIDARG, "Got %lx\n", result);
+
+    len = 259;
+    result = pDriverStoreFindDriverPackageW(L"c:\nonexistant.inf", 0, 0, 9, 0, buffer, &len);
+    ok(result == E_INVALIDARG, "Got %lx\n", result);
+
+    len = 260;
+    result = pDriverStoreFindDriverPackageW(L"c:\nonexistant.inf", 0, 0, 9, 0, buffer, &len);
+    todo_wine
+    ok(result == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), "Got %lx\n", result);
+}
+
 START_TEST(query)
 {
     get_directories();
@@ -521,4 +587,5 @@ START_TEST(query)
     test_SetupGetSourceFileLocation();
     test_SetupGetSourceInfo();
     test_SetupGetTargetPath();
+    test_DriverStoreFindDriverPackageW();
 }

[PATCH v4 1/1] setupapi: Add stub for DriverStoreFindDriverPackageW

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

Your paranoid android.

=== w7pro64 (64 bit report) ===

setupapi:
query.c:549: Test failed: Got 80070002

=== w864 (64 bit report) ===

setupapi:
query.c:549: Test failed: Got 80070002

=== w1064v1507 (64 bit report) ===

setupapi:
query.c:549: Test failed: Got 80070002

=== w1064v1809 (64 bit report) ===

setupapi:
query.c:549: Test failed: Got 80070002

=== w1064_2qxl (64 bit report) ===

setupapi:
query.c:549: Test failed: Got 80070002

=== w1064_adm (64 bit report) ===

setupapi:
query.c:549: Test failed: Got 80070002

=== w1064_tsign (64 bit report) ===

setupapi:
query.c:549: Test failed: Got 80070002

=== w10pro64 (64 bit report) ===

setupapi:
query.c:549: Test failed: Got 80070002

=== w10pro64_ar (64 bit report) ===

setupapi:
query.c:549: Test failed: Got 80070002

=== w10pro64_ja (64 bit report) ===

setupapi:
query.c:549: Test failed: Got 80070002

=== w10pro64_zh_CN (64 bit report) ===

setupapi:
query.c:549: Test failed: Got 80070002

=== w11pro64_amd (64 bit report) ===

setupapi:
query.c:549: Test failed: Got 80070002

[Fabian Maurer (＠DarkShadow44)]

On Wed Nov 15 21:21:23 2023 +0000, Alex Henrie wrote:
> In the past when I've added tests for behavior that differs from one
> Windows version to another, I've been asked to designate one behavior as
> the "correct" behavior and mark the other as broken. In this case, the
> applications in question require the function to report success, so
> testing and implementing the function's argument checking is really just
> to help us understand what the arguments are. The patch already has an
> if statement that returns E_INVALIDARG, and it would be slightly simpler
> to add to the existing if statement than to add a second if statement to
> return HRESULT_FROM_WIN32(ERROR_INVALID_NAME).
> If you would prefer to return HRESULT_FROM_WIN32(ERROR_INVALID_NAME)
> instead, or to not handle that error case at all in the stub
> implementation (as Fabian originally proposed), that's totally fine. The
> applications we're trying to support will work either way.

Seeing how it is only Win7 that returns `HRESULT_FROM_WIN32(ERROR_INVALID_NAME)` I'd argue it was the wrong behavior. Everything over Win10 returns ERROR_INVALID_NAME.

[Fabian Maurer (＠DarkShadow44)]

On Thu Nov 16 02:50:32 2023 +0000, Fabian Maurer wrote:
> changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/4343/diffs?diff_id=83877&start_sha=ef9cf2c3f926b095eccf0fb16e2647d8d337c2d4#a07c248f9876b034937c83f343dfeb8c7ad4de9f_535_535)

Thanks, fixed.

[Alex Henrie (＠alexhenrie)]

On Thu Nov 16 02:50:31 2023 +0000, Fabian Maurer wrote:
> changed this line in [version 4 of the diff](/wine/wine/-/merge_requests/4343/diffs?diff_id=83877&start_sha=ef9cf2c3f926b095eccf0fb16e2647d8d337c2d4#a07c248f9876b034937c83f343dfeb8c7ad4de9f_561_561)

You mean that Windows 10 returns E_INVALIDARG, right? To clarify what returns what, instead of putting a comment at the end of the line, could you please write `ok(result == HRESULT_FROM_WIN32(ERROR_INVALID_NAME) /* win7 */ || result == E_INVALIDARG /* win10 */, ...`?

[Alex Henrie (＠alexhenrie)]

Alex Henrie (@alexhenrie) commented about dlls/setupapi/query.c:
>  return FALSE;
>
> }
>
> +HRESULT WINAPI DriverStoreFindDriverPackageW(const WCHAR *path_in, void *unk2, void *unk3, DWORD flags, void *unk5, WCHAR *path_out, DWORD *path_size)
> +{
> - FIXME("%s, %p, %p, %lu, %p, %p, %p, %lu stub!\n", debugstr_w(path_in), unk2, unk3, flags, unk5, path_out, path_size, path_size ? *path_size : 0);
> - if (!path_in || !path_out || !path_size || *path_size < 260)

Personally I would prefer `*path_size < MAX_PATH` instead of `*path_size < 260`, but it's a matter of taste. What you wrote is fine.

[Fabian Maurer (＠DarkShadow44)]

On Thu Nov 16 04:38:59 2023 +0000, Alex Henrie wrote:
> Personally I would prefer `*path_size < MAX_PATH` instead of `*path_size
> < 260`, but it's a matter of taste. What you wrote is fine.

Tbh, I just didn't make the connection to MAX_PATH. I find it nicer with the macro as well, so I changed it.