Re: [PATCH V2] git-send-email.perl: Add --to-cmd
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Sep 23, 2010, 23:16 UTC
- Message-ID
- <AANLkTin2YvPxnbCXzWsugfaGvbUUcm6n5LtwkNVxhJfC@mail.gmail.com>
- In-Reply-To
- <7v62xwqe7i.fsf@alter.siamese.dyndns.org>
On Thu, Sep 23, 2010 at 22:37, Junio C Hamano <gitster@pobox.com> wrote:
Show 27 quoted lines
> Joe Perches <joe@perches.com> writes:
>
>> + if (defined $to_cmd) {
>> + open(F, "$to_cmd \Q$t\E |")
>> + or die "(to-cmd) Could not execute '$to_cmd'";
>> + while(<F>) {
>> + my $t = $_;
>
> "my $t" masks another $t in the outer scope; technically not a bug, but
> questionable as a style.
>
>> + $t =~ s/^\s*//g;
>> + $t =~ s/\n$//g;
>> + next if ($t eq $sender and $suppress_from);
>> + push @to, parse_address_line($t)
>> + if defined $t; # sanitized/validated later
>
> This "if defined $t" makes my head hurt. Why?
>
> * The "while (<F>)" loop wouldn't have given you an undef in $t in the
> first place;
>
> * You would have got "Use of uninitialized value" warning at these two
> s/// statements if $t were undef; and
>
> * Even if $t were undef, these two s/// statements would have made $t a
> defined, empty string.Well spotted. Also it *can't* be undef here by definition. Since $t is taken from the $_ value which comes from the <> operator. That just wraps readline(), which will exit the loop when it hit EOF (at which point readline() *would* return undef).
So this whole business of checking for the definedness of $t doesn't make any sense.