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

[PATCH v2 4/6] apply: refactor code to drop `line_allocated`

From
Patrick Steinhardt <ps@pks.im>
Date
Sep 17, 2024, 10:08 UTC
Message-ID
<6ac37186f284f9a4fcc4c9e25a1eaf102317f38a.1726567217.git.ps@pks.im>
In-Reply-To
<cover.1726567217.git.ps@pks.im>

The `struct image` has two members `line` and `line_allocated`. The former member is the one that should be used throughout the code, whereas the latter one is used to track whether the lines have been allocated or not.

In practice, the array of lines is always allocated. The reason why we have `line_allocated` is that `remove_first_line()` will advance the array pointer to drop the first entry, and thus it points into the array instead of to the array header.

Refactor the function to use memmove(3P) instead, which allows us to get rid of this double bookkeeping. This is less efficient, but I doubt that this matters much in practice. If this judgement call is found to be wrong at a later point in time we can likely refactor the surrounding loop such that we first calculate the number of leading context lines to remove and then remove them in a single call to memmove(3P).

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 apply.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/apply.c b/apply.c
index 76f7777d4c..985564ac76 100644
--- a/apply.c
+++ b/apply.c
@@ -281,7 +281,6 @@ struct image {
 	size_t len;
 	size_t nr;
 	size_t alloc;
-	struct line *line_allocated;
 	struct line *line;
 };
 #define IMAGE_INIT { 0 }
@@ -295,7 +294,7 @@ static void image_init(struct image *image)
 static void image_clear(struct image *image)
 {
 	free(image->buf);
-	free(image->line_allocated);
+	free(image->line);
 	image_init(image);
 }
 
@@ -313,10 +312,10 @@ static uint32_t hash_line(const char *cp, size_t len)
 
 static void image_add_line(struct image *img, const char *bol, size_t len, unsigned flag)
 {
-	ALLOC_GROW(img->line_allocated, img->nr + 1, img->alloc);
-	img->line_allocated[img->nr].len = len;
-	img->line_allocated[img->nr].hash = hash_line(bol, len);
-	img->line_allocated[img->nr].flag = flag;
+	ALLOC_GROW(img->line, img->nr + 1, img->alloc);
+	img->line[img->nr].len = len;
+	img->line[img->nr].hash = hash_line(bol, len);
+	img->line[img->nr].flag = flag;
 	img->nr++;
 }
 
@@ -348,15 +347,15 @@ static void image_prepare(struct image *image, char *buf, size_t len,
 		image_add_line(image, cp, next - cp, 0);
 		cp = next;
 	}
-	image->line = image->line_allocated;
 }
 
 static void image_remove_first_line(struct image *img)
 {
 	img->buf += img->line[0].len;
 	img->len -= img->line[0].len;
-	img->line++;
 	img->nr--;
+	if (img->nr)
+		MOVE_ARRAY(img->line, img->line + 1, img->nr);
 }
 
 static void image_remove_last_line(struct image *img)
@@ -2335,7 +2334,7 @@ static void update_pre_post_images(struct image *preimage,
 	       : fixed_preimage.nr <= preimage->nr);
 	for (i = 0; i < fixed_preimage.nr; i++)
 		fixed_preimage.line[i].flag = preimage->line[i].flag;
-	free(preimage->line_allocated);
+	free(preimage->line);
 	*preimage = fixed_preimage;
 
 	/*
@@ -2879,14 +2878,12 @@ static void update_image(struct apply_state *state,
 
 	/* Adjust the line table */
 	nr = img->nr + postimage->nr - preimage_limit;
