Re: [PATCH 5/7] submodule: implement `module_list` as a builtin helper

From: Stefan Beller <sbeller@google.com>
Date: Aug 19, 2015, 18:23 UTC
Message-ID: <CAGZ79kbHVAqOFT5s-4RRtHjfBO=KgvMOmup0Q1OyNZ3kSFfvxw@mail.gmail.com>
In-Reply-To: <xmqqpp2j9m7g.fsf@gitster.dls.corp.google.com>

On Wed, Aug 19, 2015 at 11:17 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 25 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> +static int module_list_compute(int argc, const char **argv,
>> +                             const char *prefix,
>> +                             struct pathspec *pathspec)
>> +{
>> +     int i;
>> +     char *max_prefix, *ps_matched = NULL;
>> +     int max_prefix_len;
>> +     parse_pathspec(pathspec, 0,
>> +                    PATHSPEC_PREFER_FULL |
>> +                    PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
>> +                    prefix, argv);
>> +
>> +     /* Find common prefix for all pathspec's */
>> +     max_prefix = common_prefix(pathspec);
>> +     max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
>> +
>> +     if (pathspec->nr)
>> +             ps_matched = xcalloc(1, pathspec->nr);
>
> Micronit.  Even though multiplication is commutative, the order of
> arguments to xcalloc() looks odd.  It lets you say "I want an array
> with nmemb elements, and each of its is size-bytes long" by giving
> it nmemb and then size.

Thanks for pointing that out! I think this would be an alignment issue in theory, if the items were larger and the number of items not dividable by the size of items.

So because 1 char has no alignment problems, we can happily allocate just one block of the member size here.

>
> No need to resend; will fix it up while queuing.
>
> Thanks.