Re: [PATCH 05/12] sequencer.c: use commit-slab to associate todo items to commits
- Date: Sat, 12 May 2018 05:28:10 -0400
- From: Jeff King <peff@xxxxxxxx>
- Subject: Re: [PATCH 05/12] sequencer.c: use commit-slab to associate todo items to commits
On Sat, May 12, 2018 at 10:00:21AM +0200, Nguyễn Thái Ngọc Duy wrote:
> @@ -3446,9 +3451,9 @@ int rearrange_squash(void)
> else if (!strchr(p, ' ') &&
> (commit2 =
> lookup_commit_reference_by_name(p)) &&
> - commit2->util)
> + *commit_todo_item_at(&commit_todo, commit2))
> /* found by commit name */
> - i2 = (struct todo_item *)commit2->util
> + i2 = *commit_todo_item_peek(&commit_todo, commit2)
> - todo_list.items;
Directly dereferencing _peek() is an anti-pattern, since it may return
NULL. We know it's OK here because the earlier call to _at() would have
extended the slab. But it probably makes sense to use _at() in both
places then (or even to use _peek() for the earlier one if you want to
avoid extending, but as I said in the other message, I kind of
suspect most of these cases end up allocating slab space for most of the
commits anyway).
-Peff