Re: [PATCH 2/3] git-cvsserver: protect against NULL in crypt(3)

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 17, 2021, 03:43 UTC
Message-ID: <xmqqsfy350kc.fsf@gitster.g>
In-Reply-To: <CAPUEspjqD5zy8TLuFA96usU7FYi=0wF84y7NgOVFqegtxL9zbw@mail.gmail.com>

Carlo Arenas <carenas@gmail.com> writes:

Show 18 quoted lines

>> It is not wrong per-se to separate the two checks into two separate
>> parts of the conditional, but because we check for definedness only
>> because comparison of it with $1 makes sense only when it is
>> defined, writing it either like this,
>>
>>                 if (defined $hash and $hash eq $1) {
>>                         $auth_ok = 1;
>>                 }
>>
>> or even like this,
>>
>>                 $auth_ok = (defined $hash and $hash eq $1);
>>
>> may be easier to read, no?
>
> yes, let's go with the earlier; I was trying to mimic the original
> code, but agree on a second read that it looks confusing.
> assuming there are no more comments, would you want a reroll?

If there are no more comments, I certainly can tweak it myself, but I'd rather not have to keep an eye to see if the thread gets any comment myself to begin with, as it will not scale as the process when there are multiple contributors and only one maintainer.

So, I'll make a local tweak myself and mark the topic as "On hold", to let you ping me after waiting for a while to see if the thread gets other comments that require a reroll.

Thanks.