From: Stefan Beller <sbeller@google.com>
Date: Thu, 18 Feb 2016 23:27:13 GMT
Subject: Re: [PATCHv12 0/7] Expose submodule parallelism to the user
Message-ID: <CAGZ79kY7aYmDcz+JYwpzizfzRV3bUeN8TU-4jRjKqToJJTBHoA@mail.gmail.com>
In-Reply-To: <xmqqio1lehbp.fsf@gitster.mtv.corp.google.com>

On Thu, Feb 18, 2016 at 3:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> On Thu, Feb 18, 2016 at 3:12 PM, Stefan Beller <sbeller@google.com> wrote:
>>>> Unless you count "I want to write differently from what was
>>>> suggested" is a desirable thing to do, I do not see a point in
>>>> favouring the above that uses an extra variable and skip_prefix()
>>>> over what I gave you as "how about" patch.  But whatever.
>>>
>>> The skip_prefix was there before, so it stuck there.
>
> Sorry, but I thought this "parsing update strategy" was all new
> code.

I meant previous patches or in my mind. That's why I was hesitant to
throw out the skip_prefix.

>
>>> Also it seems a bit more high level to me hence easier to read,
>>> (though I am biased). I'll use your suggestion.
>>
>> and it doesn't crash when passing in value == NULL.
>> (We don't do that currently, just a side observation)
>
> Hmph.  If you pass str==NULL with prefix="!" to what we have below,
> I would think the first iteration would try to read from *str and do
> a bizarre thing.
>
> static inline int skip_prefix(const char *str, const char *prefix,
>                               const char **out)
> {
>         do {
>                 if (!*prefix) {
>                         *out = str;
>                         return 1;
>                 }
>         } while (*str++ == *prefix++);
>         return 0;
> }
>
> Puzzled.

And there I was asserting properties about methods
without looking them up.

ok.