Re: What's cooking in git.git (Oct 2015, #01; Mon, 5)

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 6, 2015, 05:55 UTC
Message-ID: <xmqqd1wswnfe.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <561339C1.704@web.de>

Torsten Bögershausen <tboegi@web.de> writes:

> Minor comment from my compiler:
>
> worktree.c:181: warning: assuming signed overflow does not occur when assuming
> that (X + c) > X is always true

Thanks; I think the allocation in that function (the use it uses ALLOC_GROW()) is somewhat bogus.

It does this:

	if ((linked = get_linked_worktree(d->d_name))) {
		ALLOC_GROW(list, alloc + 1, alloc);
		list[counter++] = linked;
	}

where "alloc" keeps track of the size of the list, and "counter" keeps track of the first unused entry. The second argument to the helper macro smells bad.

The correct way to use ALLOC_GROW() helper macro is:

 * Use three variables, an array, the size of the allocation and the
   size of the used part of the array.  If you call the array $thing,
   the size of the allocation is typically called $thing_alloc and
   the size of the used part $thing_nr.  E.g. opts[], opts_alloc & opts_nr.

 * When adding a new thing at the end of the $thing, do this:

	ALLOC_GROW($thing, $thing_nr + 1, $thing_alloc);
        $thing[$thing_nr++] = <<new thing>>;

Perhaps something like this needs squashing in.

Subject: [PATCH] fixup! worktree: add a function to get worktree details

---
 worktree.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/worktree.c b/worktree.c
index 90282d9..f7304a2 100644
--- a/worktree.c
+++ b/worktree.c
@@ -178,12 +178,13 @@ struct worktree **get_worktrees(void)
 				continue;
 
 				if ((linked = get_linked_worktree(d->d_name))) {
-					ALLOC_GROW(list, alloc + 1, alloc);
+					ALLOC_GROW(list, counter + 1, alloc);
 					list[counter++] = linked;
 				}
 		}
 		closedir(dir);
 	}
+	ALLOC_GROW(list, counter + 1, alloc);
 	list[counter] = NULL;
 	return list;
 }

-- 
2.6.1-284-g1f14bb6