Re: [BUGREPORT] git diff-tree --cc SEGFAUTs
- From
Wink Saville <wink@saville.com>
- Date
- Jan 5, 2025, 22:13 UTC
- Message-ID
- <CAKk8isoPDcHJXm6HL1x4knNATWsy9mhPXTN5-P-rgFyUfZruDw@mail.gmail.com>
- In-Reply-To
- <CAKk8isrRCZsrt=2YB+L3EjK3ZAYbSk0e+1YZexqZUhB78L36dg@mail.gmail.com>
I'd like to suggest one minor tweak to Peff's change. Rather than just changing the type of combine_diff_parent::path to `char *` I like to suggest changing the name and adding a comment. My inclination is to use `changed_path` as the field name.
The resulting diff against next is attached.
diff --git a/combine-diff.c b/combine-diff.c index 641bc92dbd..be5df18d75 100644 --- a/combine-diff.c +++ b/combine-diff.c @@ -66,13 +66,9 @@ static struct combine_diff_path *intersect_paths( oidcpy(&p->parent[n].oid, &q->queue[i]->one->oid); p->parent[n].mode = q->queue[i]->one->mode; p->parent[n].status = q->queue[i]->status; - - 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].changed_path = combined_all_paths && + filename_changed(p->parent[n].status) ? + xstrdup(q->queue[i]->one->path) : NULL; *tail = p; tail = &p->next; } @@ -92,9 +88,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].changed_path); free(p); continue; } @@ -108,10 +102,9 @@ static struct combine_diff_path *intersect_paths( oidcpy(&p->parent[n].oid, &q->queue[i]->one->oid); p->parent[n].mode = q->queue[i]->one->mode; 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].changed_path = combined_all_paths && + filename_changed(p->parent[n].status) ? + xstrdup(q->queue[i]->one->path) : NULL; tail = &p->next; i++; @@ -996,8 +989,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].changed_path ? + elem->parent[i].changed_path : + elem->path; if (elem->parent[i].status == DIFF_STATUS_ADDED) dump_quoted_path("--- ", "", "/dev/null", line_prefix, c_meta, c_reset); @@ -1278,12 +1272,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].changed_path ? + p->parent[i].changed_path : + p->path; + write_name_quoted(path, stdout, inter_name_termination); } write_name_quoted(p->path, stdout, line_termination); } @@ -1645,9 +1637,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].changed_path); free(tmp); } diff --git a/diff-lib.c b/diff-lib.c index c6d3bc4d37..602ae0c84b 100644 --- a/diff-lib.c +++ b/diff-lib.c @@ -417,9 +417,11 @@ static int show_modified(struct rev_info *revs, memset(p->parent, 0, 2 * sizeof(struct combine_diff_parent)); p->parent[0].status = DIFF_STATUS_MODIFIED; p->parent[0].mode = new_entry->ce_mode; + p->parent[0].changed_path = NULL; oidcpy(&p->parent[0].oid, &new_entry->oid); p->parent[1].status = DIFF_STATUS_MODIFIED; p->parent[1].mode = old_entry->ce_mode; + p->parent[1].changed_path = NULL; oidcpy(&p->parent[1].oid, &old_entry->oid); show_combined_diff(p, 2, revs); free(p); diff --git a/diff.h b/diff.h index 6e6007c17b..d13be142dd 100644 --- 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 *changed_path; // NULL unless status == 'R' or 'C', see filename_changed() } parent[FLEX_ARRAY]; }; #define combine_diff_path_size(n, l) \ diff --git a/tree-diff.c b/tree-diff.c index d9237ffd9b..85f1d2a4a6 100644 --- a/tree-diff.c +++ b/tree-diff.c @@ -272,6 +272,7 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p, } p->parent[i].mode = mode_i; + p->parent[i].changed_path = NULL; oidcpy(&p->parent[i].oid, oid_i); }