Skip to content

Commit 3a05997

Browse files
peffgitster
authored andcommitted
combine-diff: use pointer for parent paths
Commit d76ce4f (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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 5173099 commit 3a05997

File tree

2 files changed

+12
-20
lines changed

2 files changed

+12
-20
lines changed

combine-diff.c

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,7 @@ static struct combine_diff_path *intersect_paths(
6060

6161
if (combined_all_paths &&
6262
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);
6664
}
6765
*tail = p;
6866
tail = &p->next;
@@ -83,9 +81,7 @@ static struct combine_diff_path *intersect_paths(
8381
/* p->path not in q->queue[]; drop it */
8482
*tail = p->next;
8583
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);
8985
free(p);
9086
continue;
9187
}
@@ -101,8 +97,7 @@ static struct combine_diff_path *intersect_paths(
10197
p->parent[n].status = q->queue[i]->status;
10298
if (combined_all_paths &&
10399
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);
106101

107102
tail = &p->next;
108103
i++;
@@ -987,8 +982,9 @@ static void show_combined_header(struct combine_diff_path *elem,
987982

988983
if (rev->combined_all_paths) {
989984
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;
992988
if (elem->parent[i].status == DIFF_STATUS_ADDED)
993989
dump_quoted_path("--- ", "", "/dev/null",
994990
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
12691265

12701266
for (i = 0; i < num_parent; i++)
12711267
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);
12781272
}
12791273
write_name_quoted(p->path, stdout, line_termination);
12801274
}
@@ -1636,9 +1630,7 @@ void diff_tree_combined(const struct object_id *oid,
16361630
struct combine_diff_path *tmp = paths;
16371631
paths = paths->next;
16381632
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);
16421634
free(tmp);
16431635
}
16441636

diff.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ struct combine_diff_path {
480480
char status;
481481
unsigned int mode;
482482
struct object_id oid;
483-
struct strbuf path;
483+
char *path;
484484
} parent[FLEX_ARRAY];
485485
};
486486
#define combine_diff_path_size(n, l) \

0 commit comments

Comments
 (0)