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

[PATCH v2 5/7] distinguish error versus short read from read_in_full()

From
Jeff King <peff@peff.net>
Date
Sep 27, 2017, 06:02 UTC
Message-ID
<20170927060210.euuaefuakciusudl@sigill.intra.peff.net>
In-Reply-To
<20170927055424.22ati3g24xaqtqrk@sigill.intra.peff.net>

Many callers of read_in_full() expect to see the exact number of bytes requested, but their error handling lumps together true read errors and short reads due to unexpected EOF.

We can give more specific error messages by separating these cases (showing errno when appropriate, and otherwise describing the short read).

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/get-tar-commit-id.c |  4 +++-
 bulk-checkin.c              |  5 ++++-
 packfile.c                  | 11 +++++++++--
 3 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/builtin/get-tar-commit-id.c b/builtin/get-tar-commit-id.c
index cd3e656828..2706fcfaf2 100644
--- a/builtin/get-tar-commit-id.c
+++ b/builtin/get-tar-commit-id.c
@@ -26,8 +26,10 @@ int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix)
 		usage(builtin_get_tar_commit_id_usage);
 
 	n = read_in_full(0, buffer, HEADERSIZE);
+	if (n < 0)
+		die_errno("git get-tar-commit-id: read error");
 	if (n != HEADERSIZE)
-		die("git get-tar-commit-id: read error");
+		die_errno("git get-tar-commit-id: EOF before reading tar header");
 	if (header->typeflag[0] != 'g')
 		return 1;
 	if (!skip_prefix(content, "52 comment=", &comment))