-	if (preimage_limit < postimage->nr) {
+	if (preimage_limit < postimage->nr)
 		/*
 		 * NOTE: this knows that we never call image_remove_first_line()
 		 * on anything other than pre/post image.
 		 */
 		REALLOC_ARRAY(img->line, nr);
-		img->line_allocated = img->line;
-	}
 	if (preimage_limit != postimage->nr)
 		MOVE_ARRAY(img->line + applied_pos + postimage->nr,
 			   img->line + applied_pos + preimage_limit,
@@ -3027,8 +3024,8 @@ static int apply_one_fragment(struct apply_state *state,
 	    newlines.len > 0 && newlines.buf[newlines.len - 1] == '\n') {
 		old--;
 		strbuf_setlen(&newlines, newlines.len - 1);
-		preimage.line_allocated[preimage.nr - 1].len--;
-		postimage.line_allocated[postimage.nr - 1].len--;
+		preimage.line[preimage.nr - 1].len--;
+		postimage.line[postimage.nr - 1].len--;
 	}
 
 	leading = frag->leading;
@@ -3062,8 +3059,6 @@ static int apply_one_fragment(struct apply_state *state,
 	preimage.len = old - oldlines;
 	postimage.buf = newlines.buf;
 	postimage.len = newlines.len;
-	preimage.line = preimage.line_allocated;
-	postimage.line = postimage.line_allocated;
 
 	for (;;) {
 
@@ -3151,8 +3146,8 @@ static int apply_one_fragment(struct apply_state *state,
 out:
 	free(oldlines);
 	strbuf_release(&newlines);
-	free(preimage.line_allocated);
-	free(postimage.line_allocated);
+	free(preimage.line);
+	free(postimage.line);
 
 	return (applied_pos < 0);
 }
@@ -3752,7 +3747,7 @@ static int apply_data(struct apply_state *state, struct patch *patch,
 	patch->result = image.buf;
 	patch->resultsize = image.len;
 	add_to_fn_table(state, patch);
-	free(image.line_allocated);
+	free(image.line);
 
 	if (0 < patch->is_delete && patch->resultsize)
 		return error(_("removal patch leaves file contents"));
-- 
2.46.0.551.gc5ee8f2d1c.dirty
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 16 of 20 in “apply: fix leaking buffer of `struct image`”
  1. 0/6 apply: fix leaking buffer of `struct image`Patrick Steinhardt, Sep 16, 2024
  2. 1/6 apply: reorder functions to move image-related things togetherPatrick Steinhardt, Sep 16, 2024
  3. 2/6 apply: rename functions operating on `struct image`Patrick Steinhardt, Sep 16, 2024
  4. 3/6 apply: introduce macro and function to init imagesPatrick Steinhardt, Sep 16, 2024
  5. 4/6 apply: refactor code to drop `line_allocated`Patrick Steinhardt, Sep 16, 2024
  6. Junio C HamanoSep 16, 2024
  7. Patrick SteinhardtSep 17, 2024
  8. Junio C HamanoSep 16, 2024
  9. 5/6 apply: rename members that track line count and allocation lengthPatrick Steinhardt, Sep 16, 2024
  10. 6/6 apply: refactor `struct image` to use a `struct strbuf`Patrick Steinhardt, Sep 16, 2024
  11. Junio C HamanoSep 16, 2024
  12. 0/6 apply: fix leaking buffer of `struct image`Patrick Steinhardt, Sep 17, 2024
  13. 1/6 apply: reorder functions to move image-related things togetherPatrick Steinhardt, Sep 17, 2024
  14. 2/6 apply: rename functions operating on `struct image`Patrick Steinhardt, Sep 17, 2024
  15. 3/6 apply: introduce macro and function to init imagesPatrick Steinhardt, Sep 17, 2024
  16. 4/6 apply: refactor code to drop `line_allocated`Patrick Steinhardt, Sep 17, 2024
  17. 5/6 apply: rename members that track line count and allocation lengthPatrick Steinhardt, Sep 17, 2024
  18. 6/6 apply: refactor `struct image` to use a `struct strbuf`Patrick Steinhardt, Sep 17, 2024
  19. Patrick SteinhardtSep 17, 2024
  20. Junio C HamanoSep 17, 2024

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.