On 7/24/20 2:42 PM, Jeff Smith wrote:
On Fri, Jul 24, 2020 at 11:10 AM Zebediah Figura z.figura12@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@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@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);