Re: [PATCH] apply: keep buffer/size pair in sync when parsing binary hunks
From: Felipe Contreras <hidden>
Date: 2021-08-12 20:56:22
Subsystem:
the rest · Maintainer:
Linus Torvalds
Carlo Arenas wrote:
On Mon, Aug 9, 2021 at 9:24 PM Jeff King [off-list ref] wrote:quoted
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