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

[Jeff King]

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(_todo, commit2))
>   /* found by commit name */
> - i2 = (struct todo_item *)commit2->util
> + i2 = *commit_todo_item_peek(_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

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

[Nguyễn Thái Ngọc Duy]

It's done so that commit->util can be removed. See more explanation in
the commit that removes commit->util.

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 sequencer.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/sequencer.c b/sequencer.c
index 5993ffe060..1a0a6916e3 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -3362,6 +3362,8 @@ static int subject2item_cmp(const void *fndata,
return key ? strcmp(a->subject, key) : strcmp(a->subject, b->subject);
 }
 
+define_commit_slab(commit_todo_item, struct todo_item *);
+
 /*
  * Rearrange the todo list that has both "pick commit-id msg" and "pick
  * commit-id fixup!/squash! msg" in it so that the latter is put immediately
@@ -3378,6 +3380,7 @@ int rearrange_squash(void)
struct hashmap subject2item;
int res = 0, rearranged = 0, *next, *tail, i;
char **subjects;
+   struct commit_todo_item commit_todo;
 
if (strbuf_read_file_or_whine(_list.buf, todo_file) < 0)
return -1;
@@ -3386,6 +3389,7 @@ int rearrange_squash(void)
return -1;
}
 
+   init_commit_todo_item(_todo);
/*
 * The hashmap maps onelines to the respective todo list index.
 *
@@ -3416,10 +3420,11 @@ int rearrange_squash(void)
 
if (is_fixup(item->command)) {
todo_list_release(_list);
+   clear_commit_todo_item(_todo);
return error(_("the script was already rearranged."));
}
 
-   item->commit->util = item;
+   *commit_todo_item_at(_todo, item->commit) = item;
 
parse_commit(item->commit);
commit_buffer = get_commit_buffer(item->commit, NULL);
@@ -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(_todo, commit2))
/* found by commit name */
-   i2 = (struct todo_item *)commit2->util
+   i2 = *commit_todo_item_peek(_todo, 
commit2)
- todo_list.items;
else {
/* copy can be a prefix of the commit subject */
@@ -3527,5 +3532,6 @@ int rearrange_squash(void)
hashmap_free(, 1);
todo_list_release(_list);
 
+   clear_commit_todo_item(_todo);
return res;
 }
-- 
2.17.0.705.g3525833791