[PATCH 0/4] MR6078: setupapi: Fixes for SetupGetSourceFileLocation().
Preparation for fixing bug 48455. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/6078
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/setupapi/tests/query.c | 72 +++++++++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 2 deletions(-) diff --git a/dlls/setupapi/tests/query.c b/dlls/setupapi/tests/query.c index f7aeba41153..145ca6e12eb 100644 --- a/dlls/setupapi/tests/query.c +++ b/dlls/setupapi/tests/query.c @@ -47,10 +47,21 @@ static const char inf_data1[] = "[Version]\n" "Signature=\"$Chicago$\"\n" "AdvancedINF=2.5\n" + "[copy_section]\n" + "one.txt,,2\n" + "two.txt\n" + "four.txt,five.txt\n" + "six.txt,seven.txt\n" "[SourceDisksNames]\n" "2 = %SrcDiskName%, LANCOM\\LANtools\\lanconf.cab\n" + "4=,,,here\n" + "8=name\n" "[SourceDisksFiles]\n" - "lanconf.exe = 2\n" + "one.txt=2\n" + "two.txt=4,there\n" + "three.txt=6\n" + "five.txt=8\n" + "six.txt=10\n" "[DestinationDirs]\n" "DefaultDestDir = 24, %DefaultDest%\n" "[Strings]\n" @@ -312,6 +323,7 @@ static void test_SetupGetSourceFileLocation(void) char buffer[MAX_PATH] = "not empty", inf_filename[MAX_PATH]; UINT source_id; DWORD required, error; + INFCONTEXT ctx; HINF hinf; BOOL ret; @@ -327,13 +339,67 @@ static void test_SetupGetSourceFileLocation(void) required = 0; source_id = 0; - ret = SetupGetSourceFileLocationA(hinf, NULL, "lanconf.exe", &source_id, buffer, sizeof(buffer), &required); + ret = SetupGetSourceFileLocationA(hinf, NULL, "one.txt", &source_id, buffer, sizeof(buffer), &required); ok(ret, "SetupGetSourceFileLocation failed\n"); ok(required == 1, "unexpected required size: %ld\n", required); ok(source_id == 2, "unexpected source id: %d\n", source_id); ok(!lstrcmpA("", buffer), "unexpected result string: %s\n", buffer); + ret = SetupGetSourceFileLocationA(hinf, NULL, "two.txt", &source_id, buffer, sizeof(buffer), NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + ok(source_id == 4, "Got source id %u.\n", source_id); + todo_wine ok(!strcmp(buffer, "there"), "Got relative path %s.\n", debugstr_a(buffer)); + + ret = SetupFindFirstLineA(hinf, "copy_section", NULL, &ctx); + ok(ret, "Got error %lu.\n", GetLastError()); + + ret = SetupGetSourceFileLocationA(hinf, &ctx, "two.txt", &source_id, buffer, sizeof(buffer), NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + todo_wine ok(source_id == 2, "Got source id %u.\n", source_id); + todo_wine ok(!strcmp(buffer, ""), "Got relative path %s.\n", debugstr_a(buffer)); + + /* ctx should not be changed. */ + ret = SetupGetLineTextA(&ctx, NULL, NULL, NULL, buffer, sizeof(buffer), NULL); + todo_wine ok(!strcmp(buffer, "one.txt,,2"), "Got line %s.\n", debugstr_a(buffer)); + + /* Test when the source name differs from the destination name. + * It seems SetupGetSourceFileLocation() is buggy and doesn't take that + * into account; it always uses the destination name. */ + + ret = SetupFindNextLine(&ctx, &ctx); + ok(ret, "Got error %lu.\n", GetLastError()); + ret = SetupFindNextLine(&ctx, &ctx); + todo_wine ok(ret, "Got error %lu.\n", GetLastError()); + + SetLastError(0xdeadbeef); + ret = SetupGetSourceFileLocationA(hinf, &ctx, "two.txt", &source_id, buffer, sizeof(buffer), NULL); + todo_wine ok(!ret, "Expected failure.\n"); + todo_wine ok(GetLastError() == ERROR_LINE_NOT_FOUND, "Got error %lu.\n", GetLastError()); + + ret = SetupFindNextLine(&ctx, &ctx); + ok(ret, "Got error %lu.\n", GetLastError()); + + ret = SetupGetSourceFileLocationA(hinf, &ctx, "two.txt", &source_id, buffer, sizeof(buffer), NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + todo_wine ok(source_id == 10, "Got source id %u.\n", source_id); + todo_wine ok(!strcmp(buffer, ""), "Got relative path %s.\n", debugstr_a(buffer)); + + ret = SetupGetSourceFileLocationA(hinf, NULL, "three.txt", &source_id, buffer, sizeof(buffer), NULL); + todo_wine ok(ret, "Got error %lu.\n", GetLastError()); + todo_wine ok(source_id == 6, "Got source id %u.\n", source_id); + todo_wine ok(!strcmp(buffer, ""), "Got relative path %s.\n", debugstr_a(buffer)); + + SetLastError(0xdeadbeef); + ret = SetupGetSourceFileLocationA(hinf, NULL, "four.txt", &source_id, buffer, sizeof(buffer), NULL); + ok(!ret, "Expected failure.\n"); + ok(GetLastError() == ERROR_LINE_NOT_FOUND, "Got error %lu.\n", GetLastError()); + + ret = SetupGetSourceFileLocationA(hinf, NULL, "five.txt", &source_id, buffer, sizeof(buffer), NULL); + ok(ret, "Got error %lu.\n", GetLastError()); + ok(source_id == 8, "Got source id %u.\n", source_id); + ok(!strcmp(buffer, ""), "Got relative path %s.\n", debugstr_a(buffer)); + SetupCloseInfFile(hinf); DeleteFileA(inf_filename); @@ -348,8 +414,10 @@ static void test_SetupGetSourceFileLocation(void) hinf = SetupOpenInfFileA(inf_filename, NULL, INF_STYLE_OLDNT, NULL); ok(hinf != INVALID_HANDLE_VALUE, "could not open inf file\n"); + SetLastError(0xdeadbeef); ret = SetupGetSourceFileLocationA(hinf, NULL, "", &source_id, buffer, sizeof(buffer), &required); ok(!ret, "SetupGetSourceFileLocation succeeded\n"); + ok(GetLastError() == ERROR_LINE_NOT_FOUND, "Got error %lu.\n", GetLastError()); SetupCloseInfFile(hinf); DeleteFileA(inf_filename); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6078
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/setupapi/query.c | 24 +++++++++++++++++++++--- dlls/setupapi/tests/query.c | 14 +++++++------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/dlls/setupapi/query.c b/dlls/setupapi/query.c index 88efea17473..c94fec77124 100644 --- a/dlls/setupapi/query.c +++ b/dlls/setupapi/query.c @@ -413,9 +413,27 @@ BOOL WINAPI SetupGetSourceFileLocationW( HINF hinf, PINFCONTEXT context, PCWSTR TRACE("%p, %p, %s, %p, %p, 0x%08lx, %p\n", hinf, context, debugstr_w(filename), source_id, buffer, buffer_size, required_size); - if (!context) context = &ctx; + if (context) + { + WCHAR *ctx_filename; + DWORD filename_size; + + if (!SetupGetStringFieldW( context, 1, NULL, 0, &filename_size )) + return FALSE; + if (!(ctx_filename = malloc( filename_size * sizeof(WCHAR) ))) + return FALSE; + SetupGetStringFieldW( context, 1, ctx_filename, filename_size, NULL ); + + source_id_str = get_source_id( hinf, &ctx, ctx_filename ); + + free( ctx_filename ); + } + else + { + source_id_str = get_source_id( hinf, &ctx, filename ); + } - if (!(source_id_str = get_source_id( hinf, context, filename ))) + if (!source_id_str) return FALSE; *source_id = wcstol( source_id_str, &end, 10 ); @@ -426,7 +444,7 @@ BOOL WINAPI SetupGetSourceFileLocationW( HINF hinf, PINFCONTEXT context, PCWSTR } free( source_id_str ); - if (SetupGetStringFieldW( context, 4, buffer, buffer_size, required_size )) + if (SetupGetStringFieldW( &ctx, 4, buffer, buffer_size, required_size )) return TRUE; if (required_size) *required_size = 1; diff --git a/dlls/setupapi/tests/query.c b/dlls/setupapi/tests/query.c index 145ca6e12eb..8f4400b802e 100644 --- a/dlls/setupapi/tests/query.c +++ b/dlls/setupapi/tests/query.c @@ -356,12 +356,12 @@ static void test_SetupGetSourceFileLocation(void) ret = SetupGetSourceFileLocationA(hinf, &ctx, "two.txt", &source_id, buffer, sizeof(buffer), NULL); ok(ret, "Got error %lu.\n", GetLastError()); - todo_wine ok(source_id == 2, "Got source id %u.\n", source_id); - todo_wine ok(!strcmp(buffer, ""), "Got relative path %s.\n", debugstr_a(buffer)); + ok(source_id == 2, "Got source id %u.\n", source_id); + ok(!strcmp(buffer, ""), "Got relative path %s.\n", debugstr_a(buffer)); /* ctx should not be changed. */ ret = SetupGetLineTextA(&ctx, NULL, NULL, NULL, buffer, sizeof(buffer), NULL); - todo_wine ok(!strcmp(buffer, "one.txt,,2"), "Got line %s.\n", debugstr_a(buffer)); + ok(!strcmp(buffer, "one.txt,,2"), "Got line %s.\n", debugstr_a(buffer)); /* Test when the source name differs from the destination name. * It seems SetupGetSourceFileLocation() is buggy and doesn't take that @@ -370,18 +370,18 @@ static void test_SetupGetSourceFileLocation(void) ret = SetupFindNextLine(&ctx, &ctx); ok(ret, "Got error %lu.\n", GetLastError()); ret = SetupFindNextLine(&ctx, &ctx); - todo_wine ok(ret, "Got error %lu.\n", GetLastError()); + ok(ret, "Got error %lu.\n", GetLastError()); SetLastError(0xdeadbeef); ret = SetupGetSourceFileLocationA(hinf, &ctx, "two.txt", &source_id, buffer, sizeof(buffer), NULL); - todo_wine ok(!ret, "Expected failure.\n"); - todo_wine ok(GetLastError() == ERROR_LINE_NOT_FOUND, "Got error %lu.\n", GetLastError()); + ok(!ret, "Expected failure.\n"); + ok(GetLastError() == ERROR_LINE_NOT_FOUND, "Got error %lu.\n", GetLastError()); ret = SetupFindNextLine(&ctx, &ctx); ok(ret, "Got error %lu.\n", GetLastError()); ret = SetupGetSourceFileLocationA(hinf, &ctx, "two.txt", &source_id, buffer, sizeof(buffer), NULL); - ok(ret, "Got error %lu.\n", GetLastError()); + todo_wine ok(ret, "Got error %lu.\n", GetLastError()); todo_wine ok(source_id == 10, "Got source id %u.\n", source_id); todo_wine ok(!strcmp(buffer, ""), "Got relative path %s.\n", debugstr_a(buffer)); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6078
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/setupapi/query.c | 8 +------- dlls/setupapi/tests/query.c | 14 +++++++------- 2 files changed, 8 insertions(+), 14 deletions(-) diff --git a/dlls/setupapi/query.c b/dlls/setupapi/query.c index c94fec77124..40ec30ae11f 100644 --- a/dlls/setupapi/query.c +++ b/dlls/setupapi/query.c @@ -390,12 +390,6 @@ static LPWSTR get_source_id( HINF hinf, PINFCONTEXT context, PCWSTR filename ) return NULL; } - if (!SetupFindFirstLineW( hinf, source_disks_names_platform, source_id, context ) && - !SetupFindFirstLineW( hinf, source_disks_names, source_id, context )) - { - free( source_id ); - return NULL; - } return source_id; } @@ -444,7 +438,7 @@ BOOL WINAPI SetupGetSourceFileLocationW( HINF hinf, PINFCONTEXT context, PCWSTR } free( source_id_str ); - if (SetupGetStringFieldW( &ctx, 4, buffer, buffer_size, required_size )) + if (SetupGetStringFieldW( &ctx, 2, buffer, buffer_size, required_size )) return TRUE; if (required_size) *required_size = 1; diff --git a/dlls/setupapi/tests/query.c b/dlls/setupapi/tests/query.c index 8f4400b802e..d5de41b368a 100644 --- a/dlls/setupapi/tests/query.c +++ b/dlls/setupapi/tests/query.c @@ -349,7 +349,7 @@ static void test_SetupGetSourceFileLocation(void) ret = SetupGetSourceFileLocationA(hinf, NULL, "two.txt", &source_id, buffer, sizeof(buffer), NULL); ok(ret, "Got error %lu.\n", GetLastError()); ok(source_id == 4, "Got source id %u.\n", source_id); - todo_wine ok(!strcmp(buffer, "there"), "Got relative path %s.\n", debugstr_a(buffer)); + ok(!strcmp(buffer, "there"), "Got relative path %s.\n", debugstr_a(buffer)); ret = SetupFindFirstLineA(hinf, "copy_section", NULL, &ctx); ok(ret, "Got error %lu.\n", GetLastError()); @@ -381,14 +381,14 @@ static void test_SetupGetSourceFileLocation(void) ok(ret, "Got error %lu.\n", GetLastError()); ret = SetupGetSourceFileLocationA(hinf, &ctx, "two.txt", &source_id, buffer, sizeof(buffer), NULL); - todo_wine ok(ret, "Got error %lu.\n", GetLastError()); - todo_wine ok(source_id == 10, "Got source id %u.\n", source_id); - todo_wine ok(!strcmp(buffer, ""), "Got relative path %s.\n", debugstr_a(buffer)); + ok(ret, "Got error %lu.\n", GetLastError()); + ok(source_id == 10, "Got source id %u.\n", source_id); + ok(!strcmp(buffer, ""), "Got relative path %s.\n", debugstr_a(buffer)); ret = SetupGetSourceFileLocationA(hinf, NULL, "three.txt", &source_id, buffer, sizeof(buffer), NULL); - todo_wine ok(ret, "Got error %lu.\n", GetLastError()); - todo_wine ok(source_id == 6, "Got source id %u.\n", source_id); - todo_wine ok(!strcmp(buffer, ""), "Got relative path %s.\n", debugstr_a(buffer)); + ok(ret, "Got error %lu.\n", GetLastError()); + ok(source_id == 6, "Got source id %u.\n", source_id); + ok(!strcmp(buffer, ""), "Got relative path %s.\n", debugstr_a(buffer)); SetLastError(0xdeadbeef); ret = SetupGetSourceFileLocationA(hinf, NULL, "four.txt", &source_id, buffer, sizeof(buffer), NULL); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6078
From: Zebediah Figura <zfigura(a)codeweavers.com> --- dlls/setupapi/query.c | 50 ++++++++++++------------------------------- 1 file changed, 14 insertions(+), 36 deletions(-) diff --git a/dlls/setupapi/query.c b/dlls/setupapi/query.c index 40ec30ae11f..707492ac984 100644 --- a/dlls/setupapi/query.c +++ b/dlls/setupapi/query.c @@ -369,30 +369,6 @@ BOOL WINAPI SetupGetSourceFileLocationA( HINF hinf, PINFCONTEXT context, PCSTR f return ret; } -static LPWSTR get_source_id( HINF hinf, PINFCONTEXT context, PCWSTR filename ) -{ - DWORD size; - LPWSTR source_id; - - if (!SetupFindFirstLineW( hinf, source_disks_files_platform, filename, context ) && - !SetupFindFirstLineW( hinf, source_disks_files, filename, context )) - return NULL; - - if (!SetupGetStringFieldW( context, 1, NULL, 0, &size )) - return NULL; - - if (!(source_id = malloc( size * sizeof(WCHAR) ))) - return NULL; - - if (!SetupGetStringFieldW( context, 1, source_id, size, NULL )) - { - free( source_id ); - return NULL; - } - - return source_id; -} - /*********************************************************************** * SetupGetSourceFileLocationW (SETUPAPI.@) */ @@ -402,7 +378,7 @@ BOOL WINAPI SetupGetSourceFileLocationW( HINF hinf, PINFCONTEXT context, PCWSTR PDWORD required_size ) { INFCONTEXT ctx; - WCHAR *end, *source_id_str; + int id; TRACE("%p, %p, %s, %p, %p, 0x%08lx, %p\n", hinf, context, debugstr_w(filename), source_id, buffer, buffer_size, required_size); @@ -418,25 +394,27 @@ BOOL WINAPI SetupGetSourceFileLocationW( HINF hinf, PINFCONTEXT context, PCWSTR return FALSE; SetupGetStringFieldW( context, 1, ctx_filename, filename_size, NULL ); - source_id_str = get_source_id( hinf, &ctx, ctx_filename ); + if (!SetupFindFirstLineW( hinf, source_disks_files_platform, ctx_filename, &ctx ) && + !SetupFindFirstLineW( hinf, source_disks_files, ctx_filename, &ctx )) + { + free( ctx_filename ); + return FALSE; + } free( ctx_filename ); } else { - source_id_str = get_source_id( hinf, &ctx, filename ); + if (!SetupFindFirstLineW( hinf, source_disks_files_platform, filename, &ctx ) && + !SetupFindFirstLineW( hinf, source_disks_files, filename, &ctx )) + { + return FALSE; + } } - if (!source_id_str) + if (!SetupGetIntField( &ctx, 1, &id )) return FALSE; - - *source_id = wcstol( source_id_str, &end, 10 ); - if (end == source_id_str || *end) - { - free( source_id_str ); - return FALSE; - } - free( source_id_str ); + *source_id = id; if (SetupGetStringFieldW( &ctx, 2, buffer, buffer_size, required_size )) return TRUE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/6078
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=147085 Your paranoid android. === debian11b (64 bit WoW report) === secur32: schannel.c:1138: Test failed: Wrong buffer size schannel.c:1187: Test failed: InitializeSecurityContext failed: 80090304 schannel.c:1575: Test failed: got 80090304 urlmon: protocol.c:857: Test failed: szStatusText = L"text/javascript"
participants (3)
-
Elizabeth Figura (＠zfigura) -
Marvin -
Zebediah Figura