On Thu, Sep 16, 2021 at 3:11 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Carlo Marcelo Arenas Belón <carenas@xxxxxxxxx> writes: > > > - if (crypt(descramble($password), $1) eq $1) { > > - $auth_ok = 1; > > + my $hash = crypt(descramble($password), $1); > > + if (defined $hash) { > > + $auth_ok = 1 if $hash eq $1; > > } > > 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? Carlo