global/git
1
0
Fork 0
mirror of https://github.com/git/git.git synced 2026-01-09 17:46:37 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'jk/combine-diff-cleanup'

Browse Source 
Code clean-up for code paths around combined diff.

* jk/combine-diff-cleanup:
  tree-diff: make list tail-passing more explicit
  tree-diff: simplify emit_path() list management
  tree-diff: use the name "tail" to refer to list tail
  tree-diff: drop list-tail argument to diff_tree_paths()
  combine-diff: drop public declaration of combine_diff_path_size()
  tree-diff: inline path_appendnew()
  tree-diff: pass whole path string to path_appendnew()
  tree-diff: drop path_appendnew() alloc optimization
  run_diff_files(): de-mystify the size of combine_diff_path struct
  diff: add a comment about combine_diff_path.parent.path
  combine-diff: use pointer for parent paths
  tree-diff: clear parent array in path_appendnew()
  combine-diff: add combine_diff_path_new()
  run_diff_files(): delay allocation of combine_diff_path
This commit is contained in:
Junio C Hamano
2025-02-03 10:23:33 -08:00
parent caf17423d3 6979bf6f8f
commit c43136d67b
4 changed files with 101 additions and 183 deletions
Download Patch File Download Diff File Expand all files Collapse all files

80
combine-diff.c
View File

@@ -47,31 +47,20 @@ static struct combine_diff_path *intersect_paths(
if (!n) {
for (i = 0; i < q->nr; i++) {
int len;
const char *path;
if (diff_unmodified_pair(q->queue[i]))
continue;
path = q->queue[i]->two->path;
len = strlen(path);
p = xmalloc(combine_diff_path_size(num_parent, len));
p->path = (char *) &(p->parent[num_parent]);
memcpy(p->path, path, len);
p->path[len] = 0;
p->next = NULL;
memset(p->parent, 0,
sizeof(p->parent[0]) * num_parent);
oidcpy(&p->oid, &q->queue[i]->two->oid);
p->mode = q->queue[i]->two->mode;
p = combine_diff_path_new(q->queue[i]->two->path,
strlen(q->queue[i]->two->path),
q->queue[i]->two->mode,
&q->queue[i]->two->oid,
num_parent);
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].path = xstrdup(q->queue[i]->one->path);
}
*tail = p;
tail = &p->next;
@@ -92,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;
}
@@ -110,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++;
@@ -996,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);
@@ -1278,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);
}
@@ -1443,22 +1428,19 @@ static struct combine_diff_path *find_paths_multitree(
{
int i, nparent = parents->nr;
const struct object_id **parents_oid;
struct combine_diff_path paths_head;
struct combine_diff_path *paths;
struct strbuf base;
ALLOC_ARRAY(parents_oid, nparent);
for (i = 0; i < nparent; i++)
parents_oid[i] = &parents->oid[i];
/* fake list head, so worker can assume it is non-NULL */
paths_head.next = NULL;
strbuf_init(&base, PATH_MAX);
diff_tree_paths(&paths_head, oid, parents_oid, nparent, &base, opt);
paths = diff_tree_paths(oid, parents_oid, nparent, &base, opt);
strbuf_release(&base);
free(parents_oid);
return paths_head.next;
return paths;
}
static int match_objfind(struct combine_diff_path *path,
@@ -1645,9 +1627,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);
}
@@ -1667,3 +1647,25 @@ void diff_tree_combined_merge(const struct commit *commit,
diff_tree_combined(&commit->object.oid, &parents, rev);
oid_array_clear(&parents);
}
struct combine_diff_path *combine_diff_path_new(const char *path,
size_t path_len,
unsigned int mode,
const struct object_id *oid,
size_t num_parents)
{
struct combine_diff_path *p;
size_t parent_len = st_mult(sizeof(p->parent[0]), num_parents);
p = xmalloc(st_add4(sizeof(*p), path_len, 1, parent_len));
p->path = (char *)&(p->parent[num_parents]);
memcpy(p->path, path, path_len);
p->path[path_len] = 0;
p->next = NULL;
p->mode = mode;
oidcpy(&p->oid, oid);
memset(p->parent, 0, parent_len);
return p;
}

