git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
Dec 18, 2013, 14:53 UTC
Message-ID
<1387378437-20646-6-git-send-email-pclouds@gmail.com>
In-Reply-To
<1387378437-20646-1-git-send-email-pclouds@gmail.com>

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 <pclouds@gmail.com>
---
 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
Previous: Nguyễn Thái Ngọc DuyNext: Nguyễn Thái Ngọc Duy
Message 18 of 31 in “Hard coded string length cleanup”
  1. 00/12 Hard coded string length cleanupNguyễn Thái Ngọc Duy, Dec 18, 2013
  2. 01/12 Make starts_with() a wrapper of skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  3. Junio C HamanoDec 18, 2013
  4. Junio C HamanoDec 18, 2013
  5. 02/12 Convert starts_with() to skip_prefix() for option parsingNguyễn Thái Ngọc Duy, Dec 18, 2013
  6. Johannes SixtDec 20, 2013
  7. Jeff KingDec 20, 2013
  8. Christian CouderDec 20, 2013
  9. René ScharfeDec 20, 2013
  10. Junio C HamanoDec 20, 2013
  11. Duy NguyenDec 21, 2013
  12. Junio C HamanoDec 26, 2013
  13. Jeff KingDec 28, 2013
  14. 03/12 Add and use skip_prefix_defval()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  15. Kent R. SpillnerDec 18, 2013
  16. Junio C HamanoDec 18, 2013
  17. 04/12 Replace some use of starts_with() with skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  18. 05/12 Convert a lot of starts_with() to skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  19. 06/12 fetch.c: replace some use of starts_with() with skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  20. 07/12 connect.c: replace some use of starts_with() with skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  21. 08/12 refs.c: replace some use of starts_with() with skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  22. 09/12 diff.c: reduce code duplication in --stat-xxx parsingNguyễn Thái Ngọc Duy, Dec 18, 2013
  23. 10/12 environment.c: replace starts_with() in strip_namespace() with skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  24. 11/12 diff.c: convert diff_scoreopt_parse to use skip_prefix()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  25. 12/12 refs.c: use skip_prefix() in prune_ref()Nguyễn Thái Ngọc Duy, Dec 18, 2013
  26. Junio C HamanoDec 18, 2013
  27. René ScharfeDec 19, 2013
  28. Duy NguyenDec 19, 2013
  29. René ScharfeDec 20, 2013
  30. Duy NguyenDec 20, 2013
  31. Junio C HamanoDec 20, 2013

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.