merge-file: clamp exit code to maximum 127
Git-merge-file is documented to return one of three exit codes: - zero means the merge was successful - a negative number means an error occurred - a positive number indicates the number of conflicts Unfortunately, this all gets stuffed into an 8-bit return code. Which means that if you have 256 conflicts, this wraps to zero, and the merge appears to succeed (and commits a blob full of conflict-marker cruft!). This patch clamps the return value to a maximum of 127, which we should be able to safely represent everywhere. This also leaves 128-255 for other values. Shells (and some parts of git) will typically represent signal death as 128 plus the signal number. And negative values are typically coerced to an 8-bit unsigned value (so "return -1" ends up as 255). Technically negative returns have the same problem (e.g., "-256" wraps back to 0), but this is not a problem in practice, as the only negative value we use is "-1". Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
		
				
					committed by
					
						
						Junio C Hamano
					
				
			
			
				
	
			
			
			
						parent
						
							282616c72d
						
					
				
				
					commit
					e34f80278e
				
			@ -41,7 +41,8 @@ lines from `<other-file>`, or lines from both respectively.  The length of the
 | 
			
		||||
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 (truncated to 127 if there are more than that many
 | 
			
		||||
conflicts). 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
 | 
			
		||||
 | 
			
		||||
@ -102,5 +102,8 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 | 
			
		||||
		free(result.ptr);
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	if (ret > 127)
 | 
			
		||||
		ret = 127;
 | 
			
		||||
 | 
			
		||||
	return ret;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
@ -692,4 +692,37 @@ test_expect_success GPG 'merge --no-edit tag should skip editor' '
 | 
			
		||||
	test_cmp actual expect
 | 
			
		||||
'
 | 
			
		||||
 | 
			
		||||
test_expect_success 'set up mod-256 conflict scenario' '
 | 
			
		||||
	# 256 near-identical stanzas...
 | 
			
		||||
	for i in $(test_seq 1 256); do
 | 
			
		||||
		for j in 1 2 3 4 5; do
 | 
			
		||||
			echo $i-$j
 | 
			
		||||
		done
 | 
			
		||||
	done >file &&
 | 
			
		||||
	git add file &&
 | 
			
		||||
	git commit -m base &&
 | 
			
		||||
 | 
			
		||||
	# one side changes the first line of each to "master"
 | 
			
		||||
	sed s/-1/-master/ <file >tmp &&
 | 
			
		||||
	mv tmp file &&
 | 
			
		||||
	git commit -am master &&
 | 
			
		||||
 | 
			
		||||
	# and the other to "side"; merging the two will
 | 
			
		||||
	# yield 256 separate conflicts
 | 
			
		||||
	git checkout -b side HEAD^ &&
 | 
			
		||||
	sed s/-1/-side/ <file >tmp &&
 | 
			
		||||
	mv tmp file &&
 | 
			
		||||
	git commit -am side
 | 
			
		||||
'
 | 
			
		||||
 | 
			
		||||
test_expect_success 'merge detects mod-256 conflicts (recursive)' '
 | 
			
		||||
	git reset --hard &&
 | 
			
		||||
	test_must_fail git merge -s recursive master
 | 
			
		||||
'
 | 
			
		||||
 | 
			
		||||
test_expect_success 'merge detects mod-256 conflicts (resolve)' '
 | 
			
		||||
	git reset --hard &&
 | 
			
		||||
	test_must_fail git merge -s resolve master
 | 
			
		||||
'
 | 
			
		||||
 | 
			
		||||
test_done
 | 
			
		||||
 | 
			
		||||
		Reference in New Issue
	
	Block a user