[PATCH 1/3 resend] setupapi/tests: Change return values from coinst functions.

[Jeff Smith]

As co_error and class_error both return 0xdeadbeef, use of 0xdeadbeef as
a canary value as well can lead to ambiguous tests results.

As 0xdeadbeef is a typical canary value in wine, change the return
values for co_error and class_error, avoiding the potential ambiguity.

Signed-off-by: Jeff Smith <whydoubt(a)gmail.com>
---
Patches 2 and 3 will work without patch 1.  This patch just makes tests
marginally more precise if an error does get caught.

 dlls/setupapi/tests/coinst.c  |  4 ++--
 dlls/setupapi/tests/devinst.c | 10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/dlls/setupapi/tests/coinst.c b/dlls/setupapi/tests/coinst.c
index 5e8d890cfa5..604bf74d1eb 100644
--- a/dlls/setupapi/tests/coinst.c
+++ b/dlls/setupapi/tests/coinst.c
@@ -47,7 +47,7 @@ DWORD WINAPI class_default(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *
 
 DWORD WINAPI class_error(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device)
 {
-    return 0xdeadbeef;
+    return E_FAIL;
 }
 
 DWORD WINAPI co_success(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device,
@@ -67,5 +67,5 @@ DWORD WINAPI CoDeviceInstall(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA
 DWORD WINAPI co_error(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device,
         COINSTALLER_CONTEXT_DATA *context)
 {
-    return 0xdeadbeef;
+    return E_FAIL;
 }
diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c
index d4c82dea1f6..80abc44029c 100644
--- a/dlls/setupapi/tests/devinst.c
+++ b/dlls/setupapi/tests/devinst.c
@@ -2727,17 +2727,17 @@ static void test_class_installer(void)
 
     ret = SetupDiCallClassInstaller(DIF_ALLOW_INSTALL, set, &device);
     ok(!ret, "Expected failure.\n");
-    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
+    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
 
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
     ret = SetupDiCallClassInstaller(DIF_REGISTERDEVICE, set, &device);
     ok(!ret, "Expected failure.\n");
-    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
+    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
 
     ret = SetupDiCallClassInstaller(DIF_REMOVE, set, &device);
     ok(!ret, "Expected failure.\n");
-    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
+    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
 
     SetupDiDestroyDeviceInfoList(set);
@@ -2875,12 +2875,12 @@ static void test_class_coinstaller(void)
 
     ret = SetupDiCallClassInstaller(DIF_ALLOW_INSTALL, set, &device);
     ok(!ret, "Expected failure.\n");
-    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
+    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
 
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
     ret = SetupDiCallClassInstaller(DIF_REGISTERDEVICE, set, &device);
     ok(!ret, "Expected failure.\n");
-    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
+    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
 
     SetupDiDestroyDeviceInfoList(set);
-- 
2.23.0

[PATCH 2/3 resend] setupapi/tests: Add error tests for SetupDiDestroyDeviceInfoList.

[Jeff Smith]

Signed-off-by: Jeff Smith <whydoubt(a)gmail.com>
---
 dlls/setupapi/tests/devinst.c | 40 ++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c
index 80abc44029c..06308ed4e99 100644
--- a/dlls/setupapi/tests/devinst.c
+++ b/dlls/setupapi/tests/devinst.c
@@ -2708,7 +2708,10 @@ static void test_class_installer(void)
     ok(*coinst_last_message == DIF_REMOVE, "Got unexpected message %#x.\n", *coinst_last_message);
     *coinst_callback_count = 0;
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2740,7 +2743,10 @@ static void test_class_installer(void)
     ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     /* Test returning ERROR_DI_DO_DEFAULT. */
 
@@ -2766,7 +2772,10 @@ static void test_class_installer(void)
     ok(ret, "Failed to call class installer, error %#x.\n", GetLastError());
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     /* The default entry point is ClassInstall(). */
 
@@ -2786,7 +2795,10 @@ static void test_class_installer(void)
     ok(*coinst_last_message == DIF_ALLOW_INSTALL, "Got unexpected message %#x.\n", *coinst_last_message);
     *coinst_callback_count = 0;
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2854,7 +2866,10 @@ static void test_class_coinstaller(void)
     ok(*coinst_last_message == DIF_REMOVE, "Got unexpected message %#x.\n", *coinst_last_message);
     *coinst_callback_count = 0;
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     todo_wine ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     todo_wine ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2883,7 +2898,10 @@ static void test_class_coinstaller(void)
     ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     /* The default entry point is CoDeviceInstall(). */
 
@@ -2906,7 +2924,10 @@ static void test_class_coinstaller(void)
     ok(*coinst_last_message == DIF_ALLOW_INSTALL, "Got unexpected message %#x.\n", *coinst_last_message);
     *coinst_callback_count = 0;
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2954,7 +2975,10 @@ static void test_call_class_installer(void)
     ok(ret, "Failed to call class installer, error %#x.\n", GetLastError());
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     load_resource("coinst.dll", "C:\\windows\\system32\\winetest_coinst.dll");
 
-- 
2.23.0

[PATCH 3/3 resend] setupapi: Clear error on SetupDiDestroyDeviceInfoList success.

[Jeff Smith]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49332
Signed-off-by: Jeff Smith <whydoubt(a)gmail.com>
---
 dlls/setupapi/devinst.c       |  1 +
 dlls/setupapi/tests/devinst.c | 16 ++++++++--------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/dlls/setupapi/devinst.c b/dlls/setupapi/devinst.c
index 9dacde29acb..de0413e74f5 100644
--- a/dlls/setupapi/devinst.c
+++ b/dlls/setupapi/devinst.c
@@ -2854,6 +2854,7 @@ BOOL WINAPI SetupDiDestroyDeviceInfoList(HDEVINFO devinfo)
     }
     heap_free(set);
 
+    SetLastError(ERROR_SUCCESS);
     return TRUE;
 }
 
diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c
index 06308ed4e99..707fa8351d1 100644
--- a/dlls/setupapi/tests/devinst.c
+++ b/dlls/setupapi/tests/devinst.c
@@ -2711,7 +2711,7 @@ static void test_class_installer(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2746,7 +2746,7 @@ static void test_class_installer(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     /* Test returning ERROR_DI_DO_DEFAULT. */
 
@@ -2775,7 +2775,7 @@ static void test_class_installer(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     /* The default entry point is ClassInstall(). */
 
@@ -2798,7 +2798,7 @@ static void test_class_installer(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2869,7 +2869,7 @@ static void test_class_coinstaller(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     todo_wine ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     todo_wine ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2901,7 +2901,7 @@ static void test_class_coinstaller(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     /* The default entry point is CoDeviceInstall(). */
 
@@ -2927,7 +2927,7 @@ static void test_class_coinstaller(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2978,7 +2978,7 @@ static void test_call_class_installer(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     load_resource("coinst.dll", "C:\\windows\\system32\\winetest_coinst.dll");
 
-- 
2.23.0

[Zebediah Figura]

On 7/24/20 9:34 AM, Jeff Smith wrote:
> As co_error and class_error both return 0xdeadbeef, use of 0xdeadbeef as
> a canary value as well can lead to ambiguous tests results.
>
> As 0xdeadbeef is a typical canary value in wine, change the return
> values for co_error and class_error, avoiding the potential ambiguity.

Is there a reason why we can't just use a different canary value in 2/3?
> Signed-off-by: Jeff Smith <whydoubt(a)gmail.com>
> ---
> Patches 2 and 3 will work without patch 1.  This patch just makes tests
> marginally more precise if an error does get caught.
>
> dlls/setupapi/tests/coinst.c  |  4 ++--
>  dlls/setupapi/tests/devinst.c | 10 +++++-----
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/dlls/setupapi/tests/coinst.c b/dlls/setupapi/tests/coinst.c
> index 5e8d890cfa5..604bf74d1eb 100644
> --- a/dlls/setupapi/tests/coinst.c
> +++ b/dlls/setupapi/tests/coinst.c
> @@ -47,7 +47,7 @@ DWORD WINAPI class_default(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *
>
> DWORD WINAPI class_error(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device)
>  {
> -    return 0xdeadbeef;
> +    return E_FAIL;
>  }
>
> DWORD WINAPI co_success(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device,
> @@ -67,5 +67,5 @@ DWORD WINAPI CoDeviceInstall(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA
>  DWORD WINAPI co_error(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device,
>          COINSTALLER_CONTEXT_DATA *context)
>  {
> -    return 0xdeadbeef;
> +    return E_FAIL;
>  }
> diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c
> index d4c82dea1f6..80abc44029c 100644
> --- a/dlls/setupapi/tests/devinst.c
> +++ b/dlls/setupapi/tests/devinst.c
> @@ -2727,17 +2727,17 @@ static void test_class_installer(void)
>
> ret = SetupDiCallClassInstaller(DIF_ALLOW_INSTALL, set, &device);
>      ok(!ret, "Expected failure.\n");
> -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
>
> ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
>      ret = SetupDiCallClassInstaller(DIF_REGISTERDEVICE, set, &device);
>      ok(!ret, "Expected failure.\n");
> -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
>      ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
>
> ret = SetupDiCallClassInstaller(DIF_REMOVE, set, &device);
>      ok(!ret, "Expected failure.\n");
> -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
>      ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
>
> SetupDiDestroyDeviceInfoList(set);
> @@ -2875,12 +2875,12 @@ static void test_class_coinstaller(void)
>
> ret = SetupDiCallClassInstaller(DIF_ALLOW_INSTALL, set, &device);
>      ok(!ret, "Expected failure.\n");
> -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
>
> ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
>      ret = SetupDiCallClassInstaller(DIF_REGISTERDEVICE, set, &device);
>      ok(!ret, "Expected failure.\n");
> -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
>      ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
>
> SetupDiDestroyDeviceInfoList(set);

[Jeff Smith]

On Fri, Jul 24, 2020 at 9:53 AM Zebediah Figura <z.figura12(a)gmail.com> wrote:
> On 7/24/20 9:34 AM, Jeff Smith wrote:
> > As co_error and class_error both return 0xdeadbeef, use of 0xdeadbeef as
> > a canary value as well can lead to ambiguous tests results.
> >
> > As 0xdeadbeef is a typical canary value in wine, change the return
> > values for co_error and class_error, avoiding the potential ambiguity.
>
> Is there a reason why we can't just use a different canary value in 2/3?

Technically, that would work as well, and that is what I did in the original
version of this patchset.  There was an objection raised though.
While I'm not strongly attached to one way or the other, I could see the
point of using deadbeef for the canary, and returning a different value
from co_error/class_error, as done in this patchset.
> > Signed-off-by: Jeff Smith <whydoubt(a)gmail.com>
> > ---
> > Patches 2 and 3 will work without patch 1.  This patch just makes tests
> > marginally more precise if an error does get caught.
> >
> > dlls/setupapi/tests/coinst.c  |  4 ++--
> >  dlls/setupapi/tests/devinst.c | 10 +++++-----
> >  2 files changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/dlls/setupapi/tests/coinst.c b/dlls/setupapi/tests/coinst.c
> > index 5e8d890cfa5..604bf74d1eb 100644
> > --- a/dlls/setupapi/tests/coinst.c
> > +++ b/dlls/setupapi/tests/coinst.c
> > @@ -47,7 +47,7 @@ DWORD WINAPI class_default(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *
> >
> > DWORD WINAPI class_error(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device)
> >  {
> > -    return 0xdeadbeef;
> > +    return E_FAIL;
> >  }
> >
> > DWORD WINAPI co_success(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device,
> > @@ -67,5 +67,5 @@ DWORD WINAPI CoDeviceInstall(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA
> >  DWORD WINAPI co_error(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device,
> >          COINSTALLER_CONTEXT_DATA *context)
> >  {
> > -    return 0xdeadbeef;
> > +    return E_FAIL;
> >  }
> > diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c
> > index d4c82dea1f6..80abc44029c 100644
> > --- a/dlls/setupapi/tests/devinst.c
> > +++ b/dlls/setupapi/tests/devinst.c
> > @@ -2727,17 +2727,17 @@ static void test_class_installer(void)
> >
> > ret = SetupDiCallClassInstaller(DIF_ALLOW_INSTALL, set, &device);
> >      ok(!ret, "Expected failure.\n");
> > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> >
> > ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> >      ret = SetupDiCallClassInstaller(DIF_REGISTERDEVICE, set, &device);
> >      ok(!ret, "Expected failure.\n");
> > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> >      ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> >
> > ret = SetupDiCallClassInstaller(DIF_REMOVE, set, &device);
> >      ok(!ret, "Expected failure.\n");
> > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> >      ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> >
> > SetupDiDestroyDeviceInfoList(set);
> > @@ -2875,12 +2875,12 @@ static void test_class_coinstaller(void)
> >
> > ret = SetupDiCallClassInstaller(DIF_ALLOW_INSTALL, set, &device);
> >      ok(!ret, "Expected failure.\n");
> > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> >
> > ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> >      ret = SetupDiCallClassInstaller(DIF_REGISTERDEVICE, set, &device);
> >      ok(!ret, "Expected failure.\n");
> > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> >      ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> >
> > SetupDiDestroyDeviceInfoList(set);

[Zebediah Figura]

On 7/24/20 11:08 AM, Jeff Smith wrote:
> On Fri, Jul 24, 2020 at 9:53 AM Zebediah Figura <z.figura12(a)gmail.com> wrote:
> > On 7/24/20 9:34 AM, Jeff Smith wrote:
> > > As co_error and class_error both return 0xdeadbeef, use of 0xdeadbeef as
> > > a canary value as well can lead to ambiguous tests results.
> > >
> > > As 0xdeadbeef is a typical canary value in wine, change the return
> > > values for co_error and class_error, avoiding the potential ambiguity.
> >
> > Is there a reason why we can't just use a different canary value in 2/3?
>
> Technically, that would work as well, and that is what I did in the original
> version of this patchset.  There was an objection raised though.
> While I'm not strongly attached to one way or the other, I could see the
> point of using deadbeef for the canary, and returning a different value
> from co_error/class_error, as done in this patchset.

I think that objection was rather specious. If nothing else, I don't
think E_FAIL is a very good canary value.
> > > Signed-off-by: Jeff Smith <whydoubt(a)gmail.com>
> > > ---
> > > Patches 2 and 3 will work without patch 1.  This patch just makes tests
> > > marginally more precise if an error does get caught.
> > >
> > > dlls/setupapi/tests/coinst.c  |  4 ++--
> > >  dlls/setupapi/tests/devinst.c | 10 +++++-----
> > >  2 files changed, 7 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/dlls/setupapi/tests/coinst.c b/dlls/setupapi/tests/coinst.c
> > > index 5e8d890cfa5..604bf74d1eb 100644
> > > --- a/dlls/setupapi/tests/coinst.c
> > > +++ b/dlls/setupapi/tests/coinst.c
> > > @@ -47,7 +47,7 @@ DWORD WINAPI class_default(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *
> > >
> > > DWORD WINAPI class_error(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device)
> > >  {
> > > -    return 0xdeadbeef;
> > > +    return E_FAIL;
> > >  }
> > >
> > > DWORD WINAPI co_success(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device,
> > > @@ -67,5 +67,5 @@ DWORD WINAPI CoDeviceInstall(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA
> > >  DWORD WINAPI co_error(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device,
> > >          COINSTALLER_CONTEXT_DATA *context)
> > >  {
> > > -    return 0xdeadbeef;
> > > +    return E_FAIL;
> > >  }
> > > diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c
> > > index d4c82dea1f6..80abc44029c 100644
> > > --- a/dlls/setupapi/tests/devinst.c
> > > +++ b/dlls/setupapi/tests/devinst.c
> > > @@ -2727,17 +2727,17 @@ static void test_class_installer(void)
> > >
> > > ret = SetupDiCallClassInstaller(DIF_ALLOW_INSTALL, set, &device);
> > >      ok(!ret, "Expected failure.\n");
> > > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> > >
> > > ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> > >      ret = SetupDiCallClassInstaller(DIF_REGISTERDEVICE, set, &device);
> > >      ok(!ret, "Expected failure.\n");
> > > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> > >      ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> > >
> > > ret = SetupDiCallClassInstaller(DIF_REMOVE, set, &device);
> > >      ok(!ret, "Expected failure.\n");
> > > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> > >      ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> > >
> > > SetupDiDestroyDeviceInfoList(set);
> > > @@ -2875,12 +2875,12 @@ static void test_class_coinstaller(void)
> > >
> > > ret = SetupDiCallClassInstaller(DIF_ALLOW_INSTALL, set, &device);
> > >      ok(!ret, "Expected failure.\n");
> > > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> > >
> > > ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> > >      ret = SetupDiCallClassInstaller(DIF_REGISTERDEVICE, set, &device);
> > >      ok(!ret, "Expected failure.\n");
> > > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> > >      ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> > >
> > > SetupDiDestroyDeviceInfoList(set);

[Jeff Smith]

On Fri, Jul 24, 2020 at 11:10 AM Zebediah Figura <z.figura12(a)gmail.com> wrote:
> On 7/24/20 11:08 AM, Jeff Smith wrote:
> > On Fri, Jul 24, 2020 at 9:53 AM Zebediah Figura <z.figura12(a)gmail.com> wrote:
> > > On 7/24/20 9:34 AM, Jeff Smith wrote:
> > > > As co_error and class_error both return 0xdeadbeef, use of 0xdeadbeef as
> > > > a canary value as well can lead to ambiguous tests results.
> > > >
> > > > As 0xdeadbeef is a typical canary value in wine, change the return
> > > > values for co_error and class_error, avoiding the potential ambiguity.
> > >
> > > Is there a reason why we can't just use a different canary value in 2/3?
> >
> > Technically, that would work as well, and that is what I did in the original
> > version of this patchset.  There was an objection raised though.
> > While I'm not strongly attached to one way or the other, I could see the
> > point of using deadbeef for the canary, and returning a different value
> > from co_error/class_error, as done in this patchset.
>
> I think that objection was rather specious.

Maybe.
> If nothing else, I don't
> think E_FAIL is a very good canary value.

Going to go out on a limb and say that the value returned by
co_error/class_error
(which is different from the canary) isn't that important as long as
it is unlikely to
be ambiguous as to where it came from.  So I just chose about the most generic
COM error that I could come up with.

As much as I want the tests to be the best they can be, TBH it's
really the fix (patch 3)
that I care about, so I'll quit splitting hairs.  You think forgetting
patch 1 and using a
different canary for patch 2 is the way to go?
> > > > Signed-off-by: Jeff Smith <whydoubt(a)gmail.com>
> > > > ---
> > > > Patches 2 and 3 will work without patch 1.  This patch just makes tests
> > > > marginally more precise if an error does get caught.
> > > >
> > > > dlls/setupapi/tests/coinst.c  |  4 ++--
> > > >  dlls/setupapi/tests/devinst.c | 10 +++++-----
> > > >  2 files changed, 7 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/dlls/setupapi/tests/coinst.c b/dlls/setupapi/tests/coinst.c
> > > > index 5e8d890cfa5..604bf74d1eb 100644
> > > > --- a/dlls/setupapi/tests/coinst.c
> > > > +++ b/dlls/setupapi/tests/coinst.c
> > > > @@ -47,7 +47,7 @@ DWORD WINAPI class_default(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *
> > > >
> > > > DWORD WINAPI class_error(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device)
> > > >  {
> > > > -    return 0xdeadbeef;
> > > > +    return E_FAIL;
> > > >  }
> > > >
> > > > DWORD WINAPI co_success(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device,
> > > > @@ -67,5 +67,5 @@ DWORD WINAPI CoDeviceInstall(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA
> > > >  DWORD WINAPI co_error(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device,
> > > >          COINSTALLER_CONTEXT_DATA *context)
> > > >  {
> > > > -    return 0xdeadbeef;
> > > > +    return E_FAIL;
> > > >  }
> > > > diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c
> > > > index d4c82dea1f6..80abc44029c 100644
> > > > --- a/dlls/setupapi/tests/devinst.c
> > > > +++ b/dlls/setupapi/tests/devinst.c
> > > > @@ -2727,17 +2727,17 @@ static void test_class_installer(void)
> > > >
> > > > ret = SetupDiCallClassInstaller(DIF_ALLOW_INSTALL, set, &device);
> > > >      ok(!ret, "Expected failure.\n");
> > > > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > > > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> > > >
> > > > ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> > > >      ret = SetupDiCallClassInstaller(DIF_REGISTERDEVICE, set, &device);
> > > >      ok(!ret, "Expected failure.\n");
> > > > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > > > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> > > >      ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> > > >
> > > > ret = SetupDiCallClassInstaller(DIF_REMOVE, set, &device);
> > > >      ok(!ret, "Expected failure.\n");
> > > > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > > > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> > > >      ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> > > >
> > > > SetupDiDestroyDeviceInfoList(set);
> > > > @@ -2875,12 +2875,12 @@ static void test_class_coinstaller(void)
> > > >
> > > > ret = SetupDiCallClassInstaller(DIF_ALLOW_INSTALL, set, &device);
> > > >      ok(!ret, "Expected failure.\n");
> > > > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > > > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> > > >
> > > > ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> > > >      ret = SetupDiCallClassInstaller(DIF_REGISTERDEVICE, set, &device);
> > > >      ok(!ret, "Expected failure.\n");
> > > > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > > > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> > > >      ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> > > >
> > > > SetupDiDestroyDeviceInfoList(set);

[Zebediah Figura]

On 7/24/20 2:42 PM, Jeff Smith wrote:
> On Fri, Jul 24, 2020 at 11:10 AM Zebediah Figura <z.figura12(a)gmail.com> wrote:
> > On 7/24/20 11:08 AM, Jeff Smith wrote:
> > > On Fri, Jul 24, 2020 at 9:53 AM Zebediah Figura <z.figura12(a)gmail.com> wrote:
> > > > On 7/24/20 9:34 AM, Jeff Smith wrote:
> > > > > As co_error and class_error both return 0xdeadbeef, use of 0xdeadbeef as
> > > > > a canary value as well can lead to ambiguous tests results.
> > > > >
> > > > > As 0xdeadbeef is a typical canary value in wine, change the return
> > > > > values for co_error and class_error, avoiding the potential ambiguity.
> > > >
> > > > Is there a reason why we can't just use a different canary value in 2/3?
> > >
> > > Technically, that would work as well, and that is what I did in the original
> > > version of this patchset.  There was an objection raised though.
> > > While I'm not strongly attached to one way or the other, I could see the
> > > point of using deadbeef for the canary, and returning a different value
> > > from co_error/class_error, as done in this patchset.
> >
> > I think that objection was rather specious.
>
> Maybe.
> > If nothing else, I don't
> > think E_FAIL is a very good canary value.
>
> Going to go out on a limb and say that the value returned by
> co_error/class_error
> (which is different from the canary) isn't that important as long as
> it is unlikely to
> be ambiguous as to where it came from.  So I just chose about the most generic
> COM error that I could come up with.
>
> As much as I want the tests to be the best they can be, TBH it's
> really the fix (patch 3)
> that I care about, so I'll quit splitting hairs.  You think forgetting
> patch 1 and using a
> different canary for patch 2 is the way to go?

I certainly don't want to give the impression that it matters very much.

I don't expect setupapi to throw E_FAIL, but it's less obvious than,
say, 0xdead**** that it's a sentinel.
> > > > > Signed-off-by: Jeff Smith <whydoubt(a)gmail.com>
> > > > > ---
> > > > > Patches 2 and 3 will work without patch 1.  This patch just makes tests
> > > > > marginally more precise if an error does get caught.
> > > > >
> > > > > dlls/setupapi/tests/coinst.c  |  4 ++--
> > > > >  dlls/setupapi/tests/devinst.c | 10 +++++-----
> > > > >  2 files changed, 7 insertions(+), 7 deletions(-)
> > > > >
> > > > > diff --git a/dlls/setupapi/tests/coinst.c b/dlls/setupapi/tests/coinst.c
> > > > > index 5e8d890cfa5..604bf74d1eb 100644
> > > > > --- a/dlls/setupapi/tests/coinst.c
> > > > > +++ b/dlls/setupapi/tests/coinst.c
> > > > > @@ -47,7 +47,7 @@ DWORD WINAPI class_default(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *
> > > > >
> > > > > DWORD WINAPI class_error(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device)
> > > > >  {
> > > > > -    return 0xdeadbeef;
> > > > > +    return E_FAIL;
> > > > >  }
> > > > >
> > > > > DWORD WINAPI co_success(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device,
> > > > > @@ -67,5 +67,5 @@ DWORD WINAPI CoDeviceInstall(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA
> > > > >  DWORD WINAPI co_error(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device,
> > > > >          COINSTALLER_CONTEXT_DATA *context)
> > > > >  {
> > > > > -    return 0xdeadbeef;
> > > > > +    return E_FAIL;
> > > > >  }
> > > > > diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c
> > > > > index d4c82dea1f6..80abc44029c 100644
> > > > > --- a/dlls/setupapi/tests/devinst.c
> > > > > +++ b/dlls/setupapi/tests/devinst.c
> > > > > @@ -2727,17 +2727,17 @@ static void test_class_installer(void)
> > > > >
> > > > > ret = SetupDiCallClassInstaller(DIF_ALLOW_INSTALL, set, &device);
> > > > >      ok(!ret, "Expected failure.\n");
> > > > > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > > > > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> > > > >
> > > > > ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> > > > >      ret = SetupDiCallClassInstaller(DIF_REGISTERDEVICE, set, &device);
> > > > >      ok(!ret, "Expected failure.\n");
> > > > > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > > > > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> > > > >      ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> > > > >
> > > > > ret = SetupDiCallClassInstaller(DIF_REMOVE, set, &device);
> > > > >      ok(!ret, "Expected failure.\n");
> > > > > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > > > > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> > > > >      ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> > > > >
> > > > > SetupDiDestroyDeviceInfoList(set);
> > > > > @@ -2875,12 +2875,12 @@ static void test_class_coinstaller(void)
> > > > >
> > > > > ret = SetupDiCallClassInstaller(DIF_ALLOW_INSTALL, set, &device);
> > > > >      ok(!ret, "Expected failure.\n");
> > > > > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > > > > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> > > > >
> > > > > ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> > > > >      ret = SetupDiCallClassInstaller(DIF_REGISTERDEVICE, set, &device);
> > > > >      ok(!ret, "Expected failure.\n");
> > > > > -    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
> > > > > +    ok(GetLastError() == E_FAIL, "Got unexpected error %#x.\n", GetLastError());
> > > > >      ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
> > > > >
> > > > > SetupDiDestroyDeviceInfoList(set);

[PATCH v2 1/3] setupapi/tests: Change return values from coinst functions.

[Jeff Smith]

As co_error and class_error both return 0xdeadbeef, use of 0xdeadbeef as
a canary value as well can lead to ambiguous tests results.

As 0xdeadbeef is a typical canary value in wine, change the return
values for co_error and class_error, avoiding the potential ambiguity.

Signed-off-by: Jeff Smith <whydoubt(a)gmail.com>
---
v2: Replace E_FAIL with 0xdeadc0de for the value returned by class_error
    and co_error (i.e. the value we want to be distinct from the canary
    value set in tests/devinst.c).

 dlls/setupapi/tests/coinst.c  |  4 ++--
 dlls/setupapi/tests/devinst.c | 10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/dlls/setupapi/tests/coinst.c b/dlls/setupapi/tests/coinst.c
index 5e8d890cfa5..ba526a5d3a1 100644
--- a/dlls/setupapi/tests/coinst.c
+++ b/dlls/setupapi/tests/coinst.c
@@ -47,7 +47,7 @@ DWORD WINAPI class_default(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *
 
 DWORD WINAPI class_error(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device)
 {
-    return 0xdeadbeef;
+    return 0xdeadc0de;
 }
 
 DWORD WINAPI co_success(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device,
@@ -67,5 +67,5 @@ DWORD WINAPI CoDeviceInstall(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA
 DWORD WINAPI co_error(DI_FUNCTION function, HDEVINFO set, SP_DEVINFO_DATA *device,
         COINSTALLER_CONTEXT_DATA *context)
 {
-    return 0xdeadbeef;
+    return 0xdeadc0de;
 }
diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c
index d4c82dea1f6..cb77fdf3c46 100644
--- a/dlls/setupapi/tests/devinst.c
+++ b/dlls/setupapi/tests/devinst.c
@@ -2727,17 +2727,17 @@ static void test_class_installer(void)
 
     ret = SetupDiCallClassInstaller(DIF_ALLOW_INSTALL, set, &device);
     ok(!ret, "Expected failure.\n");
-    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
+    ok(GetLastError() == 0xdeadc0de, "Got unexpected error %#x.\n", GetLastError());
 
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
     ret = SetupDiCallClassInstaller(DIF_REGISTERDEVICE, set, &device);
     ok(!ret, "Expected failure.\n");
-    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
+    ok(GetLastError() == 0xdeadc0de, "Got unexpected error %#x.\n", GetLastError());
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
 
     ret = SetupDiCallClassInstaller(DIF_REMOVE, set, &device);
     ok(!ret, "Expected failure.\n");
-    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
+    ok(GetLastError() == 0xdeadc0de, "Got unexpected error %#x.\n", GetLastError());
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
 
     SetupDiDestroyDeviceInfoList(set);
@@ -2875,12 +2875,12 @@ static void test_class_coinstaller(void)
 
     ret = SetupDiCallClassInstaller(DIF_ALLOW_INSTALL, set, &device);
     ok(!ret, "Expected failure.\n");
-    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
+    ok(GetLastError() == 0xdeadc0de, "Got unexpected error %#x.\n", GetLastError());
 
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
     ret = SetupDiCallClassInstaller(DIF_REGISTERDEVICE, set, &device);
     ok(!ret, "Expected failure.\n");
-    ok(GetLastError() == 0xdeadbeef, "Got unexpected error %#x.\n", GetLastError());
+    ok(GetLastError() == 0xdeadc0de, "Got unexpected error %#x.\n", GetLastError());
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
 
     SetupDiDestroyDeviceInfoList(set);
-- 
2.23.0

[PATCH v2 2/3] setupapi/tests: Add error tests for SetupDiDestroyDeviceInfoList.

[Jeff Smith]

Signed-off-by: Jeff Smith <whydoubt(a)gmail.com>
---
 dlls/setupapi/tests/devinst.c | 40 ++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c
index cb77fdf3c46..58b6303608d 100644
--- a/dlls/setupapi/tests/devinst.c
+++ b/dlls/setupapi/tests/devinst.c
@@ -2708,7 +2708,10 @@ static void test_class_installer(void)
     ok(*coinst_last_message == DIF_REMOVE, "Got unexpected message %#x.\n", *coinst_last_message);
     *coinst_callback_count = 0;
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2740,7 +2743,10 @@ static void test_class_installer(void)
     ok(GetLastError() == 0xdeadc0de, "Got unexpected error %#x.\n", GetLastError());
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     /* Test returning ERROR_DI_DO_DEFAULT. */
 
@@ -2766,7 +2772,10 @@ static void test_class_installer(void)
     ok(ret, "Failed to call class installer, error %#x.\n", GetLastError());
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     /* The default entry point is ClassInstall(). */
 
@@ -2786,7 +2795,10 @@ static void test_class_installer(void)
     ok(*coinst_last_message == DIF_ALLOW_INSTALL, "Got unexpected message %#x.\n", *coinst_last_message);
     *coinst_callback_count = 0;
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2854,7 +2866,10 @@ static void test_class_coinstaller(void)
     ok(*coinst_last_message == DIF_REMOVE, "Got unexpected message %#x.\n", *coinst_last_message);
     *coinst_callback_count = 0;
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     todo_wine ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     todo_wine ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2883,7 +2898,10 @@ static void test_class_coinstaller(void)
     ok(GetLastError() == 0xdeadc0de, "Got unexpected error %#x.\n", GetLastError());
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     /* The default entry point is CoDeviceInstall(). */
 
@@ -2906,7 +2924,10 @@ static void test_class_coinstaller(void)
     ok(*coinst_last_message == DIF_ALLOW_INSTALL, "Got unexpected message %#x.\n", *coinst_last_message);
     *coinst_callback_count = 0;
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2954,7 +2975,10 @@ static void test_call_class_installer(void)
     ok(ret, "Failed to call class installer, error %#x.\n", GetLastError());
     ok(!device_is_registered(set, &device), "Expected device not to be registered.\n");
 
-    SetupDiDestroyDeviceInfoList(set);
+    SetLastError(0xdeadbeef);
+    ret = SetupDiDestroyDeviceInfoList(set);
+    ok(ret, "Failed to destroy device list.\n");
+    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     load_resource("coinst.dll", "C:\\windows\\system32\\winetest_coinst.dll");
 
-- 
2.23.0

[PATCH v2 3/3] setupapi: Clear error on SetupDiDestroyDeviceInfoList success.

[Jeff Smith]

Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=49332
Signed-off-by: Jeff Smith <whydoubt(a)gmail.com>
---
 dlls/setupapi/devinst.c       |  1 +
 dlls/setupapi/tests/devinst.c | 16 ++++++++--------
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/dlls/setupapi/devinst.c b/dlls/setupapi/devinst.c
index 9dacde29acb..de0413e74f5 100644
--- a/dlls/setupapi/devinst.c
+++ b/dlls/setupapi/devinst.c
@@ -2854,6 +2854,7 @@ BOOL WINAPI SetupDiDestroyDeviceInfoList(HDEVINFO devinfo)
     }
     heap_free(set);
 
+    SetLastError(ERROR_SUCCESS);
     return TRUE;
 }
 
diff --git a/dlls/setupapi/tests/devinst.c b/dlls/setupapi/tests/devinst.c
index 58b6303608d..48b84c21e84 100644
--- a/dlls/setupapi/tests/devinst.c
+++ b/dlls/setupapi/tests/devinst.c
@@ -2711,7 +2711,7 @@ static void test_class_installer(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2746,7 +2746,7 @@ static void test_class_installer(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     /* Test returning ERROR_DI_DO_DEFAULT. */
 
@@ -2775,7 +2775,7 @@ static void test_class_installer(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     /* The default entry point is ClassInstall(). */
 
@@ -2798,7 +2798,7 @@ static void test_class_installer(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2869,7 +2869,7 @@ static void test_class_coinstaller(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     todo_wine ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     todo_wine ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2901,7 +2901,7 @@ static void test_class_coinstaller(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     /* The default entry point is CoDeviceInstall(). */
 
@@ -2927,7 +2927,7 @@ static void test_class_coinstaller(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     ok(*coinst_callback_count == 1, "Got %d callbacks.\n", *coinst_callback_count);
     ok(*coinst_last_message == DIF_DESTROYPRIVATEDATA, "Got unexpected message %#x.\n", *coinst_last_message);
@@ -2978,7 +2978,7 @@ static void test_call_class_installer(void)
     SetLastError(0xdeadbeef);
     ret = SetupDiDestroyDeviceInfoList(set);
     ok(ret, "Failed to destroy device list.\n");
-    todo_wine ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
+    ok(!GetLastError(), "Got unexpected error %#x.\n", GetLastError());
 
     load_resource("coinst.dll", "C:\\windows\\system32\\winetest_coinst.dll");
 
-- 
2.23.0