Re: [PATCH 05/12] sequencer.c: use commit-slab to associate todo items to commits

From: Jeff King <peff@peff.net>
Date: May 12, 2018, 09:28 UTC
Message-ID: <20180512092810.GF28279@sigill.intra.peff.net>
In-Reply-To: <20180512080028.29611-6-pclouds@gmail.com>

On Sat, May 12, 2018 at 10:00:21AM +0200, Nguyễn Thái Ngọc Duy wrote:

Show 10 quoted lines

> @@ -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