bug & patch: exit codes from internal commands are handled incorrectly

From: Kenneth Lorber <keni@his.com>
Date: Dec 16, 2014, 19:28 UTC
Message-ID: <13474FB1-5310-42E5-82A9-4047FEFEFF4A@his.com>

(Apologies if I’ve missed anything here - I’m still climbing the git learning curve.)

Bug: exit codes from (at least) internal commands are handled incorrectly.
E.g. git-merge-file, docs say:
        The exit value of this program is negative on error, and the number of
        conflicts otherwise. If the merge was clean, the exit value is 0.
But only 8 bits get carried through exit, so 256 conflicts gives exit(0), which means the merge was clean.

Issue shows up on:
 git version 1.8.5.2 (Apple Git-48)
 build from source (git version 2.2.0.68.g64396d6.dirty after patch below applied)

Reproduce by:

Put the following test program in an empty directory as buggen.pl

TEST PROGRAM START open OUTB, ">basefile" or die; print OUTB "this is the base file\n"; close OUTB;

open OUT1, ">bfile1" or die; open OUT2, ">bfile2" or die;

$c = shift;

while($c > 0){
        print OUT1 "a\nb\nc\nd\nchange $c file 1\n";
        print OUT2 "a\nb\nc\nd\nchange $c file 2\n";
        $c--;
}
TEST PROGRAM END

Do these tests:

perl buggen.pl 0 git merge-file -p bfile1 basefile bfile2 echo $status 0

perl buggen.pl 1 git merge-file -p bfile1 basefile bfile2 echo $status 1

perl buggen.pl 256 git merge-file -p bfile1 basefile bfile2 echo $status 0 <===OOPS

Proposed patches:

diff --git a/git.c b/git.c
index 82d7a1c..8228a3c 100644
--- a/git.c
+++ b/git.c
@@ -349,6 +349,8 @@ static int run_builtin(struct cmd_struct *p, int argc, const
        trace_argv_printf(argv, "trace: built-in: git");

        status = p->fn(argc, argv, prefix);
+       if (status > 255)
+               status = 255;   /* prevent exit() from truncating to 0 */
        if (status)
                return status;
diff --git a/Documentation/git-merge-file.txt b/Documentation/git-merge-file.txt
index d2fc12e..76e6a11 100644
--- a/Documentation/git-merge-file.txt
+++ b/Documentation/git-merge-file.txt
@@ -41,7 +41,8 @@ lines from `<other-file>`, or lines from both respectively.  T
 conflict markers can be given with the `--marker-size` option.

 The exit value of this program is negative on error, and the number of
-conflicts otherwise. If the merge was clean, the exit value is 0.
+conflicts otherwise (but 255 will be reported even if more than 255 conflicts
+exist). If the merge was clean, the exit value is 0.

 'git merge-file' is designed to be a minimal clone of RCS 'merge'; that is, it
 implements all of RCS 'merge''s functionality which is needed by

Open questions:
1) Is 255 safe for all operating systems?
2) Does this issue affect any other places?

Thanks,
Keni
keni@his.com