refactor skip_prefix to return a boolean

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 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(3) (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@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/advice.c b/advice.c
index c50ebdf5fe82d88b901ca26668806c94e03fbb37..9b420331926c6bf1aa371f973d6a96886f419b06 100644 (file)
--- a/advice.c
+++ b/advice.c
@@ -61,9 +61,12 @@ void advise(const char *advice, ...)
 
 int git_default_advice_config(const char *var, const char *value)
 {
-       const char *k = skip_prefix(var, "advice.");
+       const char *k;
        int i;
 
+       if (!skip_prefix(var, "advice.", &k))
+               return 0;
+
        for (i = 0; i < ARRAY_SIZE(advice_config); i++) {
                if (strcmp(k, advice_config[i].name))
                        continue;

diff --git a/branch.c b/branch.c
index 660097bc29a682c4481308ba245e5e02f0661681..46e8aa86df1811ff19899869a33ecb48cacb107f 100644 (file)
--- a/branch.c
+++ b/branch.c
@@ -50,11 +50,11 @@ static int should_setup_rebase(const char *origin)
 
 void install_branch_config(int flag, const char *local, const char *origin, const char *remote)
 {
-       const char *shortname = skip_prefix(remote, "refs/heads/");
+       const char *shortname = NULL;
        struct strbuf key = STRBUF_INIT;
        int rebasing = should_setup_rebase(origin);
 
-       if (shortname
+       if (skip_prefix(remote, "refs/heads/", &shortname)
            && !strcmp(local, shortname)
            && !origin) {
                warning(_("Not setting branch %s as its own upstream."),

diff --git a/builtin/branch.c b/builtin/branch.c
index 652b1d2d1484032fab51165f1d0b0a41426d114f..0591b22a483619ac9a7d889a49e45ffa9d68ab72 100644 (file)
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -294,13 +294,13 @@ static char *resolve_symref(const char *src, const char *prefix)
 {
        unsigned char sha1[20];
        int flag;
-       const char *dst, *cp;
+       const char *dst;
 
        dst = resolve_ref_unsafe(src, sha1, 0, &flag);
        if (!(dst && (flag & REF_ISSYMREF)))
                return NULL;
-       if (prefix && (cp = skip_prefix(dst, prefix)))
-               dst = cp;
+       if (prefix)
+               skip_prefix(dst, prefix, &dst);
        return xstrdup(dst);
 }
 

diff --git a/builtin/clone.c b/builtin/clone.c
index b12989d1caecb48128e8a468014f07b237b1a4cc..a5b2d9db360eb59d5a73c2e221fed047a94b0615 100644 (file)
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -584,11 +584,11 @@ static void update_remote_refs(const struct ref *refs,
 static void update_head(const struct ref *our, const struct ref *remote,
                        const char *msg)
 {
-       if (our && starts_with(our->name, "refs/heads/")) {
+       const char *head;
+       if (our && skip_prefix(our->name, "refs/heads/", &head)) {
                /* Local default branch link */
                create_symref("HEAD", our->name, NULL);
                if (!option_bare) {
-                       const char *head = skip_prefix(our->name, "refs/heads/");
                        update_ref(msg, "HEAD", our->old_sha1, NULL, 0,
                                   UPDATE_REFS_DIE_ON_ERR);
                        install_branch_config(0, head, option_origin, our->name);
@@ -703,9 +703,12 @@ static void write_refspec_config(const char* src_ref_prefix,
                                        strbuf_addf(&value, "+%s:%s%s", our_head_points_at->name,
                                                branch_top->buf, option_branch);
                        } else if (remote_head_points_at) {
+                               const char *head = remote_head_points_at->name;
+                               if (!skip_prefix(head, "refs/heads/", &head))
+                                       die("BUG: remote HEAD points at non-head?");
+
                                strbuf_addf(&value, "+%s:%s%s", remote_head_points_at->name,
-                                               branch_top->buf,
-                                               skip_prefix(remote_head_points_at->name, "refs/heads/"));
+                                               branch_top->buf, head);
                        }
                        /*
                         * otherwise, the next "git fetch" will

diff --git a/builtin/commit.c b/builtin/commit.c
index 5e2221c8e8c842314cb177d55223e87328b3da26..ec753412381f26b29274805d32524515171bb39d 100644 (file)
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1020,7 +1020,7 @@ static int message_is_empty(struct strbuf *sb)
 static int template_untouched(struct strbuf *sb)
 {
        struct strbuf tmpl = STRBUF_INIT;
-       char *start;
+       const char *start;
 
        if (cleanup_mode == CLEANUP_NONE && sb->len)
                return 0;
@@ -1029,8 +1029,7 @@ static int template_untouched(struct strbuf *sb)
                return 0;
 
        stripspace(&tmpl, cleanup_mode == CLEANUP_ALL);
-       start = (char *)skip_prefix(sb->buf, tmpl.buf);
-       if (!start)
+       if (!skip_prefix(sb->buf, tmpl.buf, &start))
                start = sb->buf;
        strbuf_release(&tmpl);
        return rest_is_empty(sb, start - sb->buf);

diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index 72378e6810a33b8d0e0285b14babc57b39287bb9..3c19241554da225423fb1f10eaaebb56db7e73fd 100644 (file)
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -297,7 +297,7 @@ static void credit_people(struct strbuf *out,
        if (!them->nr ||
            (them->nr == 1 &&
             me &&
-            (me = skip_prefix(me, them->items->string)) != NULL &&
+            skip_prefix(me, them->items->string, &me) &&
             starts_with(me, " <")))
                return;
        strbuf_addf(out, "\n%c %s ", comment_line_char, label);

diff --git a/builtin/push.c b/builtin/push.c
index f8dfea41e1ad8b6d888c1a2adc13eee87083491e..f50e3d5e77db6842ff3450560aad330f6a4bd91d 100644 (file)
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -127,11 +127,10 @@ static NORETURN int die_push_simple(struct branch *branch, struct remote *remote
         * them the big ugly fully qualified ref.
         */
        const char *advice_maybe = "";
-       const char *short_upstream =
-               skip_prefix(branch->merge[0]->src, "refs/heads/");
+       const char *short_upstream = branch->merge[0]->src;
+
+       skip_prefix(short_upstream, "refs/heads/", &short_upstream);
 
-       if (!short_upstream)
-               short_upstream = branch->merge[0]->src;
        /*
         * Don't show advice for people who explicitly set
         * push.default.

diff --git a/builtin/remote.c b/builtin/remote.c
index c9102e8fe94b41af5136a9cc23db939effae147a..a8efe3da4d7dad3b17582c91671f0f475d54a9ac 100644 (file)
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -250,9 +250,7 @@ static struct string_list branch_list;
 
 static const char *abbrev_ref(const char *name, const char *prefix)
 {
-       const char *abbrev = skip_prefix(name, prefix);
-       if (abbrev)
-               return abbrev;
+       skip_prefix(name, prefix, &name);
        return name;
 }
 #define abbrev_branch(name) abbrev_ref((name), "refs/heads/")

diff --git a/column.c b/column.c
index 1a468debb4abdda3cbb90fcc4972b3d40da6f2ed..ca878bcea7a42476a7c2b03e74897a2af87bec3f 100644 (file)
--- a/column.c
+++ b/column.c
@@ -336,8 +336,9 @@ static int column_config(const char *var, const char *value,
 int git_column_config(const char *var, const char *value,
                      const char *command, unsigned int *colopts)
 {
-       const char *it = skip_prefix(var, "column.");
-       if (!it)
+       const char *it;
+
+       if (!skip_prefix(var, "column.", &it))
                return 0;
 
        if (!strcmp(it, "ui"))

diff --git a/commit.c b/commit.c
index 881be3baa3ccc70afab916331588afb9330258ed..dfc0eb02415edccba6195b4d2925196caa242a1a 100644 (file)
--- a/commit.c
+++ b/commit.c
@@ -556,8 +556,7 @@ static void record_author_date(struct author_date_slab *author_date,
             buf;
             buf = line_end + 1) {
                line_end = strchrnul(buf, '\n');
-               ident_line = skip_prefix(buf, "author ");
-               if (!ident_line) {
+               if (!skip_prefix(buf, "author ", &ident_line)) {
                        if (!line_end[0] || line_end[1] == '\n')
                                return; /* end of header */
                        continue;
@@ -1183,8 +1182,7 @@ static void parse_gpg_output(struct signature_check *sigc)
        for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
                const char *found, *next;
 
-               found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);
-               if (!found) {
+               if (!skip_prefix(buf, sigcheck_gpg_status[i].check + 1, &found)) {
                        found = strstr(buf, sigcheck_gpg_status[i].check);
                        if (!found)
                                continue;

diff --git a/config.c b/config.c
index a1aef1cf3eca01e328b3e319636269236ea91ae0..ba882a1f51de347dc52fa8c52623dbc376f36d80 100644 (file)
--- a/config.c
+++ b/config.c
@@ -138,8 +138,7 @@ int git_config_include(const char *var, const char *value, void *data)
        if (ret < 0)
                return ret;
 
-       type = skip_prefix(var, "include.");
-       if (!type)
+       if (!skip_prefix(var, "include.", &type))
                return ret;
 
        if (!strcmp(type, "path"))

diff --git a/credential-cache--daemon.c b/credential-cache--daemon.c
index 390f194252cc6d7c0c9f43e2a48692324bf0b665..3b370ca5e529c74e17e0f39da5c689ebc4ccfc25 100644 (file)
--- a/credential-cache--daemon.c
+++ b/credential-cache--daemon.c
@@ -109,14 +109,12 @@ static int read_request(FILE *fh, struct credential *c,
        const char *p;
 
        strbuf_getline(&item, fh, '\n');
-       p = skip_prefix(item.buf, "action=");
-       if (!p)
+       if (!skip_prefix(item.buf, "action=", &p))
                return error("client sent bogus action line: %s", item.buf);
        strbuf_addstr(action, p);
 
        strbuf_getline(&item, fh, '\n');
-       p = skip_prefix(item.buf, "timeout=");
-       if (!p)
+       if (!skip_prefix(item.buf, "timeout=", &p))
                return error("client sent bogus timeout line: %s", item.buf);
        *timeout = atoi(p);
 

diff --git a/credential.c b/credential.c
index e54753c75d1c2abf7916f1aa7075d4ada3cfc61f..4d79d320f89e956aa9233a170120f05b2473ca59 100644 (file)
--- a/credential.c
+++ b/credential.c
@@ -40,8 +40,7 @@ static int credential_config_callback(const char *var, const char *value,
        struct credential *c = data;
        const char *key, *dot;
 
-       key = skip_prefix(var, "credential.");
-       if (!key)
+       if (!skip_prefix(var, "credential.", &key))
                return 0;
 
        if (!value)

diff --git a/fsck.c b/fsck.c
index abed62bac77c67e7c7447a18fea75f228406baac..bdbea2b995dd5714e8422e18c0bed08ae1a41afa 100644 (file)
--- a/fsck.c
+++ b/fsck.c
@@ -278,20 +278,18 @@ static int fsck_ident(const char **ident, struct object *obj, fsck_error error_f
 
 static int fsck_commit(struct commit *commit, fsck_error error_func)
 {
-       const char *buffer = commit->buffer, *tmp;
+       const char *buffer = commit->buffer;
        unsigned char tree_sha1[20], sha1[20];
        struct commit_graft *graft;
        int parents = 0;
        int err;
 
-       buffer = skip_prefix(buffer, "tree ");
-       if (!buffer)
+       if (!skip_prefix(buffer, "tree ", &buffer))
                return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'tree' line");
        if (get_sha1_hex(buffer, tree_sha1) || buffer[40] != '\n')
                return error_func(&commit->object, FSCK_ERROR, "invalid 'tree' line format - bad sha1");
        buffer += 41;
-       while ((tmp = skip_prefix(buffer, "parent "))) {
-               buffer = tmp;
+       while (skip_prefix(buffer, "parent ", &buffer)) {
                if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n')
                        return error_func(&commit->object, FSCK_ERROR, "invalid 'parent' line format - bad sha1");
                buffer += 41;
@@ -318,14 +316,12 @@ static int fsck_commit(struct commit *commit, fsck_error error_func)
                if (p || parents)
                        return error_func(&commit->object, FSCK_ERROR, "parent objects missing");
        }
-       buffer = skip_prefix(buffer, "author ");
-       if (!buffer)
+       if (!skip_prefix(buffer, "author ", &buffer))
                return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'author' line");
        err = fsck_ident(&buffer, &commit->object, error_func);
        if (err)
                return err;
-       buffer = skip_prefix(buffer, "committer ");
-       if (!buffer)
+       if (!skip_prefix(buffer, "committer ", &buffer))
                return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'committer' line");
        err = fsck_ident(&buffer, &commit->object, error_func);
        if (err)

diff --git a/git-compat-util.h b/git-compat-util.h
index b6f03b36dc762b891e77a5ca44e7ca5f270c5af9..d29e1dff08de9caa2b37eff71bffe7d37225796e 100644 (file)
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -349,13 +349,32 @@ 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 the string "str" begins with the string found in "prefix", return 1.
+ * The "out" parameter is set to "str + strlen(prefix)" (i.e., to the point in
+ * the string right after the prefix).
+ *
+ * Otherwise, return 0 and leave "out" 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)

diff --git a/parse-options.c b/parse-options.c
index b536896f2689dee23afae7d3a169c065592aaabe..e7dafa80d55adb9b863c23f711389e5eaa6b2c5e 100644 (file)
--- a/parse-options.c
+++ b/parse-options.c
@@ -231,7 +231,8 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
                        continue;
 
 again:
-               rest = skip_prefix(arg, long_name);
+               if (!skip_prefix(arg, long_name, &rest))
+                       rest = NULL;
                if (options->type == OPTION_ARGUMENT) {
                        if (!rest)
                                continue;
@@ -280,12 +281,13 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
                                continue;
                        }
                        flags |= OPT_UNSET;
-                       rest = skip_prefix(arg + 3, long_name);
-                       /* abbreviated and negated? */
-                       if (!rest && starts_with(long_name, arg + 3))
-                               goto is_abbreviated;
-                       if (!rest)
-                               continue;
+                       if (!skip_prefix(arg + 3, long_name, &rest)) {
+                               /* abbreviated and negated? */
+                               if (starts_with(long_name, arg + 3))
+                                       goto is_abbreviated;
+                               else
+                                       continue;
+                       }
                }
                if (*rest) {
                        if (*rest != '=')

diff --git a/transport.c b/transport.c
index 325f03e1eef97df296bb1cc9f7d2d33c218dde26..59c9727d8d63d548001513c1560b5c7a7e065b95 100644 (file)
--- a/transport.c
+++ b/transport.c
@@ -192,7 +192,9 @@ static void set_upstreams(struct transport *transport, struct ref *refs,
 
 static const char *rsync_url(const char *url)
 {
-       return !starts_with(url, "rsync://") ? skip_prefix(url, "rsync:") : url;
+       if (!starts_with(url, "rsync://"))
+               skip_prefix(url, "rsync:", &url);
+       return url;
 }
 
 static struct ref *get_refs_via_rsync(struct transport *transport, int for_push)

diff --git a/urlmatch.c b/urlmatch.c
index ec87cba75099ebe05e936430ca3b688275e01a4a..3d4c54b5cd5d6f6cb4e0ea97550799eb9d46f033 100644 (file)
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -483,8 +483,7 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)
        int user_matched = 0;
        int retval;
 
-       key = skip_prefix(var, collect->section);
-       if (!key || *(key++) != '.') {
+       if (!skip_prefix(var, collect->section, &key) || *(key++) != '.') {
                if (collect->cascade_fn)
                        return collect->cascade_fn(var, value, cb);
                return 0; /* not interested */