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

[Fabian Maurer (＠DarkShadow44)]

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

--
  v6: setupapi: Add stub for DriverStoreFindDriverPackageW

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

[PATCH v6 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..24f0537fe9c 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 < MAX_PATH)
+        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..864c8d6939b 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:\nonexistent.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:\nonexistent.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:\nonexistent.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) /* win7 */ || result == E_INVALIDARG /* win10 */, "Got %lx\n", result);
+
+    result = pDriverStoreFindDriverPackageW(L"c:\nonexistent.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:\nonexistent.inf", 0, 0, 9, 0, buffer, &len);
+    ok(result == E_INVALIDARG, "Got %lx\n", result);
+
+    len = 259;
+    result = pDriverStoreFindDriverPackageW(L"c:\nonexistent.inf", 0, 0, 9, 0, buffer, &len);
+    ok(result == E_INVALIDARG, "Got %lx\n", result);
+
+    len = 260;
+    result = pDriverStoreFindDriverPackageW(L"c:\nonexistent.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 v6 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=139924

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

[PATCH v6 0/1] MR4343: setupapi: Add stub for DriverStoreFindDriverPackageW - approved

[Alex Henrie (＠alexhenrie)]

This merge request was approved by Alex Henrie.

[Zebediah Figura (＠zfigura)]

> 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.

That doesn't mean that's always the right thing to do. It takes some amount of judgement, I think. Behaviour that's specific to some Windows versions is an argument in favour of broken(), and so is behaviour that simply doesn't make sense, and behaviour that contradicts the documentation. And vice versa. But I don't think there are any hard rules.

[Alex Henrie (＠alexhenrie)]

On Sun Nov 19 18:51:32 2023 +0000, Zebediah Figura 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.
> That doesn't mean that's always the right thing to do. It takes some
> amount of judgement, I think. Behaviour that's specific to some Windows
> versions is an argument in favour of broken(), and so is behaviour that
> simply doesn't make sense, and behaviour that contradicts the
> documentation. And vice versa. But I don't think there are any hard rules.

Sounds reasonable. Does that mean you have no objections to merging this patch as it currently stands?

[Zebediah Figura (＠zfigura)]

Zebediah Figura (@zfigura) commented about dlls/setupapi/tests/query.c:
> - 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:\nonexistent.inf", 0, 0, 9, 0, buffer, &len);
> - todo_wine
> - ok(result == HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND), "Got %lx\n", result);

What is "9"? How do we know that it's flags, and if it is flags, should we be defining symbolic constants for these?

[Fabian Maurer (＠DarkShadow44)]

On Wed Nov 29 19:35:32 2023 +0000, Zebediah Figura wrote:
> What is "9"? How do we know that it's flags, and if it is flags, should
> we be defining symbolic constants for these?

I don't know, it's what is passed by the program, and win64 wants that or it won't work. I just made an educated guess and called it flags, I don't know much more.