34
diff-lib.c
View File

@@ -153,21 +153,8 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
struct diff_filepair *pair;
unsigned int wt_mode = 0;
int num_compare_stages = 0;
size_t path_len;
struct stat st;
path_len = ce_namelen(ce);
dpath = xmalloc(combine_diff_path_size(5, path_len));
dpath->path = (char *) &(dpath->parent[5]);
dpath->next = NULL;
memcpy(dpath->path, ce->name, path_len);
dpath->path[path_len] = '\0';
oidclr(&dpath->oid, the_repository->hash_algo);
memset(&(dpath->parent[0]), 0,
sizeof(struct combine_diff_parent)*5);
changed = check_removed(ce, &st);
if (!changed)
wt_mode = ce_mode_from_stat(ce, st.st_mode);
@@ -178,7 +165,14 @@ void run_diff_files(struct rev_info *revs, unsigned int option)
}
wt_mode = 0;
}
dpath->mode = wt_mode;
/*
* Allocate space for two parents, which will come from
* index stages #2 and #3, if present. Below we'll fill
* these from (stage - 2).
*/
dpath = combine_diff_path_new(ce->name, ce_namelen(ce),
wt_mode, null_oid(), 2);
while (i < entries) {
struct cache_entry *nce = istate->cache[i];
@@ -405,16 +399,10 @@ static int show_modified(struct rev_info *revs,
if (revs->combine_merges && !cached &&
(!oideq(oid, &old_entry->oid) || !oideq(&old_entry->oid, &new_entry->oid))) {
struct combine_diff_path *p;
int pathlen = ce_namelen(new_entry);
p = xmalloc(combine_diff_path_size(2, pathlen));
p->path = (char *) &p->parent[2];
p->next = NULL;
memcpy(p->path, new_entry->name, pathlen);
p->path[pathlen] = 0;
p->mode = mode;
oidclr(&p->oid, the_repository->hash_algo);
memset(p->parent, 0, 2 * sizeof(struct combine_diff_parent));
p = combine_diff_path_new(new_entry->name,
ce_namelen(new_entry),
mode, null_oid(), 2);
p->parent[0].status = DIFF_STATUS_MODIFIED;
p->parent[0].mode = new_entry->ce_mode;
oidcpy(&p->parent[0].oid, &new_entry->oid);

18
diff.h
View File

@@ -462,7 +462,7 @@ const char *diff_line_prefix(struct diff_options *);
extern const char mime_boundary_leader[];
struct combine_diff_path *diff_tree_paths(
struct combine_diff_path *p, const struct object_id *oid,
const struct object_id *oid,
const struct object_id **parents_oid, int nparent,
struct strbuf *base, struct diff_options *opt);
void diff_tree_oid(const struct object_id *old_oid,
@@ -480,12 +480,20 @@ struct combine_diff_path {
char status;
unsigned int mode;
struct object_id oid;
struct strbuf path;
/*
* This per-parent path is filled only when doing a combined
* diff with revs.combined_all_paths set, and only if the path
* differs from the post-image (e.g., a rename or copy).
* Otherwise it is left NULL.
*/
char *path;
} parent[FLEX_ARRAY];
};
#define combine_diff_path_size(n, l) \
st_add4(sizeof(struct combine_diff_path), (l), 1, \
st_mult(sizeof(struct combine_diff_parent), (n)))
struct combine_diff_path *combine_diff_path_new(const char *path,
size_t path_len,
unsigned int mode,
const struct object_id *oid,
size_t num_parents);
void show_combined_diff(struct combine_diff_path *elem, int num_parent,
struct rev_info *);

152
tree-diff.c
View File

@@ -48,8 +48,8 @@
free((x)); \
} while(0)
static struct combine_diff_path *ll_diff_tree_paths(
struct combine_diff_path *p, const struct object_id *oid,
static void ll_diff_tree_paths(
struct combine_diff_path ***tail, const struct object_id *oid,
const struct object_id **parents_oid, int nparent,
struct strbuf *base, struct diff_options *opt,
int depth);
@@ -124,72 +124,6 @@ static int emit_diff_first_parent_only(struct diff_options *opt, struct combine_
}
/*
* Make a new combine_diff_path from path/mode/sha1
* and append it to paths list tail.
*
* Memory for created elements could be reused:
*
* - if last->next == NULL, the memory is allocated;
*
* - if last->next != NULL, it is assumed that p=last->next was returned
* earlier by this function, and p->next was *not* modified.
* The memory is then reused from p.
*
* so for clients,
*
* - if you do need to keep the element
*
* p = path_appendnew(p, ...);
* process(p);
* p->next = NULL;
*
* - if you don't need to keep the element after processing
*
* pprev = p;
* p = path_appendnew(p, ...);
* process(p);
* p = pprev;
* ; don't forget to free tail->next in the end
*
* p->parent[] remains uninitialized.
*/
static struct combine_diff_path *path_appendnew(struct combine_diff_path *last,
int nparent, const struct strbuf *base, const char *path, int pathlen,
unsigned mode, const struct object_id *oid)
{
struct combine_diff_path *p;
size_t len = st_add(base->len, pathlen);
size_t alloclen = combine_diff_path_size(nparent, len);
/* if last->next is !NULL - it is a pre-allocated memory, we can reuse */
p = last->next;
if (p && (alloclen > (intptr_t)p->next)) {
FREE_AND_NULL(p);
}
if (!p) {
p = xmalloc(alloclen);
/*
* until we go to it next round, .next holds how many bytes we
* allocated (for faster realloc - we don't need copying old data).
*/
p->next = (struct combine_diff_path *)(intptr_t)alloclen;
}
last->next = p;
p->path = (char *)&(p->parent[nparent]);
memcpy(p->path, base->buf, base->len);
memcpy(p->path + base->len, path, pathlen);
p->path[len] = 0;
p->mode = mode;
oidcpy(&p->oid, oid ? oid : null_oid());
return p;
}
/*
* new path should be added to combine diff
*
@@ -200,10 +134,10 @@ static struct combine_diff_path *path_appendnew(struct combine_diff_path *last,
* t, tp -> path modified/added
* (M for tp[i]=tp[imin], A otherwise)
*/
static struct combine_diff_path *emit_path(struct combine_diff_path *p,
struct strbuf *base, struct diff_options *opt, int nparent,
struct tree_desc *t, struct tree_desc *tp,
int imin, int depth)
static void emit_path(struct combine_diff_path ***tail,
struct strbuf *base, struct diff_options *opt,
int nparent, struct tree_desc *t, struct tree_desc *tp,
int imin, int depth)
{
unsigned short mode;
const char *path;
@@ -243,8 +177,13 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
if (emitthis) {
int keep;
struct combine_diff_path *pprev = p;
p = path_appendnew(p, nparent, base, path, pathlen, mode, oid);
struct combine_diff_path *p;
strbuf_add(base, path, pathlen);
p = combine_diff_path_new(base->buf, base->len, mode,
oid ? oid : null_oid(),
nparent);
strbuf_setlen(base, old_baselen);
for (i = 0; i < nparent; ++i) {
/*
@@ -279,21 +218,12 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
if (opt->pathchange)
keep = opt->pathchange(opt, p);
/*
* If a path was filtered or consumed - we don't need to add it
* to the list and can reuse its memory, leaving it as
* pre-allocated element on the tail.
*
* On the other hand, if path needs to be kept, we need to
* correct its .next to NULL, as it was pre-initialized to how
* much memory was allocated.
*
* see path_appendnew() for details.
*/
if (!keep)
p = pprev;
else
p->next = NULL;
if (keep) {
**tail = p;
*tail = &p->next;
} else {
free(p);
}
}
if (recurse) {
@@ -309,13 +239,12 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
strbuf_add(base, path, pathlen);
strbuf_addch(base, '/');
p = ll_diff_tree_paths(p, oid, parents_oid, nparent, base, opt,
depth + 1);
ll_diff_tree_paths(tail, oid, parents_oid, nparent, base, opt,
depth + 1);
FAST_ARRAY_FREE(parents_oid, nparent);
}
strbuf_setlen(base, old_baselen);
return p;
}
static void skip_uninteresting(struct tree_desc *t, struct strbuf *base,
@@ -428,8 +357,8 @@ static inline void update_tp_entries(struct tree_desc *tp, int nparent)
update_tree_entry(&tp[i]);
}
static struct combine_diff_path *ll_diff_tree_paths(
struct combine_diff_path *p, const struct object_id *oid,
static void ll_diff_tree_paths(
struct combine_diff_path ***tail, const struct object_id *oid,
const struct object_id **parents_oid, int nparent,
struct strbuf *base, struct diff_options *opt,
int depth)
@@ -533,8 +462,8 @@ static struct combine_diff_path *ll_diff_tree_paths(
}
/* D += {δ(t,pi) if pi=p[imin]; "+a" if pi > p[imin]} */
p = emit_path(p, base, opt, nparent,
&t, tp, imin, depth);
emit_path(tail, base, opt, nparent,
&t, tp, imin, depth);
skip_emit_t_tp:
/* t↓, ∀ pi=p[imin] pi↓ */
@@ -545,8 +474,8 @@ static struct combine_diff_path *ll_diff_tree_paths(
/* t < p[imin] */
else if (cmp < 0) {
/* D += "+t" */
p = emit_path(p, base, opt, nparent,
&t, /*tp=*/NULL, -1, depth);
emit_path(tail, base, opt, nparent,
&t, /*tp=*/NULL, -1, depth);
/* t↓ */
update_tree_entry(&t);
@@ -561,8 +490,8 @@ static struct combine_diff_path *ll_diff_tree_paths(
goto skip_emit_tp;
}
p = emit_path(p, base, opt, nparent,
/*t=*/NULL, tp, imin, depth);
emit_path(tail, base, opt, nparent,
/*t=*/NULL, tp, imin, depth);
skip_emit_tp:
/* ∀ pi=p[imin] pi↓ */
@@ -575,24 +504,16 @@ static struct combine_diff_path *ll_diff_tree_paths(
free(tptree[i]);
FAST_ARRAY_FREE(tptree, nparent);
FAST_ARRAY_FREE(tp, nparent);
return p;
}
struct combine_diff_path *diff_tree_paths(
struct combine_diff_path *p, const struct object_id *oid,
const struct object_id *oid,
const struct object_id **parents_oid, int nparent,
struct strbuf *base, struct diff_options *opt)
{
p = ll_diff_tree_paths(p, oid, parents_oid, nparent, base, opt, 0);
/*
* free pre-allocated last element, if any
* (see path_appendnew() for details about why)
*/
FREE_AND_NULL(p->next);
return p;
struct combine_diff_path *head = NULL, **tail = &head;
ll_diff_tree_paths(&tail, oid, parents_oid, nparent, base, opt, 0);
return head;
}
/*
@@ -708,14 +629,13 @@ static void ll_diff_tree_oid(const struct object_id *old_oid,
const struct object_id *new_oid,
struct strbuf *base, struct diff_options *opt)
{
struct combine_diff_path phead, *p;
struct combine_diff_path *paths, *p;
pathchange_fn_t pathchange_old = opt->pathchange;
phead.next = NULL;
opt->pathchange = emit_diff_first_parent_only;
diff_tree_paths(&phead, new_oid, &old_oid, 1, base, opt);
paths = diff_tree_paths(new_oid, &old_oid, 1, base, opt);
for (p = phead.next; p;) {
for (p = paths; p;) {
struct combine_diff_path *pprev = p;
p = p->next;
free(pprev);