Re: [PATCH v4 05/12] ref-filter: introduce parsing functions for each valid atom

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Feb 7, 2016, 06:33 UTC
Message-ID: <CAPig+cSt9Dub88ywP8mc8dPq6pXFvn4OTSJmEWbAiTeirRB7xA@mail.gmail.com>
In-Reply-To: <CAOLa=ZQxbCYd_bpf4PSpRVvejOgi=farNPtHgP_mZZypOf6cnQ@mail.gmail.com>

On Sat, Feb 6, 2016 at 10:15 AM, Karthik Nayak <karthik.188@gmail.com> wrote:

Show 24 quoted lines

> On Sun, Jan 31, 2016 at 11:12 PM, Karthik Nayak <karthik.188@gmail.com> wrote:
>> @@ -138,10 +140,9 @@ int parse_ref_filter_atom(const char *atom, const char *ep)
>>                  * shouldn't be used for checking against the valid_atom
>>                  * table.
>>                  */
>> -               const char *formatp = strchr(sp, ':');
>> -               if (!formatp || ep < formatp)
>> -                       formatp = ep;
>> -               if (len == formatp - sp && !memcmp(valid_atom[i].name, sp, len))
>> +               arg = memchr(sp, ':', ep - sp);
>> +               if ((!arg || len == arg - sp) &&
>> +                   !memcmp(valid_atom[i].name, sp, len))
>>                         break;
>>         }
>
> Also having a look at this, this breaks the previous error checking we
> had at parse_ref_filter_atom().
> e.g: git for-each-ref --format="%(refnameboo)" would not throw an error.
>
> I think the code needs to be changed to:
>
> -               if ((!arg || len == arg - sp) &&
> +               if ((arg || len == ep - sp) &&
> +                   (!arg || len == arg - sp) &&

For completeness, for people reading the mailing list archive, a couple alternate fixes were presented elsewhere[1], with a personal bias toward:

    arg = memchr(...);
    if (!arg)
        arg = ep;
    if (len == arg - sp && !memcmp(...))
        ...

[1]: http://git.661346.n2.nabble.com/PATCH-ref-filter-c-don-t-stomp-on-memory-tp7647432p7647433.html