Subject: [PATCH 05/12] Convert a lot of starts_with() to skip_prefix()

From: Nguyễn Thái Ngọc Duy <[email-masked]>
Date: Wed, 18 Dec 2013 21:53:50 +0700

The purpose is remove hard coded string length. Some could be a few
lines away from the string comparison and easy to be missed when the
string is changed.

Signed-off-by: Nguyễn Thái Ngọc Duy <[email-masked]>
---
 builtin/for-each-ref.c |  9 +++++----
 builtin/mailinfo.c     |  6 +++---
 builtin/merge.c        |  8 +++++---
 builtin/remote.c       |  3 +--
 commit.c               |  5 +----
 diff.c                 |  9 +++------
 fetch-pack.c           |  9 +++++----
 http-backend.c         |  5 +++--
 http-push.c            |  6 +++---
 http.c                 |  5 +++--
 log-tree.c             |  5 +++--
 pager.c                |  2 +-
 pathspec.c             |  5 +++--
 refs.c                 | 12 +++++++-----
 sha1_name.c            | 12 +++---------
 transport-helper.c     | 15 +++++++--------
 transport.c            | 14 ++++++++------
 17 files changed, 64 insertions(+), 66 deletions(-)

diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 6551e7b..25c1388 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -662,6 +662,7 @@ static void populate_value(struct refinfo *ref)
 		const char *refname;
 		const char *formatp;
 		struct branch *branch = NULL;
