Re: [PATCH 6/8] sequencer: simplify allocation of result array in todo_list_rearrange_squash()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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);



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux