Michael Stefaniuc [mailto:[email protected]]
> > 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
