Re: What's cooking in git.git (Oct 2015, #01; Mon, 5)
- From
Mike Rappazzo <rappazzo@gmail.com>
- Date
- Oct 6, 2015, 19:30 UTC
- Message-ID
- <CANoM8SX6U0fdoUtemiaoxYErAKx0MfEf+N9UR7aDip2g=QA8+Q@mail.gmail.com>
- In-Reply-To
- <xmqqd1wswnfe.fsf@gitster.mtv.corp.google.com>
On Tue, Oct 6, 2015 at 1:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 64 quoted lines
> 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
>Thanks for clearing this up for me. I will add it to my branch an re-roll in a day or two.