git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Carlo Arenas <carenas@gmail.com>
Cc: git@vger.kernel.org, sam@vilain.net, avarab@gmail.com
Subject: Re: [PATCH 2/3] git-cvsserver: protect against NULL in crypt(3)
Date: Thu, 16 Sep 2021 20:43:31 -0700	[thread overview]
Message-ID: <xmqqsfy350kc.fsf@gitster.g> (raw)
In-Reply-To: <CAPUEspjqD5zy8TLuFA96usU7FYi=0wF84y7NgOVFqegtxL9zbw@mail.gmail.com> (Carlo Arenas's message of "Thu, 16 Sep 2021 15:44:10 -0700")

Carlo Arenas <carenas@gmail.com> writes:

>> 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.

  reply	other threads:[~2021-09-17  3:43 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-15  8:09 [PATCH 0/3] cvsserver: correctly validate pserver passwords Carlo Marcelo Arenas Belón
2021-09-15  8:09 ` [PATCH 1/3] git-cvsserver: use crypt correctly to compare password hashes Carlo Marcelo Arenas Belón
2021-09-15  8:09 ` [PATCH 2/3] git-cvsserver: protect against NULL in crypt(3) Carlo Marcelo Arenas Belón
2021-09-16 22:11   ` Junio C Hamano
2021-09-16 22:44     ` Carlo Arenas
2021-09-17  3:43       ` Junio C Hamano [this message]
2021-09-15  8:09 ` [PATCH 3/3] Documentation: cleanup git-cvsserver Carlo Marcelo Arenas Belón

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqsfy350kc.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=carenas@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=sam@vilain.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).