Hi Oswald
On 23/03/2023 16:22, Oswald Buddenhagen wrote:
The operation doesn't change the number of elements in the array, so we do
not need to allocate the result piecewise.
I think the reasoning behind this patch is sound.
Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@xxxxxx>
---
sequencer.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index f8a7f4e721..fb224445fa 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -6225,7 +6225,7 @@ static int skip_fixupish(const char *subject, const char **p) {
int todo_list_rearrange_squash(struct todo_list *todo_list)
{
struct hashmap subject2item;
- int rearranged = 0, *next, *tail, i, nr = 0, alloc = 0;
+ int rearranged = 0, *next, *tail, i, nr = 0;
char **subjects;
struct commit_todo_item commit_todo;
struct todo_item *items = NULL;
@@ -6334,6 +6334,8 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
}
if (rearranged) {
+ items = ALLOC_ARRAY(items, todo_list->nr);
+
for (i = 0; i < todo_list->nr; i++) {
enum todo_command command = todo_list->items[i].command;
int cur = i;
@@ -6346,16 +6348,15 @@ int todo_list_rearrange_squash(struct todo_list *todo_list)
continue;
while (cur >= 0) {
- ALLOC_GROW(items, nr + 1, alloc);
items[nr++] = todo_list->items[cur];
cur = next[cur];
}
}
+ assert(nr == todo_list->nr);
If this assert fails we may have already had some out of bounds memory
accesses.
+ todo_list->alloc = nr;
FREE_AND_NULL(todo_list->items);
I think it would be cleaner to keep the original ordering and free the
old list before assigning todo_list->alloc
todo_list->items = items;
- todo_list->nr = nr;
- todo_list->alloc = alloc;
}
Best Wishes
Phillip
free(next);
