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

[PATCH v2 4/7] diff: die when failing to read index in git-diff builtin

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

When the git-diff program fails to read the index in its diff-files or diff-index helper functions, it propagates the error up the stack. This eventually lands in diff_result_code(), which does not handle it well (as discussed in the previous patch).

Since the only sensible thing here is to exit with an error code (and what we were expecting the propagated error code to cause), let's just do that directly.

There's no test here, as I'm not even sure this case can be triggered. The index-reading functions tend to die() themselves when encountering any errors, and the return value is just the number of entries in the file (and so always 0 or positive). But let's err on the conservative side and keep checking the return value. It may be worth digging into as a separate topic (though index-reading is low-level enough that we probably want to eventually teach it to propagate errors anyway for lib-ification purposes, at which point this code would already be doing the right thing).

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/diff.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/builtin/diff.c b/builtin/diff.c
index d0e187ec18..005f415d36 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -163,12 +163,10 @@ static int builtin_diff_index(struct rev_info *revs,
 		setup_work_tree();
 		if (repo_read_index_preload(the_repository,
 					    &revs->diffopt.pathspec, 0) < 0) {
-			perror("repo_read_index_preload");
-			return -1;
+			die_errno("repo_read_index_preload");
 		}
 	} else if (repo_read_index(the_repository) < 0) {
-		perror("repo_read_cache");
-		return -1;
+		die_errno("repo_read_cache");
 	}
 	return run_diff_index(revs, option);
 }
@@ -289,8 +287,7 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv
 	setup_work_tree();
 	if (repo_read_index_preload(the_repository, &revs->diffopt.pathspec,
 				    0) < 0) {
-		perror("repo_read_index_preload");
-		return -1;
+		die_errno("repo_read_index_preload");
 	}
 	return run_diff_files(revs, options);
 }
-- 
2.42.0.rc2.423.g967ecb4f2b
Previous: Jeff KingNext: Junio C Hamano
Message 13 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.