[PATCH v4 0/1] MR4343: setupapi: Add stub for DriverStoreFindDriverPackageW
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
From: Fabian Maurer <dark.shadow4(a)web.de> 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(); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/4343
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
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4343#note_52461
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4343#note_52465
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 */, ...`?
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4343#note_52467
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. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/4343#note_52468
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.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/4343#note_52469
participants (4)
-
Alex Henrie (＠alexhenrie) -
Fabian Maurer
-
Fabian Maurer (＠DarkShadow44)
-
Marvin