Code review comment for lp:~midauth-pisa-devs/hipl/midauth-firewall

Begin forwarded message:
> From: René Hummen <email address hidden>
> Date: 2. November 2011 17:57:02 MEZ
> To: <email address hidden>
> Subject: Re: [hipl-dev] Re: [Merge] lp:~midauth-pisa-devs/hipl/midauth-firewall into lp:hipl
>
> On 25.10.2011, at 23:15, Christof Mroz wrote:
>> On 25.10.2011 16:35, René Hummen wrote:
>>>>>>> + if (!found) {
>>>>>>> + HIP_ERROR("CHALLENGE_RESPONSE found, but no matching nonce\n");
>>>>>>> + return ignore_missing_challenge_response ? 1 : 0;
>>>>>>> + }
>>>>>>> +
>>>>>>> + return found ? 1 : 0;
>>>>>>
>>>>>> You just checked found, no need to do it again.
>>>>>
>>>>> I guess that this has been done to ensure correct type conversion from
>>>>> bool to int.
>>>>
>>>> Read the code again, it's nonsense, "found" is initialized once and
>>>> never set again.
>>>
>>> You're right. I removed it and changed it to
>>>
>>>>>>> return ignore_missing_challenge_response ? 1 : 0;
>>
>> This doesn't look correct... you signal failure even though the CHALLENGE_RESPONSE was found? I think Diego meant to simply write
>>
>> return 1;
>>
>> Because we know that found is true at this point.
>
> found was always false before I removed the variable. Hence, !found evaluates to true. As this would be the path that the original code would follow, I decided to mimic it. Furthermore, if a correct response is found the while loop returns to the caller.