+		const char *next;
 
 		if (*name == '*') {
 			deref = 1;
@@ -674,18 +675,18 @@ static void populate_value(struct refinfo *ref)
 			refname = ref->symref ? ref->symref : "";
 		else if (starts_with(name, "upstream")) {
 			/* only local branches may have an upstream */
-			if (!starts_with(ref->refname, "refs/heads/"))
+			if ((next = skip_prefix(ref->refname, "refs/heads/")) == NULL)
 				continue;
-			branch = branch_get(ref->refname + 11);
+			branch = branch_get(next);
 
 			if (!branch || !branch->merge || !branch->merge[0] ||
 			    !branch->merge[0]->dst)
 				continue;
 			refname = branch->merge[0]->dst;
-		} else if (starts_with(name, "color:")) {
+		} else if ((next = skip_prefix(name, "color:")) != NULL) {
 			char color[COLOR_MAXLEN] = "";
 
-			color_parse(name + 6, "--format", color);
+			color_parse(next, "--format", color);
 			v->s = xstrdup(color);
 			continue;
 		} else if (!strcmp(name, "flag")) {
diff --git a/builtin/mailinfo.c b/builtin/mailinfo.c
index 2100e23..daaafbd 100644
--- a/builtin/mailinfo.c
+++ b/builtin/mailinfo.c
@@ -328,13 +328,13 @@ static int check_header(const struct strbuf *line,
 	}
 
 	/* for inbody stuff */
-	if (starts_with(line->buf, ">From") && isspace(line->buf[5])) {
+	if (isspace(*skip_prefix_defval(line->buf, ">From", "NOSPACE"))) {
 		ret = 1; /* Should this return 0? */
 		goto check_header_out;
 	}
-	if (starts_with(line->buf, "[PATCH]") && isspace(line->buf[7])) {
+	if (isspace(*skip_prefix_defval(line->buf, "[PATCH]", "NOSPACE"))) {
 		for (i = 0; header[i]; i++) {
-			if (!memcmp("Subject", header[i], 7)) {
+			if (starts_with(header[i], "Subject")) {
 				handle_header(&hdr_data[i], line);
 				ret = 1;
 				goto check_header_out;
diff --git a/builtin/merge.c b/builtin/merge.c
index 590d907..603f80a 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -569,10 +569,12 @@ static void parse_branch_merge_options(char *bmo)
 static int git_merge_config(const char *k, const char *v, void *cb)
 {
 	int status;
+	const char *kk, *kkk;
 
-	if (branch && starts_with(k, "branch.") &&
-		starts_with(k + 7, branch) &&
-		!strcmp(k + 7 + strlen(branch), ".mergeoptions")) {
+	if (branch &&
+	    (kk = skip_prefix(k, "branch.")) != NULL &&
+	    (kkk = skip_prefix(kk, branch)) != NULL &&
+	    !strcmp(kkk, ".mergeoptions")) {
 		free(branch_mergeoptions);
 		branch_mergeoptions = xstrdup(v);
 		return 0;
diff --git a/builtin/remote.c b/builtin/remote.c
index b3ab4cf..218c8c8 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -259,14 +259,13 @@ static const char *abbrev_ref(const char *name, const char *prefix)
 
 static int config_read_branches(const char *key, const char *value, void *cb)
 {
-	if (starts_with(key, "branch.")) {
+	if ((key = skip_prefix(key, "branch.")) != NULL) {
 		const char *orig_key = key;
 		char *name;
 		struct string_list_item *item;
 		struct branch_info *info;
 		enum { REMOTE, MERGE, REBASE } type;
 
-		key += 7;
 		if (ends_with(key, ".remote")) {
 			name = xstrndup(key, strlen(key) - 7);
 			type = REMOTE;
diff --git a/commit.c b/commit.c
index 5df1df7..eed2ff9 100644
--- a/commit.c
+++ b/commit.c
@@ -1193,10 +1193,7 @@ static void parse_gpg_output(struct signature_check *sigc)
 	for (i = 0; i < ARRAY_SIZE(sigcheck_gpg_status); i++) {
 		const char *found, *next;
 
-		if (starts_with(buf, sigcheck_gpg_status[i].check + 1)) {
-			/* At the very beginning of the buffer */
-			found = buf + strlen(sigcheck_gpg_status[i].check + 1);
-		} else {
+		if ((found = skip_prefix(buf, sigcheck_gpg_status[i].check + 1)) == NULL) {
 			found = strstr(buf, sigcheck_gpg_status[i].check);
 			if (!found)
 				continue;
diff --git a/diff.c b/diff.c
index 90a1929..d754e2f 100644
--- a/diff.c
+++ b/diff.c
@@ -3388,13 +3388,10 @@ static inline int short_opt(char opt, const char **argv,
 int parse_long_opt(const char *opt, const char **argv,
 		   const char **optarg)
 {
-	const char *arg = argv[0];
-	if (arg[0] != '-' || arg[1] != '-')
-		return 0;
-	arg += strlen("--");
-	if (!starts_with(arg, opt))
+	const char *arg;
+	if ((arg = skip_prefix(argv[0], "--")) == NULL ||
+	    (arg = skip_prefix(arg, opt)) == NULL)
 		return 0;
-	arg += strlen(opt);
 	if (*arg == '=') { /* stuck form: --option=value */
 		*optarg = arg + 1;
 		return 1;
diff --git a/fetch-pack.c b/fetch-pack.c
index 760ed16..723ff06 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -317,18 +317,19 @@ static int find_common(struct fetch_pack_args *args,
 
 	if (args->depth > 0) {
 		char *line;
+		const char *sha1_str;
 		unsigned char sha1[20];
 
 		send_request(args, fd[1], &req_buf);
 		while ((line = packet_read_line(fd[0], NULL))) {
-			if (starts_with(line, "shallow ")) {
-				if (get_sha1_hex(line + 8, sha1))
+			if ((sha1_str = skip_prefix(line, "shallow ")) != NULL) {
+				if (get_sha1_hex(sha1_str, sha1))
 					die("invalid shallow line: %s", line);
 				register_shallow(sha1);
 				continue;
 			}
-			if (starts_with(line, "unshallow ")) {
-				if (get_sha1_hex(line + 10, sha1))
+			if ((sha1_str = skip_prefix(line, "unshallow ")) != NULL) {
+				if (get_sha1_hex(sha1_str, sha1))
 					die("invalid unshallow line: %s", line);
 				if (!lookup_object(sha1))
 					die("object not found: %s", line);
diff --git a/http-backend.c b/http-backend.c
index d2c0a62..e780c55 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -221,17 +221,18 @@ static void get_idx_file(char *name)
 
 static int http_config(const char *var, const char *value, void *cb)
 {
+	const char *p;
 	if (!strcmp(var, "http.getanyfile")) {
 		getanyfile = git_config_bool(var, value);
 		return 0;
 	}
 
-	if (starts_with(var, "http.")) {
+	if ((p = skip_prefix(var, "http.")) != NULL) {
 		int i;
 
 		for (i = 0; i < ARRAY_SIZE(rpc_service); i++) {
 			struct rpc_service *svc = &rpc_service[i];
-			if (!strcmp(var + 5, svc->config_name)) {
+			if (!strcmp(p, svc->config_name)) {
 				svc->enabled = git_config_bool(var, value);
 				return 0;
 			}
diff --git a/http-push.c b/http-push.c
index d4b40c9..5db6f28 100644
--- a/http-push.c
+++ b/http-push.c
@@ -771,9 +771,9 @@ static void handle_new_lock_ctx(struct xml_ctx *ctx, int tag_closed)
 			lock->owner = xmalloc(strlen(ctx->cdata) + 1);
 			strcpy(lock->owner, ctx->cdata);
 		} else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TIMEOUT)) {
-			if (starts_with(ctx->cdata, "Second-"))
-				lock->timeout =
-					strtol(ctx->cdata + 7, NULL, 10);
+			const char *p;
+			if ((p = skip_prefix(ctx->cdata, "Second-")) != NULL)
+				lock->timeout = strtol(p, NULL, 10);
 		} else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TOKEN)) {
 			lock->token = xmalloc(strlen(ctx->cdata) + 1);
 			strcpy(lock->token, ctx->cdata);
diff --git a/http.c b/http.c
index 70eaa26..1120ed2 100644
--- a/http.c
+++ b/http.c
@@ -1098,6 +1098,7 @@ int http_fetch_ref(const char *base, struct ref *ref)
 	char *url;
 	struct strbuf buffer = STRBUF_INIT;
 	int ret = -1;
+	const char *p;
 
 	options.no_cache = 1;
 
@@ -1106,8 +1107,8 @@ int http_fetch_ref(const char *base, struct ref *ref)
 		strbuf_rtrim(&buffer);
 		if (buffer.len == 40)
 			ret = get_sha1_hex(buffer.buf, ref->old_sha1);
-		else if (starts_with(buffer.buf, "ref: ")) {
-			ref->symref = xstrdup(buffer.buf + 5);
+		else if ((p = skip_prefix(buffer.buf, "ref: ")) != NULL) {
+			ref->symref = xstrdup(p);
 			ret = 0;
 		}
 	}
diff --git a/log-tree.c b/log-tree.c
index 642faff..cef7c8d 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -96,13 +96,14 @@ static void add_name_decoration(enum decoration_type type, const char *name, str
 static int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
 {
 	struct object *obj;
+	const char *name;
 	enum decoration_type type = DECORATION_NONE;
 
-	if (starts_with(refname, "refs/replace/")) {
+	if ((name = skip_prefix(refname, "refs/replace/")) != NULL) {
 		unsigned char original_sha1[20];
 		if (!read_replace_refs)
 			return 0;
-		if (get_sha1_hex(refname + 13, original_sha1)) {
+		if (get_sha1_hex(name, original_sha1)) {
 			warning("invalid replace ref %s", refname);
 			return 0;
 		}
diff --git a/pager.c b/pager.c
index 345b0bc..175cd9f 100644
--- a/pager.c
+++ b/pager.c
@@ -151,7 +151,7 @@ int decimal_width(int number)
 static int pager_command_config(const char *var, const char *value, void *data)
 {
 	struct pager_config *c = data;
-	if (starts_with(var, "pager.") && !strcmp(var + 6, c->cmd)) {
+	if (!strcmp(skip_prefix_defval(var, "pager.", ""), c->cmd)) {
 		int b = git_config_maybe_bool(var, value);
 		if (b >= 0)
 			c->want = b;
diff --git a/pathspec.c b/pathspec.c
index 52d38a4..e15f215 100644
--- a/pathspec.c
+++ b/pathspec.c
@@ -149,14 +149,15 @@ static unsigned prefix_pathspec(struct pathspec_item *item,
 			if (!len)
 				continue;
 			for (i = 0; i < ARRAY_SIZE(pathspec_magic); i++) {
+				const char *prefix_str;
 				if (strlen(pathspec_magic[i].name) == len &&
 				    !strncmp(pathspec_magic[i].name, copyfrom, len)) {
 					magic |= pathspec_magic[i].bit;
 					break;
 				}
-				if (starts_with(copyfrom, "prefix:")) {
+				if ((prefix_str = skip_prefix(copyfrom, "prefix:")) != NULL) {
 					char *endptr;
-					pathspec_prefix = strtol(copyfrom + 7,
+					pathspec_prefix = strtol(prefix_str,
 								 &endptr, 10);
 					if (endptr - copyfrom != len)
 						die(_("invalid parameter for pathspec magic 'prefix'"));
diff --git a/refs.c b/refs.c
index 3926136..5e378bc 100644
--- a/refs.c
+++ b/refs.c
@@ -1873,11 +1873,13 @@ int for_each_rawref(each_ref_fn fn, void *cb_data)
 
 const char *prettify_refname(const char *name)
 {
-	return name + (
-		starts_with(name, "refs/heads/") ? 11 :
-		starts_with(name, "refs/tags/") ? 10 :
-		starts_with(name, "refs/remotes/") ? 13 :
-		0);
+	const char *p;
+	if ((p = skip_prefix(name, "refs/heads/")) != NULL ||
+	    (p = skip_prefix(name, "refs/tags/")) != NULL ||
+	    (p = skip_prefix(name, "refs/remotes/")) != NULL)
+		return p;
+	else
+		return name;
 }
 
 const char *ref_rev_parse_rules[] = {
diff --git a/sha1_name.c b/sha1_name.c
index b1873d8..3fc4ede 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -546,14 +546,10 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 		if (read_ref_at(real_ref, at_time, nth, sha1, NULL,
 				&co_time, &co_tz, &co_cnt)) {
 			if (!len) {
-				if (starts_with(real_ref, "refs/heads/")) {
-					str = real_ref + 11;
-					len = strlen(real_ref + 11);
-				} else {
+				if ((str = skip_prefix(real_ref, "refs/heads/")) == NULL)
 					/* detached HEAD */
 					str = "HEAD";
-					len = 4;
-				}
+				len = strlen(str);
 			}
 			if (at_time)
 				warning("Log for '%.*s' only goes "
@@ -909,10 +905,8 @@ static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
 	const char *match = NULL, *target = NULL;
 	size_t len;
 
-	if (starts_with(message, "checkout: moving from ")) {
-		match = message + strlen("checkout: moving from ");
+	if ((match = skip_prefix(message, "checkout: moving from ")) != NULL)
 		target = strstr(match, " to ");
-	}
 
 	if (!match || !target)
 		return 0;
diff --git a/transport-helper.c b/transport-helper.c
index 2010674..601aba8 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -373,10 +373,10 @@ static int fetch_with_fetch(struct transport *transport,
 	sendline(data, &buf);
 
 	while (1) {
+		const char *name;
 		recvline(data, &buf);
 
-		if (starts_with(buf.buf, "lock ")) {
-			const char *name = buf.buf + 5;
+		if ((name = skip_prefix(buf.buf, "lock ")) != NULL) {
 			if (transport->pack_lockfile)
 				warning("%s also locked %s", data->name, name);
 			else
@@ -643,16 +643,15 @@ static int push_update_ref_status(struct strbuf *buf,
 				   struct ref **ref,
 				   struct ref *remote_refs)
 {
-	char *refname, *msg;
+	const char *refname;
+	char *msg;
 	int status;
 
-	if (starts_with(buf->buf, "ok ")) {
+	if ((refname = skip_prefix(buf->buf, "ok ")) != NULL)
 		status = REF_STATUS_OK;
-		refname = buf->buf + 3;
-	} else if (starts_with(buf->buf, "error ")) {
+	else if ((refname = skip_prefix(buf->buf, "error ")) != NULL)
 		status = REF_STATUS_REMOTE_REJECT;
-		refname = buf->buf + 6;
-	} else
+	else
 		die("expected ok/error, helper said '%s'", buf->buf);
 
 	msg = strchr(refname, ' ');
diff --git a/transport.c b/transport.c
index 824c5b9..e88c2dc 100644
--- a/transport.c
+++ b/transport.c
@@ -147,9 +147,9 @@ static void set_upstreams(struct transport *transport, struct ref *refs,
 {
 	struct ref *ref;
 	for (ref = refs; ref; ref = ref->next) {
-		const char *localname;
+		const char *localname, *short_local;
 		const char *tmp;
-		const char *remotename;
+		const char *remotename, *short_remote;
 		unsigned char sha[20];
 		int flag = 0;
 		/*
@@ -173,18 +173,20 @@ static void set_upstreams(struct transport *transport, struct ref *refs,
 			localname = tmp;
 
 		/* Both source and destination must be local branches. */
-		if (!localname || !starts_with(localname, "refs/heads/"))
+		if (!localname ||
+		    (short_local = skip_prefix(localname, "refs/heads/")) == NULL)
 			continue;
-		if (!remotename || !starts_with(remotename, "refs/heads/"))
+		if (!remotename ||
+		    (short_remote = skip_prefix(remotename, "refs/heads/")) == NULL)
 			continue;
 
 		if (!pretend)
 			install_branch_config(BRANCH_CONFIG_VERBOSE,
-				localname + 11, transport->remote->name,
+				short_local, transport->remote->name,
 				remotename);
 		else
 			printf("Would set upstream of '%s' to '%s' of '%s'\n",
-				localname + 11, remotename + 11,
+				short_local, short_remote,
 				transport->remote->name);
 	}
 }
-- 
1.8.5.1.208.g019362e