Re: dlls/advapi32/tests/security.c - Bugfix in test!

[Alexandre Julliard]

Jakob Eriksson  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.

dlls/advapi32/tests/security.c - Bugfix in test!

[Jakob Eriksson]

Alexandre Julliard wrote:
> Jakob Eriksson  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

dlls/advapi32/tests/security.c - Bugfix in test!

[Jakob Eriksson]

Jakob Eriksson wrote:
> Alexandre Julliard wrote:
> > Jakob Eriksson  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

dlls/advapi32/tests/security.c - Bugfix in test!

[Filip Navara]

Jakob Eriksson wrote:
> Jakob Eriksson wrote:
> > Alexandre Julliard wrote:
> > > Jakob Eriksson  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

dlls/advapi32/tests/security.c - Bugfix in test!

[C. Scott Ananian]

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