diff mbox series

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

Message ID 20250225062900.GB1293961@coredump.intra.peff.net (mailing list archive)
State New
Headers show
Series some zlib inflating bug fixes | expand

Commit Message

Jeff King Feb. 25, 2025, 6:29 a.m. UTC
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 mbox series

Patch

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;
 }