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>

diff --git a/combine-diff.c b/combine-diff.c
index 45548fd438321fe10aa13c70a7be3b987232086f..ae3cbfc699ccc070b14e702820f0a6a34671e35d 100644 (file)
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -60,9 +60,7 @@ static struct combine_diff_path *intersect_paths(
 
                        if (combined_all_paths &&
                            filename_changed(p->parent[n].status)) {
-                               strbuf_init(&p->parent[n].path, 0);
-                               strbuf_addstr(&p->parent[n].path,
-                                             q->queue[i]->one->path);
+                               p->parent[n].path = xstrdup(q->queue[i]->one->path);
                        }
                        *tail = p;
                        tail = &p->next;
@@ -83,9 +81,7 @@ static struct combine_diff_path *intersect_paths(
                        /* p->path not in q->queue[]; drop it */
                        *tail = p->next;
                        for (j = 0; j < num_parent; j++)
-                               if (combined_all_paths &&
-                                   filename_changed(p->parent[j].status))
-                                       strbuf_release(&p->parent[j].path);
+                               free(p->parent[j].path);
                        free(p);
                        continue;
                }
@@ -101,8 +97,7 @@ static struct combine_diff_path *intersect_paths(
                p->parent[n].status = q->queue[i]->status;
                if (combined_all_paths &&
                    filename_changed(p->parent[n].status))
-                       strbuf_addstr(&p->parent[n].path,
-                                     q->queue[i]->one->path);
+                       p->parent[n].path = xstrdup(q->queue[i]->one->path);
 
                tail = &p->next;
                i++;
@@ -987,8 +982,9 @@ static void show_combined_header(struct combine_diff_path *elem,
 
        if (rev->combined_all_paths) {
                for (i = 0; i < num_parent; i++) {
-                       char *path = filename_changed(elem->parent[i].status)
-                               ? elem->parent[i].path.buf : elem->path;
+                       const char *path = elem->parent[i].path ?
+                                          elem->parent[i].path :
+                                          elem->path;
                        if (elem->parent[i].status == DIFF_STATUS_ADDED)
                                dump_quoted_path("--- ", "", "/dev/null",
                                                 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
 
        for (i = 0; i < num_parent; i++)
                if (rev->combined_all_paths) {
-                       if (filename_changed(p->parent[i].status))
-                               write_name_quoted(p->parent[i].path.buf, stdout,
-                                                 inter_name_termination);
-                       else
-                               write_name_quoted(p->path, stdout,
-                                                 inter_name_termination);
+                       const char *path = p->parent[i].path ?
+                                          p->parent[i].path :
+                                          p->path;
+                       write_name_quoted(path, stdout, inter_name_termination);
                }
        write_name_quoted(p->path, stdout, line_termination);
 }
@@ -1636,9 +1630,7 @@ void diff_tree_combined(const struct object_id *oid,
                struct combine_diff_path *tmp = paths;
                paths = paths->next;
                for (i = 0; i < num_parent; i++)
-                       if (rev->combined_all_paths &&
-                           filename_changed(tmp->parent[i].status))
-                               strbuf_release(&tmp->parent[i].path);
+                       free(tmp->parent[i].path);
                free(tmp);
        }
 

diff --git a/diff.h b/diff.h
index 5cddd5a8706e4073adb338a4cb480c9d09d02bb9..f5f6ea00fb3770457984f3badefd497363706bfb 100644 (file)
--- a/diff.h
+++ b/diff.h
@@ -480,7 +480,7 @@ struct combine_diff_path {
                char status;
                unsigned int mode;
                struct object_id oid;
-               struct strbuf path;
+               char *path;
        } parent[FLEX_ARRAY];
 };
 #define combine_diff_path_size(n, l) \