Paul Vriens wrote:
> On 01/12/2010 02:51 PM, Michael Stefaniuc wrote:
>> Hello Paul,
>>
>> Paul Vriens wrote:
>>> On 01/12/2010 02:32 PM, Michael Stefaniuc wrote:
>>>> Paul Vriens wrote:
>>>>> We already check and return FALSE a few lines up.
>>>>>
>>>>> Found with the help of Coccinelle and the CocciCheck scripts.
>>>>>
>>>>> Changelog
>>>>> Don't check result twice (Coccinelle)
>>>>>
>>>> --- a/dlls/rsaenh/tests/rsaenh.c
>>>> +++ b/dlls/rsaenh/tests/rsaenh.c
>>>> @@ -275,7 +275,6 @@ static BOOL derive_key(ALG_ID aiAlgid, HCRYPTKEY
>>>> *phKey, DWORD len)
>>>> return FALSE;
>>>> }
>>>> ok(result, "%08x\n", GetLastError());
>>>> - if (!result) return FALSE;
>> ^^^ old result value
>>>> result = CryptHashData(hHash, pbData, sizeof(pbData), 0);
>> ^^^ new result value
>>>> ok(result, "%08x\n", GetLastError());
>>>> if (!result) return FALSE;
>>>> that is not the same "result"; it gets a new value after the first
>>>> "return FALSE".
>>>
>>> Where?
>>>
>>> The only thing between the "return FALSE;" and the "if (!result)" is an
>>> ok() statement.
>> I see a "result = CryptHashData()" in between.
>>
>> bye
>> michael
>>
>>
> 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.
bye
michael
