On Tue, Oct 6, 2015 at 1:55 AM, Junio C Hamano <gits...@pobox.com> wrote:
> Torsten Bögershausen <tbo...@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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to