Junio C Hamano <gitster@xxxxxxxxx> writes:
>> +static int match_atom_arg_value(const char *to_parse, const char *candidate,
>> + const char **end, const char **valuestart,
>> + size_t *valuelen)
>> +{
>> + const char *atom;
>> +
>> + if (!(skip_prefix(to_parse, candidate, &atom)))
>> + return 0;
>> + if (valuestart) {
>
> As far as I saw, no callers pass NULL to valuestart. Getting rid of
> this if() statement and always entering its body would clarify what
> is going on, I think.
Specifically, ...
>> + if (*atom == '=') {
>> + *valuestart = atom + 1;
>> + *valuelen = strcspn(*valuestart, ",\0");
>> + atom = *valuestart + *valuelen;
>> + } else {
>> + if (*atom != ',' && *atom != '\0')
>> + return 0;
>> + *valuestart = NULL;
>> + *valuelen = 0;
>> + }
>> + }
>> + if (*atom == ',') {
>> + *end = atom + 1;
>> + return 1;
>> + }
>> + if (*atom == '\0') {
>> + *end = atom;
>> + return 1;
>> + }
>> + return 0;
>> +}
... I think the body of the function would become easier to read if
written like so:
if (!skip_prefix(to_parse, candidate, &atom))
return 0; /* definitely not "candidate" */
if (*atom == '=') {
/* we just saw "candidate=" */
*valuestart = atom + 1;
atom = strchrnul(*valuestart, ',');
*valuelen = atom - *valuestart;
} else if (*atom != ',' && *atom != '\0') {
/* key begins with "candidate" but has more chars */
return 0;
} else {
/* just "candidate" without "=val" */
*valuestart = NULL;
*valuelen = 0;
}
/* atom points at either the ',' or NUL after this key[=val] */
if (*atom == ',')
atom++;
else if (*atom)
BUG("should not happen");
*end = atom;
return 1;
as it is clear that *valuestart, *valuelen, and *end are not touched
when the function returns 0 and they are all filled when the function
returns 1.
Also, avoid passing ",\0" to strcspn(); its effect is exactly the
same as passing ",", and at that point you are better off using
strchnul().
Thanks.
