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

[PATCH 02/10] unpack_loose_header(): simplify next_out assignment

From
Jeff King <peff@peff.net>
Date
Feb 25, 2025, 06:29 UTC
Message-ID
<20250225062900.GB1293961@coredump.intra.peff.net>
In-Reply-To
<20250225062518.GA1293854@coredump.intra.peff.net>

When using OBJECT_INFO_ALLOW_UNKNOWN_TYPE to unpack a header that doesn't fit into our initial 32-byte buffer, we loop over calls git_inflate(), feeding it our buffer to the "next_out" pointer each time. As the code is written, we reset next_out after each inflate call (and after reading the output), ready for the next loop.

This isn't wrong, but there are a few advantages to setting up "next_out" right before each inflate call, rather than after:

  1. It drops a few duplicated lines of code.
  2. It makes it obvious that we always feed a fresh buffer on each call
     (and thus can never see Z_BUF_ERROR due to due to a lack of output
     space).
  3. After we exit the loop, we'll leave stream->next_out pointing to
     the end of the fetched data (this is how zlib callers find out how
     much data is in the buffer). This doesn't matter in practice, since
     nobody looks at it again. But it's probably the least-surprising
     thing to do, as it matches how next_out is left when the whole
     thing fits in the initial 32-byte buffer (and we don't enter the
     loop at all).
Signed-off-by: Jeff King <peff@peff.net>
---
Not strictly necessary, but I think it makes some of the reasoning in
patch 4 easier.
 object-file.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/object-file.c b/object-file.c
index 45b251ba04..0fd42981fb 100644
--- a/object-file.c
+++ b/object-file.c
@@ -1385,18 +1385,17 @@ enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
 	 * reading the stream.
 	 */
 	strbuf_add(header, buffer, stream->next_out - (unsigned char *)buffer);
-	stream->next_out = buffer;
-	stream->avail_out = bufsiz;
 
 	do {
+		stream->next_out = buffer;
+		stream->avail_out = bufsiz;
+
 		obj_read_unlock();
 		status = git_inflate(stream, 0);
 		obj_read_lock();
 		strbuf_add(header, buffer, stream->next_out - (unsigned char *)buffer);
 		if (memchr(buffer, '\0', stream->next_out - (unsigned char *)buffer))
 			return 0;
-		stream->next_out = buffer;
-		stream->avail_out = bufsiz;
 	} while (status != Z_STREAM_END);
 	return ULHR_TOO_LONG;
 }
-- 
2.48.1.709.gf47ae731ff
Previous: Taylor BlauNext: Taylor Blau
Message 9 of 34 in “some zlib inflating bug fixes”
  1. 0/10 some zlib inflating bug fixesJeff King, Feb 25, 2025
  2. 01/10 loose_object_info(): BUG() on inflating content with unknown typeJeff King, Feb 25, 2025
  3. Patrick SteinhardtFeb 25, 2025
  4. Junio C HamanoFeb 26, 2025
  5. Taylor BlauFeb 28, 2025
  6. Jeff KingMar 4, 2025
  7. Junio C HamanoMar 4, 2025
  8. Taylor BlauFeb 28, 2025
  9. 02/10 unpack_loose_header(): simplify next_out assignmentJeff King, Feb 25, 2025
  10. Taylor BlauFeb 28, 2025
  11. 03/10 unpack_loose_header(): report headers without NUL as "bad"Jeff King, Feb 25, 2025
  12. 04/10 unpack_loose_header(): fix infinite loop on broken zlib inputJeff King, Feb 25, 2025
  13. Patrick SteinhardtFeb 25, 2025
  14. Eric SunshineFeb 25, 2025
  15. Junio C HamanoFeb 26, 2025
  16. Taylor BlauFeb 28, 2025
  17. 05/10 git_inflate(): skip zlib_post_call() sanity check on Z_NEED_DICTJeff King, Feb 25, 2025
  18. Junio C HamanoFeb 26, 2025
  19. Taylor BlauFeb 28, 2025
  20. Jeff KingMar 4, 2025
  21. 06/10 unpack_loose_header(): avoid numeric comparison of zlib statusJeff King, Feb 25, 2025
  22. Taylor BlauFeb 28, 2025
  23. Jeff KingMar 4, 2025
  24. 07/10 unpack_loose_rest(): avoid numeric comparison of zlib statusJeff King, Feb 25, 2025
  25. 08/10 unpack_loose_rest(): never clean up zstreamJeff King, Feb 25, 2025
  26. Junio C HamanoFeb 26, 2025
  27. 09/10 unpack_loose_rest(): simplify error handlingJeff King, Feb 25, 2025
  28. Junio C HamanoFeb 26, 2025
  29. Taylor BlauFeb 28, 2025
  30. 10/10 unpack_loose_rest(): rewrite return handling for clarityJeff King, Feb 25, 2025
  31. Taylor BlauFeb 28, 2025
  32. Jeff KingMar 4, 2025
  33. Taylor BlauMar 4, 2025
  34. Taylor BlauFeb 28, 2025

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.