Re: [PATCH] apply: keep buffer/size pair in sync when parsing binary hunks

[Felipe Contreras <felipe.contreras@gmail.com>]

Carlo Arenas wrote:
> On Mon, Aug 9, 2021 at 9:24 PM Jeff King <peff@peff.net> wrote:
> > diff --git a/apply.c b/apply.c
> > index 44bc31d6eb..4ed4b27169 100644
> > --- a/apply.c
> > +++ b/apply.c
> > @@ -1917,6 +1917,7 @@ static struct fragment *parse_binary_hunk(struct apply_state *state,
> >
> >         state->linenr++;
> >         buffer += llen;
> > +       size -= llen;
> >         while (1) {
> 
> Ironically, I was looking at this code because of your previous
> patch[1] that you suggested was ugly
> and because I was going to suggest moving from a for to a while loop
> to avoid the overly long line.
> 
> It is interesting to note though, that having a for (and obviously
> removing the last 2 lines from the loop) with a comma separated
> increment instead would
> have made this issue IMHO more obvious, and also why I decided against
> that; would it be a good idea to fix that as well?

What's the point in updating size when it can be calculated? Just add a
pointer to the end of the buffer, and then size is the difference
between the buffer which is moving, and the end pointer which is fixed:

-- 8< --

diff --git a/apply.c b/apply.c
index 44bc31d6eb..cfb5a00356 100644
--- a/apply.c
+++ b/apply.c
@@ -1891,15 +1891,15 @@ static struct fragment *parse_binary_hunk(struct apply_state *state,
 	 * to 1-26 bytes, and 'a'-'z' corresponds to 27-52 bytes.
 	 */
 	int llen, used;
-	unsigned long size = *sz_p;
 	char *buffer = *buf_p;
+	char *end = *buf_p + *sz_p;
 	int patch_method;
 	unsigned long origlen;
 	char *data = NULL;
 	int hunk_size = 0;
 	struct fragment *frag;
 
-	llen = linelen(buffer, size);
+	llen = linelen(buffer, end - buffer);
 	used = llen;
 
 	*status_p = 0;
@@ -1919,13 +1919,12 @@ static struct fragment *parse_binary_hunk(struct apply_state *state,
 	buffer += llen;
 	while (1) {
 		int byte_length, max_byte_length, newsize;
-		llen = linelen(buffer, size);
+		llen = linelen(buffer, end - buffer);
 		used += llen;
 		state->linenr++;
 		if (llen == 1) {
 			/* consume the blank line */
 			buffer++;
-			size--;
 			break;
 		}
 		/*
@@ -1955,7 +1954,6 @@ static struct fragment *parse_binary_hunk(struct apply_state *state,
 			goto corrupt;
 		hunk_size = newsize;
 		buffer += llen;
-		size -= llen;
 	}
 
 	CALLOC_ARRAY(frag, 1);
@@ -1966,7 +1964,7 @@ static struct fragment *parse_binary_hunk(struct apply_state *state,
 	free(data);
 	frag->size = origlen;
 	*buf_p = buffer;
-	*sz_p = size;
+	*sz_p = end - buffer;
 	*used_p = used;
 	frag->binary_patch_method = patch_method;
 	return frag;

-- 
Felipe Contreras