Re: dlls/advapi32/tests/security.c - Bugfix in test!
Jakob Eriksson <jakov(a)vmlinux.org> writes:
--- dlls/advapi32/tests/security.c 14 Mar 2005 17:20:58 -0000 1.12 +++ dlls/advapi32/tests/security.c 16 Mar 2005 09:32:28 -0000 @@ -289,8 +289,8 @@ luid.LowPart = i; cchName = sizeof(buf); ret = pLookupPrivilegeNameA(NULL, &luid, buf, &cchName); - ok( ret && GetLastError() != ERROR_NO_SUCH_PRIVILEGE, - "LookupPrivilegeNameA(0.%ld) failed: %ld\n", i, GetLastError()); + if (GetLastError() != ERROR_NO_SUCH_PRIVILEGE) + ok( ret, "LookupPrivilegeNameA(0.%ld) failed: %ld\n", i, GetLastError());
It doesn't really make sense to check the last error if the function succeeded. -- Alexandre Julliard julliard(a)winehq.org
Alexandre Julliard wrote:
Jakob Eriksson <jakov(a)vmlinux.org> writes:
--- dlls/advapi32/tests/security.c 14 Mar 2005 17:20:58 -0000 1.12 +++ dlls/advapi32/tests/security.c 16 Mar 2005 09:32:28 -0000 @@ -289,8 +289,8 @@ luid.LowPart = i; cchName = sizeof(buf); ret = pLookupPrivilegeNameA(NULL, &luid, buf, &cchName); - ok( ret && GetLastError() != ERROR_NO_SUCH_PRIVILEGE, - "LookupPrivilegeNameA(0.%ld) failed: %ld\n", i, GetLastError()); + if (GetLastError() != ERROR_NO_SUCH_PRIVILEGE) + ok( ret, "LookupPrivilegeNameA(0.%ld) failed: %ld\n", i, GetLastError());
It doesn't really make sense to check the last error if the function succeeded.
True. Don't know what I was thinking. regards, Jakob
Jakob Eriksson wrote:
Alexandre Julliard wrote:
Jakob Eriksson <jakov(a)vmlinux.org> writes:
--- dlls/advapi32/tests/security.c 14 Mar 2005 17:20:58 -0000 1.12 +++ dlls/advapi32/tests/security.c 16 Mar 2005 09:32:28 -0000 @@ -289,8 +289,8 @@ luid.LowPart = i; cchName = sizeof(buf); ret = pLookupPrivilegeNameA(NULL, &luid, buf, &cchName); - ok( ret && GetLastError() != ERROR_NO_SUCH_PRIVILEGE, - "LookupPrivilegeNameA(0.%ld) failed: %ld\n", i, GetLastError()); + if (GetLastError() != ERROR_NO_SUCH_PRIVILEGE) + ok( ret, "LookupPrivilegeNameA(0.%ld) failed: %ld\n", i, GetLastError());
It doesn't really make sense to check the last error if the function succeeded.
True. Don't know what I was thinking.
Now I know. If the error was ERROR_NO_SUCH_PRIVILEGE, it's ok, we don't care. Move on. NT4 has this behaviour. If it isn't, but ret is 0, AKA LookupPrivilegeName() failed, I wan't to know exactly what the error was. It's a trace. So I think the patch is valid, there is method to the madness. regards, Jakob
Jakob Eriksson wrote:
Jakob Eriksson wrote:
Alexandre Julliard wrote:
Jakob Eriksson <jakov(a)vmlinux.org> writes:
--- dlls/advapi32/tests/security.c 14 Mar 2005 17:20:58 -0000 1.12 +++ dlls/advapi32/tests/security.c 16 Mar 2005 09:32:28 -0000 @@ -289,8 +289,8 @@ luid.LowPart = i; cchName = sizeof(buf); ret = pLookupPrivilegeNameA(NULL, &luid, buf, &cchName); - ok( ret && GetLastError() != ERROR_NO_SUCH_PRIVILEGE, - "LookupPrivilegeNameA(0.%ld) failed: %ld\n", i, GetLastError()); + if (GetLastError() != ERROR_NO_SUCH_PRIVILEGE) + ok( ret, "LookupPrivilegeNameA(0.%ld) failed: %ld\n", i, GetLastError());
It doesn't really make sense to check the last error if the function succeeded.
True. Don't know what I was thinking.
Now I know. If the error was ERROR_NO_SUCH_PRIVILEGE, it's ok, we don't care. Move on. NT4 has this behaviour.
What about: "ok( ret || GetLastError() == ERROR_NO_SUCH_PRIVILEGE,"? - Filip
On Sat, 19 Mar 2005, Jakob Eriksson wrote:
Now I know. If the error was ERROR_NO_SUCH_PRIVILEGE, it's ok, we don't care. Move on. NT4 has this behaviour.
If it isn't, but ret is 0, AKA LookupPrivilegeName() failed, I wan't to know exactly what the error was. It's a trace. So I think the patch is valid, there is method to the madness.
I don't quite follow this, but I'd suggest that, even if the patch is valid, a comment at this location explaining the logic is definitely called for. --scott D5 SLBM Ft. Meade arrangements Leitrim munitions SDI SSBN 731 struggle KUGOWN NORAD Washington SMOTH supercomputer quiche Khaddafi LPMEDLEY ( http://cscott.net/ )
participants (4)
-
Alexandre Julliard -
C. Scott Ananian -
Filip Navara -
Jakob Eriksson