On Wed, Jun 18, 2014 at 3:44 PM, Jeff King <peff@xxxxxxxx> wrote:
> The skip_prefix function returns a pointer to the content
> past the prefix, or NULL if the prefix was not found. While
> this is nice and simple, in practice it makes it hard to use
> for two reasons:
>
> 1. When you want to conditionally skip or keep the string
> as-is, you have to introduce a second temporary
> variable. For example:
>
> tmp = skip_prefix(buf, "foo");
> if (tmp)
> buf = tmp;
>
> 2. It is verbose to check the outcome in a conditional, as
> you need extra parentheses to silence compiler
> warnings. For example:
>
> if ((cp = skip_prefix(buf, "foo"))
> /* do something with cp */
>
> Both of these make it harder to use for long if-chains, and
> we tend to use starts_with instead. However, the first line
> of "do something" is often to then skip forward in buf past
> the prefix, either using a magic constant or with an extra
> strlen (which is generally computed at compile time, but
> means we are repeating ourselves).
>
> This patch refactors skip_prefix to return a simple boolean,
> and to provide the pointer value as an out-parameter. If the
> prefix is not found, the out-parameter is untouched. This
> lets you write:
>
> if (skip_prefix(arg, "foo ", &arg))
> do_foo(arg);
> else if (skip_prefix(arg, "bar ", &arg))
> do_bar(arg);
>
> Signed-off-by: Jeff King <peff@xxxxxxxx>
> ---
> diff --git a/git-compat-util.h b/git-compat-util.h
> index b6f03b3..556c839 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -349,13 +349,31 @@ extern void set_die_is_recursing_routine(int (*routine)(void));
> extern int starts_with(const char *str, const char *prefix);
> extern int ends_with(const char *str, const char *suffix);
>
> -static inline const char *skip_prefix(const char *str, const char *prefix)
> +/*
> + * If "str" begins with "prefix", return 1. If out is non-NULL,
> + * it it set to str + strlen(prefix) (i.e., the prefix is skipped).
The documentation claims that 'out' can be NULL, however, the code
does not respect this. NULL 'out' seems rather pointless (unless you
want an alias for starts_with()), so presumably the documentation is
incorrect.
> + *
> + * Otherwise, returns 0 and out is left untouched.
> + *
> + * Examples:
> + *
> + * [extract branch name, fail if not a branch]
> + * if (!skip_prefix(ref, "refs/heads/", &branch)
> + * return -1;
> + *
> + * [skip prefix if present, otherwise use whole string]
> + * skip_prefix(name, "refs/heads/", &name);
> + */
> +static inline int skip_prefix(const char *str, const char *prefix,
> + const char **out)
> {
> do {
> - if (!*prefix)
> - return str;
> + if (!*prefix) {
> + *out = str;
> + return 1;
> + }
> } while (*str++ == *prefix++);
> - return NULL;
> + return 0;
> }
>
> #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html