Re: [PATCH 1/6] Remove return undef from validate_patch

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";
 		}
 	}
 }