Skip to content

Commit

Permalink
refactor skip_prefix to return a boolean
Browse files Browse the repository at this point in the history
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 <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>
peff authored and gitster committed Jun 20, 2014
1 parent c026418 commit cf4fff5
Showing 18 changed files with 73 additions and 58 deletions.
5 changes: 4 additions & 1 deletion advice.c
Original file line number Diff line number Diff line change
@@ -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;
4 changes: 2 additions & 2 deletions branch.c
Original file line number Diff line number Diff line change
@@ -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."),
6 changes: 3 additions & 3 deletions builtin/branch.c
Original file line number Diff line number Diff line change
@@ -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);
}

11 changes: 7 additions & 4 deletions builtin/clone.c
Original file line number Diff line number Diff line change
@@ -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
5 changes: 2 additions & 3 deletions builtin/commit.c
Original file line number Diff line number Diff line change
@@ -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);
2 changes: 1 addition & 1 deletion builtin/fmt-merge-msg.c
Original file line number Diff line number Diff line change
@@ -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);
7 changes: 3 additions & 4 deletions builtin/push.c
Original file line number Diff line number Diff line change
@@ -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.
4 changes: 1 addition & 3 deletions builtin/remote.c
Original file line number Diff line number Diff line change
@@ -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/")
5 changes: 3 additions & 2 deletions column.c
Original file line number Diff line number Diff line change
@@ -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"))
6 changes: 2 additions & 4 deletions commit.c
Original file line number Diff line number Diff line change
@@ -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;
3 changes: 1 addition & 2 deletions config.c
Original file line number Diff line number Diff line change
@@ -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"))
6 changes: 2 additions & 4 deletions credential-cache--daemon.c
Original file line number Diff line number Diff line change
@@ -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);

3 changes: 1 addition & 2 deletions credential.c
Original file line number Diff line number Diff line change
@@ -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)
14 changes: 5 additions & 9 deletions fsck.c
Original file line number Diff line number Diff line change
@@ -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)
27 changes: 23 additions & 4 deletions git-compat-util.h
Original file line number Diff line number Diff line change
@@ -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)
16 changes: 9 additions & 7 deletions parse-options.c
Original file line number Diff line number Diff line change
@@ -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 != '=')
4 changes: 3 additions & 1 deletion transport.c
Original file line number Diff line number Diff line change
@@ -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)
3 changes: 1 addition & 2 deletions urlmatch.c
Original file line number Diff line number Diff line change
@@ -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 */

0 comments on commit cf4fff5

Please sign in to comment.