On Wed, Apr 29, 2009 at 09:12:18AM -0400, Bill Pemberton wrote:
> Returning undef is rarely the correct way to return a failure.
> Replace it with return 0
No, it's the right way to return failure here. The function returns
either an error string, prefixed with the problematic line number, or
undef. So 'undef' is working as a sentinel value here, not as part of a
boolean.
That being said, the _calling_ code is a bit sloppy in checking "$error"
instead of "defined($error)". It is not an actual bug because the
beginning of the string is always a line number >= 1, so it always
triggers as desired. However, it would probably be more clear to write
it like this:
---
diff --git a/git-send-email.perl b/git-send-email.perl
index cccbf45..168b2c2 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -495,7 +495,7 @@ if ($validate) {
foreach my $f (@files) {
unless (-p $f) {
my $error = validate_patch($f);
- $error and die "fatal: $f: $error\nwarning: no patches were sent\n";
+ defined($error) and die "fatal: $f: $error\nwarning: no patches were sent\n";
}
}
}
--
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