Re: [RFC/PATCH v11 02/13] bisect: rewrite `check_term_format` shell function in C

[Pranit Bauva]

Hey Junio,

On Tue, Aug 2, 2016 at 11:01 PM, Junio C Hamano  wrote:
> Pranit Bauva  writes:
>
>> +/*
>> + * Check whether the string `term` belongs to the set of strings
>> + * included in the variable arguments.
>> + */
>> +static int one_of(const char *term, ...)
>> +{
>> + int res = 0;
>> + va_list matches;
>> + const char *match;
>> +
>> + va_start(matches, term);
>> + while (!res && (match = va_arg(matches, const char *)))
>> + res = !strcmp(term, match);
>> + va_end(matches);
>> +
>> + return res;
>> +}
>
> It might be safer to mark this function with LAST_ARG_MUST_BE_NULL,
> but because this is static to this function, it may not matter too
> much.  Just an observation, not a strong suggestion to change the
> patch.

Yes, I could do that.

>> +static int check_term_format(const char *term, const char *orig_term)
>> +{
>> + int res;
>> + char *new_term = xstrfmt("refs/bisect/%s", term);
>> +
>> + res = check_refname_format(new_term, 0);
>> + free(new_term);
>
> Yup, that looks much more straight-forward than using a one-time-use
> strbuf.

Thanks!

Regards,
Pranit Bauva
--
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

Re: [RFC/PATCH v11 02/13] bisect: rewrite `check_term_format` shell function in C

[Junio C Hamano]

Pranit Bauva  writes:

> +/*
> + * Check whether the string `term` belongs to the set of strings
> + * included in the variable arguments.
> + */
> +static int one_of(const char *term, ...)
> +{
> + int res = 0;
> + va_list matches;
> + const char *match;
> +
> + va_start(matches, term);
> + while (!res && (match = va_arg(matches, const char *)))
> + res = !strcmp(term, match);
> + va_end(matches);
> +
> + return res;
> +}

It might be safer to mark this function with LAST_ARG_MUST_BE_NULL,
but because this is static to this function, it may not matter too
much.  Just an observation, not a strong suggestion to change the
patch.

> +static int check_term_format(const char *term, const char *orig_term)
> +{
> + int res;
> + char *new_term = xstrfmt("refs/bisect/%s", term);
> +
> + res = check_refname_format(new_term, 0);
> + free(new_term);

Yup, that looks much more straight-forward than using a one-time-use
strbuf.
--
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