RE: [rsaenh/tests] Don't check result twice (Coccinelle)

[Rolf Kalbermatter]

Michael Stefaniuc [mailto:[e-mail]]
> > But we are already returning a few lines above:
> >
> > 271     result = CryptCreateHash(hProv, CALG_MD2, 0, 0, &hHash);
> >  272     if (!result) {
> >  273         /* rsaenh compiled without OpenSSL */
> >  274         ok(GetLastError()==NTE_BAD_ALGID, "%08x\n",
>
> GetLastError());
> > 275         return FALSE;
> >  276     }
> >
> > We will not get here if result = 0;
> >
> > 277     ok(result, "%08x\n", GetLastError());
> >  278     if (!result) return FALSE;
> >
> > So that last one is needless.
>
> Sorry about that, I should have looked at the code and not only at the
> patch. But the ok() line 277 is pretty pointless there as it won't
> produce an error ever. Well of course you might want to keep it to
> increase the "successful tests" counter.

Actually that is not completely true. The first ok will usually produce an error except when the OpenSSL provider is not available and the second ok() will print out the success. It is equivalent to this:

272     if (!result) {
273         /* rsaenh compiled without OpenSSL */
274         ok(GetLastError()==NTE_BAD_ALGID, "%08x\n", GetLastError());
275         return FALSE;
276     }
        else {
277         ok(result, "%08x\n", GetLastError());
278         if (!result) return FALSE;
        }

And could be rewritten like this without functional change:

272     ok((result || GetLastError()==NTE_BAD_ALGID), "%08x\n", GetLastError());  
273     if (!result) return FALSE;

But boolean logic is in general not as obvious in one glance.

Rolf Kalbermatter