Skip to content

Commit b5581e6

Browse files
committed
Merge branch 'rr/send-email-perl-critique' into maint
* rr/send-email-perl-critique: send-email: use the three-arg form of open in recipients_cmd send-email: drop misleading function prototype send-email: use "return;" not "return undef;" on error codepaths
2 parents 6a29370 + a47eab0 commit b5581e6

File tree

1 file changed

+10
-8
lines changed

1 file changed

+10
-8
lines changed

git-send-email.perl

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -512,8 +512,9 @@ sub split_addrs {
512512

513513
($sender) = expand_aliases($sender) if defined $sender;
514514

515-
# returns 1 if the conflict must be solved using it as a format-patch argument
516-
sub check_file_rev_conflict($) {
515+
# is_format_patch_arg($f) returns 0 if $f names a patch, or 1 if
516+
# $f is a revision list specification to be passed to format-patch.
517+
sub is_format_patch_arg {
517518
return unless $repo;
518519
my $f = shift;
519520
try {
@@ -529,6 +530,7 @@ ($)
529530
* Giving --format-patch option if you mean a range.
530531
EOF
531532
} catch Git::Error::Command with {
533+
# Not a valid revision. Treat it as a filename.
532534
return 0;
533535
}
534536
}
@@ -540,14 +542,14 @@ ($)
540542
if ($f eq "--") {
541543
push @rev_list_opts, "--", @ARGV;
542544
@ARGV = ();
543-
} elsif (-d $f and !check_file_rev_conflict($f)) {
545+
} elsif (-d $f and !is_format_patch_arg($f)) {
544546
opendir my $dh, $f
545547
or die "Failed to opendir $f: $!";
546548

547549
push @files, grep { -f $_ } map { catfile($f, $_) }
548550
sort readdir $dh;
549551
closedir $dh;
550-
} elsif ((-f $f or -p $f) and !check_file_rev_conflict($f)) {
552+
} elsif ((-f $f or -p $f) and !is_format_patch_arg($f)) {
551553
push @files, $f;
552554
} else {
553555
push @rev_list_opts, $f;
@@ -711,7 +713,7 @@ sub ask {
711713
}
712714
}
713715
}
714-
return undef;
716+
return;
715717
}
716718

717719
my %broken_encoding;
@@ -833,7 +835,7 @@ sub extract_valid_address {
833835
# less robust/correct than the monster regexp in Email::Valid,
834836
# but still does a 99% job, and one less dependency
835837
return $1 if $address =~ /($local_part_regexp\@$domain_regexp)/;
836-
return undef;
838+
return;
837839
}
838840

839841
sub extract_valid_address_or_die {
@@ -1438,7 +1440,7 @@ sub recipients_cmd {
14381440

14391441
my $sanitized_sender = sanitize_address($sender);
14401442
my @addresses = ();
1441-
open my $fh, "$cmd \Q$file\E |"
1443+
open my $fh, "-|", "$cmd \Q$file\E"
14421444
or die "($prefix) Could not execute '$cmd'";
14431445
while (my $address = <$fh>) {
14441446
$address =~ s/^\s*//g;
@@ -1484,7 +1486,7 @@ sub validate_patch {
14841486
return "$.: patch contains a line longer than 998 characters";
14851487
}
14861488
}
1487-
return undef;
1489+
return;
14881490
}
14891491

14901492
sub file_has_nonascii {

0 commit comments

Comments
 (0)