Skip to content

Commit d8baf08

Browse files
peffgitster
authored andcommitted
tree-diff: use the name "tail" to refer to list tail
The ll_diff_tree_paths() function and its helpers all append to a running list by taking in a pointer to the old tail and returning the new tail. But they just call this argument "p", which is not very descriptive. It gets particularly confusing in emit_path(), where we actually add to the list, because "p" does double-duty: it is the tail of the list, but it is also the entry which we add. Except that in some cases we _don't_ add a new entry (or we might even add it and roll it back) if the path isn't interesting. At first glance, this makes it look like a bug that we pass "p" on to ll_diff_tree_paths() to recurse; sometimes it is getting the new entry we made and sometimes not! But it's not a bug, because ll_diff_tree_paths() does not care about the entry itself at all. It is only using its "next" pointer as the tail of the list. Let's swap out "p" for "tail" to make this obvious. And then in emit_path() we'll continue to use "p" for our newly allocated entry. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent a5c4e31 commit d8baf08

File tree

1 file changed

+17
-16
lines changed

1 file changed

+17
-16
lines changed

tree-diff.c

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949
} while(0)
5050

5151
static struct combine_diff_path *ll_diff_tree_paths(
52-
struct combine_diff_path *p, const struct object_id *oid,
52+
struct combine_diff_path *tail, const struct object_id *oid,
5353
const struct object_id **parents_oid, int nparent,
5454
struct strbuf *base, struct diff_options *opt,
5555
int depth);
@@ -134,7 +134,7 @@ static int emit_diff_first_parent_only(struct diff_options *opt, struct combine_
134134
* t, tp -> path modified/added
135135
* (M for tp[i]=tp[imin], A otherwise)
136136
*/
137-
static struct combine_diff_path *emit_path(struct combine_diff_path *p,
137+
static struct combine_diff_path *emit_path(struct combine_diff_path *tail,
138138
struct strbuf *base, struct diff_options *opt, int nparent,
139139
struct tree_desc *t, struct tree_desc *tp,
140140
int imin, int depth)
@@ -177,13 +177,14 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
177177

178178
if (emitthis) {
179179
int keep;
180-
struct combine_diff_path *pprev = p;
180+
struct combine_diff_path *pprev = tail, *p;
181181

182182
strbuf_add(base, path, pathlen);
183183
p = combine_diff_path_new(base->buf, base->len, mode,
184184
oid ? oid : null_oid(),
185185
nparent);
186-
pprev->next = p;
186+
tail->next = p;
187+
tail = p;
187188
strbuf_setlen(base, old_baselen);
188189

189190
for (i = 0; i < nparent; ++i) {
@@ -222,7 +223,7 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
222223
if (!keep) {
223224
free(p);
224225
pprev->next = NULL;
225-
p = pprev;
226+
tail = pprev;
226227
}
227228
}
228229

@@ -239,13 +240,13 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
239240

240241
strbuf_add(base, path, pathlen);
241242
strbuf_addch(base, '/');
242-
p = ll_diff_tree_paths(p, oid, parents_oid, nparent, base, opt,
243-
depth + 1);
243+
tail = ll_diff_tree_paths(tail, oid, parents_oid, nparent, base, opt,
244+
depth + 1);
244245
FAST_ARRAY_FREE(parents_oid, nparent);
245246
}
246247

247248
strbuf_setlen(base, old_baselen);
248-
return p;
249+
return tail;
249250
}
250251

251252
static void skip_uninteresting(struct tree_desc *t, struct strbuf *base,
@@ -359,7 +360,7 @@ static inline void update_tp_entries(struct tree_desc *tp, int nparent)
359360
}
360361

361362
static struct combine_diff_path *ll_diff_tree_paths(
362-
struct combine_diff_path *p, const struct object_id *oid,
363+
struct combine_diff_path *tail, const struct object_id *oid,
363364
const struct object_id **parents_oid, int nparent,
364365
struct strbuf *base, struct diff_options *opt,
365366
int depth)
@@ -463,8 +464,8 @@ static struct combine_diff_path *ll_diff_tree_paths(
463464
}
464465

465466
/* D += {δ(t,pi) if pi=p[imin]; "+a" if pi > p[imin]} */
466-
p = emit_path(p, base, opt, nparent,
467-
&t, tp, imin, depth);
467+
tail = emit_path(tail, base, opt, nparent,
468+
&t, tp, imin, depth);
468469

469470
skip_emit_t_tp:
470471
/* t↓, ∀ pi=p[imin] pi↓ */
@@ -475,8 +476,8 @@ static struct combine_diff_path *ll_diff_tree_paths(
475476
/* t < p[imin] */
476477
else if (cmp < 0) {
477478
/* D += "+t" */
478-
p = emit_path(p, base, opt, nparent,
479-
&t, /*tp=*/NULL, -1, depth);
479+
tail = emit_path(tail, base, opt, nparent,
480+
&t, /*tp=*/NULL, -1, depth);
480481

481482
/* t↓ */
482483
update_tree_entry(&t);
@@ -491,8 +492,8 @@ static struct combine_diff_path *ll_diff_tree_paths(
491492
goto skip_emit_tp;
492493
}
493494

494-
p = emit_path(p, base, opt, nparent,
495-
/*t=*/NULL, tp, imin, depth);
495+
tail = emit_path(tail, base, opt, nparent,
496+
/*t=*/NULL, tp, imin, depth);
496497

497498
skip_emit_tp:
498499
/* ∀ pi=p[imin] pi↓ */
@@ -506,7 +507,7 @@ static struct combine_diff_path *ll_diff_tree_paths(
506507
FAST_ARRAY_FREE(tptree, nparent);
507508
FAST_ARRAY_FREE(tp, nparent);
508509

509-
return p;
510+
return tail;
510511
}
511512

512513
struct combine_diff_path *diff_tree_paths(

0 commit comments

Comments
 (0)