@matheusbsbs / matheus-bagliano / commits / d8baf083c5

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 <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jeff King committed Jan 9, 2025 at 03:53 UTC d8baf083c5a1a3e8dd27fe73e6000c5f6dddf1ca

1 file changed +17 -16

tree-diff.c

+17 -16

@@ -49,7 +49,7 @@

49 } while(0)

51 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,

53 const struct object_id **parents_oid, int nparent,

54 struct strbuf *base, struct diff_options *opt,

55 int depth);

@@ -134,7 +134,7 @@ static int emit_diff_first_parent_only(struct diff_options *opt, struct combine_

134 * t, tp -> path modified/added

135 * (M for tp[i]=tp[imin], A otherwise)

136 */

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,

138 struct strbuf *base, struct diff_options *opt, int nparent,

139 struct tree_desc *t, struct tree_desc *tp,

140 int imin, int depth)

@@ -177,13 +177,14 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,

177

178 if (emitthis) {

179 int keep;

180 - struct combine_diff_path *pprev = p;

180 + struct combine_diff_path *pprev = tail, *p;

181

182 strbuf_add(base, path, pathlen);

183 p = combine_diff_path_new(base->buf, base->len, mode,

184 oid ? oid : null_oid(),

185 nparent);

186 - pprev->next = p;

186 + tail->next = p;

187 + tail = p;

188 strbuf_setlen(base, old_baselen);

189

190 for (i = 0; i < nparent; ++i) {

@@ -222,7 +223,7 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,

223 if (!keep) {

224 free(p);

225 pprev->next = NULL;

225 - p = pprev;

226 + tail = pprev;

227 }

228 }

229

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

240

241 strbuf_add(base, path, pathlen);

242 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);

245 FAST_ARRAY_FREE(parents_oid, nparent);

246 }

247

248 strbuf_setlen(base, old_baselen);

248 - return p;

249 + return tail;

250 }

251

252 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)

360 }

361

362 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,

364 const struct object_id **parents_oid, int nparent,

365 struct strbuf *base, struct diff_options *opt,

366 int depth)

@@ -463,8 +464,8 @@ static struct combine_diff_path *ll_diff_tree_paths(

464 }

465

466 /* 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);

469

470 skip_emit_t_tp:

471 /* t↓, ∀ pi=p[imin] pi↓ */

@@ -475,8 +476,8 @@ static struct combine_diff_path *ll_diff_tree_paths(

476 /* t < p[imin] */

477 else if (cmp < 0) {

478 /* 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);

481

482 /* t↓ */

483 update_tree_entry(&t);

@@ -491,8 +492,8 @@ static struct combine_diff_path *ll_diff_tree_paths(

492 goto skip_emit_tp;

493 }

494

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);

497

498 skip_emit_tp:

499 /* ∀ pi=p[imin] pi↓ */

@@ -506,7 +507,7 @@ static struct combine_diff_path *ll_diff_tree_paths(

507 FAST_ARRAY_FREE(tptree, nparent);

508 FAST_ARRAY_FREE(tp, nparent);

509

509 - return p;

510 + return tail;

511 }

512

513 struct combine_diff_path *diff_tree_paths(