On Thu, Sep 23, 2010 at 22:37, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Joe Perches <joe@xxxxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html