@matheusbsbs / matheus-bagliano / commits / 3a0599788f

combine-diff: use pointer for parent paths

Commit d76ce4f734 (log,diff-tree: add --combined-all-paths option, 2019-02-07) added a "path" field to each combine_diff_parent struct. It's defined as a strbuf, but this is overkill. We never manipulate the buffer beyond inserting a single string into it. And in fact there's a small bug: we zero the parent structs, including the path strbufs. For the 0th parent, we strbuf_init() the strbuf before adding to it. But for subsequent parents, we never do the init. This is technically violating the strbuf API, though the code there is resilient enough to handle this zero'd state. This patch switches us to just store an allocated string pointer. Zeroing it is enough to properly initialize it there (modulo the usual assumption we make that a NULL pointer is all-zeroes). And as a bonus, we can just check for a non-NULL value to see if it is present, rather than repeating the combined_all_paths logic at each site. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jan 9, 2025 at 03:42 UTC 3a0599788fd38822dcd2f32de538afdd36a478aa

2 files changed +12 -20

combine-diff.c

+11 -19

@@ -60,9 +60,7 @@ static struct combine_diff_path *intersect_paths(

61 if (combined_all_paths &&

62 filename_changed(p->parent[n].status)) {

63 - strbuf_init(&p->parent[n].path, 0);

64 - strbuf_addstr(&p->parent[n].path,

65 - q->queue[i]->one->path);

63 + p->parent[n].path = xstrdup(q->queue[i]->one->path);

64 }

65 *tail = p;

66 tail = &p->next;

@@ -83,9 +81,7 @@ static struct combine_diff_path *intersect_paths(

81 /* p->path not in q->queue[]; drop it */

82 *tail = p->next;

83 for (j = 0; j < num_parent; j++)

86 - if (combined_all_paths &&

87 - filename_changed(p->parent[j].status))

88 - strbuf_release(&p->parent[j].path);

84 + free(p->parent[j].path);

85 free(p);

86 continue;

87 }

@@ -101,8 +97,7 @@ static struct combine_diff_path *intersect_paths(

97 p->parent[n].status = q->queue[i]->status;

98 if (combined_all_paths &&

99 filename_changed(p->parent[n].status))

104 - strbuf_addstr(&p->parent[n].path,

105 - q->queue[i]->one->path);

100 + p->parent[n].path = xstrdup(q->queue[i]->one->path);

101

102 tail = &p->next;

103 i++;

@@ -987,8 +982,9 @@ static void show_combined_header(struct combine_diff_path *elem,

982

983 if (rev->combined_all_paths) {

984 for (i = 0; i < num_parent; i++) {

990 - char *path = filename_changed(elem->parent[i].status)

991 - ? elem->parent[i].path.buf : elem->path;

985 + const char *path = elem->parent[i].path ?

986 + elem->parent[i].path :

987 + elem->path;

988 if (elem->parent[i].status == DIFF_STATUS_ADDED)

989 dump_quoted_path("--- ", "", "/dev/null",

990 line_prefix, c_meta, c_reset);

@@ -1269,12 +1265,10 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re

1265

1266 for (i = 0; i < num_parent; i++)

1267 if (rev->combined_all_paths) {

1272 - if (filename_changed(p->parent[i].status))

1273 - write_name_quoted(p->parent[i].path.buf, stdout,

1274 - inter_name_termination);

1275 - else

1276 - write_name_quoted(p->path, stdout,

1277 - inter_name_termination);

1268 + const char *path = p->parent[i].path ?

1269 + p->parent[i].path :

1270 + p->path;

1271 + write_name_quoted(path, stdout, inter_name_termination);

1272 }

1273 write_name_quoted(p->path, stdout, line_termination);

1274 }

@@ -1636,9 +1630,7 @@ void diff_tree_combined(const struct object_id *oid,

1630 struct combine_diff_path *tmp = paths;

1631 paths = paths->next;

1632 for (i = 0; i < num_parent; i++)

1639 - if (rev->combined_all_paths &&

1640 - filename_changed(tmp->parent[i].status))

1641 - strbuf_release(&tmp->parent[i].path);

1633 + free(tmp->parent[i].path);

1634 free(tmp);

1635 }

1636

diff.h

+1 -1

@@ -480,7 +480,7 @@ struct combine_diff_path {

480 char status;

481 unsigned int mode;

482 struct object_id oid;

483 - struct strbuf path;

483 + char *path;

484 } parent[FLEX_ARRAY];

485 };

486 #define combine_diff_path_size(n, l) \