[PATCH v2 0/4] MR6089: setupapi: Fixes for get_source_info().

[Elizabeth Figura (＠zfigura)]

--
  v2: setupapi: Use SetupGetIntField() in SetupGetSourceFileLocation().
      setupapi: Return the file's relative path from SetupGetSourceFileLocation().
      setupapi: Correctly interpret the INFCONTEXT parameter in SetupGetSourceFileLocation().
      setupapi/tests: Add more tests for SetupGetSourceFileLocation().

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

[PATCH v2 1/4] setupapi/tests: Add more tests for SetupGetSourceFileLocation().

[Elizabeth Figura]

From: Elizabeth Figura

---
 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);

[PATCH v2 2/4] setupapi: Correctly interpret the INFCONTEXT parameter in SetupGetSourceFileLocation().

[Elizabeth Figura]

From: Elizabeth Figura

---
 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));

[PATCH v2 3/4] setupapi: Return the file's relative path from SetupGetSourceFileLocation().

[Elizabeth Figura]

From: Elizabeth Figura

---
 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);

[PATCH v2 4/4] setupapi: Use SetupGetIntField() in SetupGetSourceFileLocation().

[Elizabeth Figura]

From: Elizabeth Figura

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

[PATCH v2 4/4] setupapi: Use SetupGetIntField() in SetupGetSourceFileLocation().

[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 full results can be found at:
https://testbot.winehq.org/JobDetails.pl?Key=147153

Your paranoid android.

=== build (build log) ===

error: patch failed: dlls/setupapi/tests/query.c:47
error: patch failed: dlls/setupapi/query.c:413
error: patch failed: dlls/setupapi/tests/query.c:356
error: patch failed: dlls/setupapi/query.c:390
error: patch failed: dlls/setupapi/tests/query.c:349
error: patch failed: dlls/setupapi/query.c:369
Task: Patch failed to apply

=== debian11 (build log) ===

error: patch failed: dlls/setupapi/tests/query.c:47
error: patch failed: dlls/setupapi/query.c:413
error: patch failed: dlls/setupapi/tests/query.c:356
error: patch failed: dlls/setupapi/query.c:390
error: patch failed: dlls/setupapi/tests/query.c:349
error: patch failed: dlls/setupapi/query.c:369
Task: Patch failed to apply

=== debian11b (build log) ===

error: patch failed: dlls/setupapi/tests/query.c:47
error: patch failed: dlls/setupapi/query.c:413
error: patch failed: dlls/setupapi/tests/query.c:356
error: patch failed: dlls/setupapi/query.c:390
error: patch failed: dlls/setupapi/tests/query.c:349
error: patch failed: dlls/setupapi/query.c:369
Task: Patch failed to apply