@matheusbsbs / matheus-bagliano / commits / cf4fff579e

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>

Jeff King committed Jun 18, 2014 at 15:44 UTC cf4fff579e02d8585d59f6c8739534b7b0d617dd

18 files changed +73 -58

advice.c

+4 -1

@@ -61,9 +61,12 @@ void advise(const char *advice, ...)

62 int git_default_advice_config(const char *var, const char *value)

63 {

64 - const char *k = skip_prefix(var, "advice.");

64 + const char *k;

65 int i;

67 + if (!skip_prefix(var, "advice.", &k))

68 + return 0;

69 +

70 for (i = 0; i < ARRAY_SIZE(advice_config); i++) {

71 if (strcmp(k, advice_config[i].name))

72 continue;

branch.c

+2 -2

@@ -50,11 +50,11 @@ static int should_setup_rebase(const char *origin)

51 void install_branch_config(int flag, const char *local, const char *origin, const char *remote)

52 {

53 - const char *shortname = skip_prefix(remote, "refs/heads/");

53 + const char *shortname = NULL;

54 struct strbuf key = STRBUF_INIT;

55 int rebasing = should_setup_rebase(origin);

57 - if (shortname

57 + if (skip_prefix(remote, "refs/heads/", &shortname)

58 && !strcmp(local, shortname)

59 && !origin) {

60 warning(_("Not setting branch %s as its own upstream."),

builtin/branch.c

+3 -3

@@ -294,13 +294,13 @@ static char *resolve_symref(const char *src, const char *prefix)

294 {

295 unsigned char sha1[20];

296 int flag;

297 - const char *dst, *cp;

297 + const char *dst;

298

299 dst = resolve_ref_unsafe(src, sha1, 0, &flag);

300 if (!(dst && (flag & REF_ISSYMREF)))

301 return NULL;

302 - if (prefix && (cp = skip_prefix(dst, prefix)))

303 - dst = cp;

302 + if (prefix)

303 + skip_prefix(dst, prefix, &dst);

304 return xstrdup(dst);

305 }

306

builtin/clone.c

+7 -4

@@ -584,11 +584,11 @@ static void update_remote_refs(const struct ref *refs,

584 static void update_head(const struct ref *our, const struct ref *remote,

585 const char *msg)

586 {

587 - if (our && starts_with(our->name, "refs/heads/")) {

587 + const char *head;

588 + if (our && skip_prefix(our->name, "refs/heads/", &head)) {

589 /* Local default branch link */

590 create_symref("HEAD", our->name, NULL);

591 if (!option_bare) {

591 - const char *head = skip_prefix(our->name, "refs/heads/");

592 update_ref(msg, "HEAD", our->old_sha1, NULL, 0,

593 UPDATE_REFS_DIE_ON_ERR);

594 install_branch_config(0, head, option_origin, our->name);

@@ -703,9 +703,12 @@ static void write_refspec_config(const char* src_ref_prefix,

703 strbuf_addf(&value, "+%s:%s%s", our_head_points_at->name,

704 branch_top->buf, option_branch);

705 } else if (remote_head_points_at) {

706 + const char *head = remote_head_points_at->name;

707 + if (!skip_prefix(head, "refs/heads/", &head))

708 + die("BUG: remote HEAD points at non-head?");

709 +

710 strbuf_addf(&value, "+%s:%s%s", remote_head_points_at->name,

707 - branch_top->buf,

708 - skip_prefix(remote_head_points_at->name, "refs/heads/"));

711 + branch_top->buf, head);

712 }

713 /*

714 * otherwise, the next "git fetch" will

builtin/commit.c

+2 -3

@@ -1020,7 +1020,7 @@ static int message_is_empty(struct strbuf *sb)

1020 static int template_untouched(struct strbuf *sb)

1021 {

1022 struct strbuf tmpl = STRBUF_INIT;

1023 - char *start;

1023 + const char *start;

1024

1025 if (cleanup_mode == CLEANUP_NONE && sb->len)

1026 return 0;

@@ -1029,8 +1029,7 @@ static int template_untouched(struct strbuf *sb)

1029 return 0;

1030

1031 stripspace(&tmpl, cleanup_mode == CLEANUP_ALL);

1032 - start = (char *)skip_prefix(sb->buf, tmpl.buf);

1033 - if (!start)

1032 + if (!skip_prefix(sb->buf, tmpl.buf, &start))

1033 start = sb->buf;

1034 strbuf_release(&tmpl);

1035 return rest_is_empty(sb, start - sb->buf);

builtin/fmt-merge-msg.c

+1 -1

@@ -297,7 +297,7 @@ static void credit_people(struct strbuf *out,

297 if (!them->nr ||

298 (them->nr == 1 &&

299 me &&

300 - (me = skip_prefix(me, them->items->string)) != NULL &&

300 + skip_prefix(me, them->items->string, &me) &&

301 starts_with(me, " <")))

302 return;

303 strbuf_addf(out, "\n%c %s ", comment_line_char, label);

builtin/push.c

+3 -4

@@ -127,11 +127,10 @@ static NORETURN int die_push_simple(struct branch *branch, struct remote *remote

127 * them the big ugly fully qualified ref.

128 */

129 const char *advice_maybe = "";

130 - const char *short_upstream =

131 - skip_prefix(branch->merge[0]->src, "refs/heads/");

130 + const char *short_upstream = branch->merge[0]->src;

131 +

132 + skip_prefix(short_upstream, "refs/heads/", &short_upstream);

133

133 - if (!short_upstream)

134 - short_upstream = branch->merge[0]->src;

134 /*

135 * Don't show advice for people who explicitly set

136 * push.default.

builtin/remote.c

+1 -3

@@ -250,9 +250,7 @@ static struct string_list branch_list;

250

251 static const char *abbrev_ref(const char *name, const char *prefix)

252 {

253 - const char *abbrev = skip_prefix(name, prefix);

254 - if (abbrev)

255 - return abbrev;

253 + skip_prefix(name, prefix, &name);

254 return name;

255 }

256 #define abbrev_branch(name) abbrev_ref((name), "refs/heads/")

column.c

+3 -2

@@ -336,8 +336,9 @@ static int column_config(const char *var, const char *value,

336 int git_column_config(const char *var, const char *value,

337 const char *command, unsigned int *colopts)

338 {

339 - const char *it = skip_prefix(var, "column.");

340 - if (!it)

339 + const char *it;

340 +

341 + if (!skip_prefix(var, "column.", &it))

342 return 0;

343

344 if (!strcmp(it, "ui"))

commit.c

+2 -4

@@ -556,8 +556,7 @@ static void record_author_date(struct author_date_slab *author_date,

556 buf;

557 buf = line_end + 1) {

558 line_end = strchrnul(buf, '\n');

559 - ident_line = skip_prefix(buf, "author ");

560 - if (!ident_line) {

559 + if (!skip_prefix(buf, "author ", &ident_line)) {

560 if (!line_end[0] || line_end[1] == '\n')

561 return; /* end of header */

562 continue;

@@ -1183,8 +1182,7 @@ static void parse_gpg_output(struct signature_check *sigc)

1182 for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {

1183 const char *found, *next;

1184

1186 - found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1);

1187 - if (!found) {

1185 + if (!skip_prefix(buf, sigcheck_gpg_status[i].check + 1, &found)) {

1186 found = strstr(buf, sigcheck_gpg_status[i].check);

1187 if (!found)

1188 continue;

config.c

+1 -2

@@ -138,8 +138,7 @@ int git_config_include(const char *var, const char *value, void *data)

138 if (ret < 0)

139 return ret;

140

141 - type = skip_prefix(var, "include.");

142 - if (!type)

141 + if (!skip_prefix(var, "include.", &type))

142 return ret;

143

144 if (!strcmp(type, "path"))

credential-cache--daemon.c

+2 -4

@@ -109,14 +109,12 @@ static int read_request(FILE *fh, struct credential *c,

109 const char *p;

110

111 strbuf_getline(&item, fh, '\n');

112 - p = skip_prefix(item.buf, "action=");

113 - if (!p)

112 + if (!skip_prefix(item.buf, "action=", &p))

113 return error("client sent bogus action line: %s", item.buf);

114 strbuf_addstr(action, p);

115

116 strbuf_getline(&item, fh, '\n');

118 - p = skip_prefix(item.buf, "timeout=");

119 - if (!p)

117 + if (!skip_prefix(item.buf, "timeout=", &p))

118 return error("client sent bogus timeout line: %s", item.buf);

119 *timeout = atoi(p);

120

credential.c

+1 -2

@@ -40,8 +40,7 @@ static int credential_config_callback(const char *var, const char *value,

40 struct credential *c = data;

41 const char *key, *dot;

43 - key = skip_prefix(var, "credential.");

44 - if (!key)

43 + if (!skip_prefix(var, "credential.", &key))

44 return 0;

46 if (!value)

fsck.c

+5 -9

@@ -278,20 +278,18 @@ static int fsck_ident(const char **ident, struct object *obj, fsck_error error_f

278

279 static int fsck_commit(struct commit *commit, fsck_error error_func)

280 {

281 - const char *buffer = commit->buffer, *tmp;

281 + const char *buffer = commit->buffer;

282 unsigned char tree_sha1[20], sha1[20];

283 struct commit_graft *graft;

284 int parents = 0;

285 int err;

286

287 - buffer = skip_prefix(buffer, "tree ");

288 - if (!buffer)

287 + if (!skip_prefix(buffer, "tree ", &buffer))

288 return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'tree' line");

289 if (get_sha1_hex(buffer, tree_sha1) || buffer[40] != '\n')

290 return error_func(&commit->object, FSCK_ERROR, "invalid 'tree' line format - bad sha1");

291 buffer += 41;

293 - while ((tmp = skip_prefix(buffer, "parent "))) {

294 - buffer = tmp;

292 + while (skip_prefix(buffer, "parent ", &buffer)) {

293 if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n')

294 return error_func(&commit->object, FSCK_ERROR, "invalid 'parent' line format - bad sha1");

295 buffer += 41;

@@ -318,14 +316,12 @@ static int fsck_commit(struct commit *commit, fsck_error error_func)

316 if (p || parents)

317 return error_func(&commit->object, FSCK_ERROR, "parent objects missing");

318 }

321 - buffer = skip_prefix(buffer, "author ");

322 - if (!buffer)

319 + if (!skip_prefix(buffer, "author ", &buffer))

320 return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'author' line");

321 err = fsck_ident(&buffer, &commit->object, error_func);

322 if (err)

323 return err;

327 - buffer = skip_prefix(buffer, "committer ");

328 - if (!buffer)

324 + if (!skip_prefix(buffer, "committer ", &buffer))

325 return error_func(&commit->object, FSCK_ERROR, "invalid format - expected 'committer' line");

326 err = fsck_ident(&buffer, &commit->object, error_func);

327 if (err)

git-compat-util.h

+23 -4

@@ -349,13 +349,32 @@ extern void set_die_is_recursing_routine(int (*routine)(void));

349 extern int starts_with(const char *str, const char *prefix);

350 extern int ends_with(const char *str, const char *suffix);

351

352 -static inline const char *skip_prefix(const char *str, const char *prefix)

352 +/*

353 + * If the string "str" begins with the string found in "prefix", return 1.

354 + * The "out" parameter is set to "str + strlen(prefix)" (i.e., to the point in

355 + * the string right after the prefix).

356 + *

357 + * Otherwise, return 0 and leave "out" untouched.

358 + *

359 + * Examples:

360 + *

361 + * [extract branch name, fail if not a branch]

362 + * if (!skip_prefix(ref, "refs/heads/", &branch)

363 + * return -1;

364 + *

365 + * [skip prefix if present, otherwise use whole string]

366 + * skip_prefix(name, "refs/heads/", &name);

367 + */

368 +static inline int skip_prefix(const char *str, const char *prefix,

369 + const char **out)

370 {

371 do {

355 - if (!*prefix)

356 - return str;

372 + if (!*prefix) {

373 + *out = str;

374 + return 1;

375 + }

376 } while (*str++ == *prefix++);

358 - return NULL;

377 + return 0;

378 }

379

380 #if defined(NO_MMAP) || defined(USE_WIN32_MMAP)

parse-options.c

+9 -7

@@ -231,7 +231,8 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,

231 continue;

232

233 again:

234 - rest = skip_prefix(arg, long_name);

234 + if (!skip_prefix(arg, long_name, &rest))

235 + rest = NULL;

236 if (options->type == OPTION_ARGUMENT) {

237 if (!rest)

238 continue;

@@ -280,12 +281,13 @@ is_abbreviated:

281 continue;

282 }

283 flags |= OPT_UNSET;

283 - rest = skip_prefix(arg + 3, long_name);

284 - /* abbreviated and negated? */

285 - if (!rest && starts_with(long_name, arg + 3))

286 - goto is_abbreviated;

287 - if (!rest)

288 - continue;

284 + if (!skip_prefix(arg + 3, long_name, &rest)) {

285 + /* abbreviated and negated? */

286 + if (starts_with(long_name, arg + 3))

287 + goto is_abbreviated;

288 + else

289 + continue;

290 + }

291 }

292 if (*rest) {

293 if (*rest != '=')

transport.c

+3 -1

@@ -192,7 +192,9 @@ static void set_upstreams(struct transport *transport, struct ref *refs,

192

193 static const char *rsync_url(const char *url)

194 {

195 - return !starts_with(url, "rsync://") ? skip_prefix(url, "rsync:") : url;

195 + if (!starts_with(url, "rsync://"))

196 + skip_prefix(url, "rsync:", &url);

197 + return url;

198 }

199

200 static struct ref *get_refs_via_rsync(struct transport *transport, int for_push)

urlmatch.c

+1 -2

@@ -483,8 +483,7 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)

483 int user_matched = 0;

484 int retval;

485

486 - key = skip_prefix(var, collect->section);

487 - if (!key || *(key++) != '.') {

486 + if (!skip_prefix(var, collect->section, &key) || *(key++) != '.') {

487 if (collect->cascade_fn)

488 return collect->cascade_fn(var, value, cb);

489 return 0; /* not interested */