Web lists-archives.com

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