diff --git a/bulk-checkin.c b/bulk-checkin.c
index 9a1f6c49ab..3310fd210a 100644
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -115,7 +115,10 @@ static int stream_to_pack(struct bulk_checkin_state *state,
 
 		if (size && !s.avail_in) {
 			ssize_t rsize = size < sizeof(ibuf) ? size : sizeof(ibuf);
-			if (read_in_full(fd, ibuf, rsize) != rsize)
+			ssize_t read_result = read_in_full(fd, ibuf, rsize);
+			if (read_result < 0)
+				die_errno("failed to read from '%s'", path);
+			if (read_result != rsize)
 				die("failed to read %d bytes from '%s'",
 				    (int)rsize, path);
 			offset += rsize;
diff --git a/packfile.c b/packfile.c
index f69a5c8d60..eab7542487 100644
--- a/packfile.c
+++ b/packfile.c
@@ -442,6 +442,7 @@ static int open_packed_git_1(struct packed_git *p)
 	unsigned char sha1[20];
 	unsigned char *idx_sha1;
 	long fd_flag;
+	ssize_t read_result;
 
 	if (!p->index_data && open_pack_index(p))
 		return error("packfile %s index unavailable", p->pack_name);
@@ -483,7 +484,10 @@ static int open_packed_git_1(struct packed_git *p)
 		return error("cannot set FD_CLOEXEC");
 
 	/* Verify we recognize this pack file format. */
-	if (read_in_full(p->pack_fd, &hdr, sizeof(hdr)) != sizeof(hdr))
+	read_result = read_in_full(p->pack_fd, &hdr, sizeof(hdr));
+	if (read_result < 0)
+		return error_errno("error reading from %s", p->pack_name);
+	if (read_result != sizeof(hdr))
 		return error("file %s is far too short to be a packfile", p->pack_name);
 	if (hdr.hdr_signature != htonl(PACK_SIGNATURE))
 		return error("file %s is not a GIT packfile", p->pack_name);
@@ -500,7 +504,10 @@ static int open_packed_git_1(struct packed_git *p)
 			     p->num_objects);
 	if (lseek(p->pack_fd, p->pack_size - sizeof(sha1), SEEK_SET) == -1)
 		return error("end of packfile %s is unavailable", p->pack_name);
-	if (read_in_full(p->pack_fd, sha1, sizeof(sha1)) != sizeof(sha1))
+	read_result = read_in_full(p->pack_fd, sha1, sizeof(sha1));
+	if (read_result < 0)
+		return error_errno("error reading from %s", p->pack_name);
+	if (read_result != sizeof(sha1))
 		return error("packfile %s signature is unavailable", p->pack_name);
 	idx_sha1 = ((unsigned char *)p->index_data) + p->index_size - 40;
 	if (hashcmp(sha1, idx_sha1))
-- 
2.14.2.988.g01c8b37dde
Previous: Junio C HamanoNext: Jeff King
Message 48 of 51 in “read/write_in_full leftovers”
  1. 0/7 read/write_in_full leftoversJeff King, Sep 25, 2017
  2. 1/7 files-backend: prefer "0" for write_in_full() error checkJeff King, Sep 25, 2017
  3. Jonathan NiederSep 25, 2017
  4. Jeff KingSep 25, 2017
  5. 2/7 notes-merge: drop dead zero-write codeJeff King, Sep 25, 2017
  6. Jonathan NiederSep 25, 2017
  7. 3/7 read_in_full: reset errno before readingJeff King, Sep 25, 2017
  8. Jonathan NiederSep 25, 2017
  9. Jeff KingSep 25, 2017
  10. Jonathan NiederSep 25, 2017
  11. Jeff KingSep 25, 2017
  12. Jeff KingSep 25, 2017
  13. Jonathan NiederSep 25, 2017
  14. Jeff KingSep 26, 2017
  15. Stefan BellerSep 26, 2017
  16. Jeff KingSep 26, 2017
  17. Jonathan NiederSep 26, 2017
  18. Jonathan NiederSep 26, 2017
  19. Jeff KingSep 26, 2017
  20. Jonathan NiederSep 26, 2017
  21. Junio C HamanoSep 26, 2017
  22. Jeff KingSep 27, 2017
  23. Junio C HamanoSep 27, 2017
  24. Jonathan NiederSep 26, 2017
  25. Jeff KingSep 26, 2017
  26. Jonathan NiederSep 26, 2017
  27. Jeff KingSep 26, 2017
  28. Jonathan NiederSep 26, 2017
  29. Jonathan NiederSep 25, 2017
  30. 4/7 get-tar-commit-id: prefer "!=" for read_in_full() error checkJeff King, Sep 25, 2017
  31. Jonathan NiederSep 25, 2017
  32. 5/7 worktree: use xsize_t to access file sizeJeff King, Sep 25, 2017
  33. Jonathan NiederSep 25, 2017
  34. 6/7 worktree: check the result of read_in_full()Jeff King, Sep 25, 2017
  35. Jonathan NiederSep 25, 2017
  36. Jeff KingSep 25, 2017
  37. 7/7 add xread_in_full() helperJeff King, Sep 25, 2017
  38. Jonathan NiederSep 25, 2017
  39. Jeff KingSep 25, 2017
  40. 0/7 read/write_in_full leftoversJeff King, Sep 27, 2017
  41. 1/7 files-backend: prefer "0" for write_in_full() error checkJeff King, Sep 27, 2017
  42. 2/7 notes-merge: drop dead zero-write codeJeff King, Sep 27, 2017
  43. 3/7 prefer "!=" when checking read_in_full() resultJeff King, Sep 27, 2017
  44. Junio C HamanoSep 27, 2017
  45. Jeff KingSep 27, 2017
  46. 4/7 avoid looking at errno for short read_in_full() returnsJeff King, Sep 27, 2017
  47. Junio C HamanoSep 27, 2017
  48. 5/7 distinguish error versus short read from read_in_full()Jeff King, Sep 27, 2017
  49. 6/7 worktree: use xsize_t to access file sizeJeff King, Sep 27, 2017
  50. 7/7 worktree: check the result of read_in_full()Jeff King, Sep 27, 2017
  51. Junio C HamanoSep 27, 2017

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.