git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 2/7] diff-files: avoid negative exit value

From
Jeff King <peff@peff.net>
Date
Aug 21, 2023, 20:15 UTC
Message-ID
<20230821201510.GB1798590@coredump.intra.peff.net>
In-Reply-To
<20230821201358.GA2663749@coredump.intra.peff.net>

If loading the index fails, we print an error and then return "-1" from the function. But since this is a builtin, we end up with exit(-1), which produces odd results since program exit codes are unsigned. Because of integer conversion, it usually becomes 255, which is at least still an error, but values above 128 are usually interpreted as signal death.

Since we know the program is exiting immediately, we can just replace the error return with a die().

Signed-off-by: Jeff King <peff@peff.net>
---
I'm actually not sure this can be triggered in practice; see patch 4 for
more discussion. Regardless, it seems like a strict improvement.
 builtin/diff-files.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/builtin/diff-files.c b/builtin/diff-files.c
index 50330b8dd2..2e3e948583 100644
--- a/builtin/diff-files.c
+++ b/builtin/diff-files.c
@@ -80,14 +80,10 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
 	    (rev.diffopt.output_format & DIFF_FORMAT_PATCH))
 		diff_merges_set_dense_combined_if_unset(&rev);
 
-	if (repo_read_index_preload(the_repository, &rev.diffopt.pathspec, 0) < 0) {
-		perror("repo_read_index_preload");
-		result = -1;
-		goto cleanup;
-	}
+	if (repo_read_index_preload(the_repository, &rev.diffopt.pathspec, 0) < 0)
+		die_errno("repo_read_index_preload");
 	result = run_diff_files(&rev, options);
 	result = diff_result_code(&rev.diffopt, result);
-cleanup:
 	release_revisions(&rev);
 	return result;
 }
-- 
2.42.0.rc2.423.g967ecb4f2b
Previous: Jeff KingNext: Jeff King
Message 11 of 19 in “"git diff --no-pager --exit-code" errors out but returns zero exit code”
  1. Romain ChossartAug 20, 2023
  2. diff: handle negative status in diff_result_code()Jeff King, Aug 21, 2023
  3. Junio C HamanoAug 21, 2023
  4. diff: spell DIFF_INDEX_CACHED out when calling run_diff_index()Junio C Hamano, Aug 21, 2023
  5. Jeff KingAug 21, 2023
  6. Junio C HamanoAug 21, 2023
  7. Jeff KingAug 21, 2023
  8. Junio C HamanoAug 21, 2023
  9. 0/7 cleaning up diff_result_code()Jeff King, Aug 21, 2023
  10. 1/7 diff: spell DIFF_INDEX_CACHED out when calling run_diff_index()Jeff King, Aug 21, 2023
  11. 2/7 diff-files: avoid negative exit valueJeff King, Aug 21, 2023
  12. 3/7 diff: show usage for unknown builtin_diff_files() optionsJeff King, Aug 21, 2023
  13. 4/7 diff: die when failing to read index in git-diff builtinJeff King, Aug 21, 2023
  14. Junio C HamanoAug 22, 2023
  15. 5/7 diff: drop useless return from run_diff_{files,index} functionsJeff King, Aug 21, 2023
  16. 6/7 diff: drop useless return values in git-diff helpersJeff King, Aug 21, 2023
  17. 7/7 diff: drop useless "status" parameter from diff_result_code()Jeff King, Aug 21, 2023
  18. Junio C HamanoAug 22, 2023
  19. Jeff KingAug 23, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.