[PATCH v2] C: use skip-prefix to avoid hardcoded string length

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



We often skip an optional prefix in a string with a hardcoded
constant, e.g.

	if (starts_with(string, "prefix"))
		string += 6;

which is less error prone when written

	skip_prefix(string, "prefix", &string);

Note that this changes a few error messages from "git reflog expire
--expire=nonsense.timestamp", which used to complain by saying

    '--expire=nonsense.timestamp' is not a valid timestamp

but with this change, we say

    'nonsense.timestamp' is not a valid timestamp

which is more technically correct (the string with --expire= as
a prefix obviously cannot be a valid timestamp, but the error is
about the part of the input without that prefix).

Helped-by: Jeff King <peff@xxxxxxxx>
Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
---

 * v2 uses a simpler conversion that has a side effect of changing
   the error message from "git reflog expire" in a better way.

 builtin/fast-export.c | 3 +--
 builtin/reflog.c      | 9 +++++----
 notes.c               | 6 ++----
 parse-options.c       | 3 +--
 refs/files-backend.c  | 3 +--
 remote-curl.c         | 5 +++--
 sha1-name.c           | 9 ++-------
 transport-helper.c    | 5 +++--
 8 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index dbec4df92b..164406fdd4 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -870,8 +870,7 @@ static void handle_tag(const char *name, struct tag *tag)
 		printf("reset %s\nfrom %s\n\n",
 		       name, oid_to_hex(&null_oid));
 	}
-	if (starts_with(name, "refs/tags/"))
-		name += 10;
+	skip_prefix(name, "refs/tags/", &name);
 	printf("tag %s\n", name);
 	if (mark_tags) {
 		mark_next_object(&tag->object);
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 4d3430900d..81dfd563c0 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -560,15 +560,16 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
+
 		if (!strcmp(arg, "--dry-run") || !strcmp(arg, "-n"))
 			flags |= EXPIRE_REFLOGS_DRY_RUN;
-		else if (starts_with(arg, "--expire=")) {
-			if (parse_expiry_date(arg + 9, &cb.cmd.expire_total))
+		else if (skip_prefix(arg, "--expire=", &arg)) {
+			if (parse_expiry_date(arg, &cb.cmd.expire_total))
 				die(_("'%s' is not a valid timestamp"), arg);
 			explicit_expiry |= EXPIRE_TOTAL;
 		}
-		else if (starts_with(arg, "--expire-unreachable=")) {
-			if (parse_expiry_date(arg + 21, &cb.cmd.expire_unreachable))
+		else if (skip_prefix(arg, "--expire-unreachable=", &arg)) {
+			if (parse_expiry_date(arg, &cb.cmd.expire_unreachable))
 				die(_("'%s' is not a valid timestamp"), arg);
 			explicit_expiry |= EXPIRE_UNREACH;
 		}
diff --git a/notes.c b/notes.c
index 0c79964c26..a0349fa778 100644
--- a/notes.c
+++ b/notes.c
@@ -1279,10 +1279,8 @@ static void format_note(struct notes_tree *t, const struct object_id *object_oid
 		if (!ref || !strcmp(ref, GIT_NOTES_DEFAULT_REF)) {
 			strbuf_addstr(sb, "\nNotes:\n");
 		} else {
-			if (starts_with(ref, "refs/"))
-				ref += 5;
-			if (starts_with(ref, "notes/"))
-				ref += 6;
+			skip_prefix(ref, "refs/", &ref);
+			skip_prefix(ref, "notes/", &ref);
 			strbuf_addf(sb, "\nNotes (%s):\n", ref);
 		}
 	}
diff --git a/parse-options.c b/parse-options.c
index 60fae3ad21..e8c04109ba 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -357,8 +357,7 @@ static enum parse_opt_result parse_long_opt(
 			}
 			/* negated? */
 			if (!starts_with(arg, "no-")) {
-				if (starts_with(long_name, "no-")) {
-					long_name += 3;
+				if (skip_prefix(long_name, "no-", &long_name)) {
 					opt_flags |= OPT_UNSET;
 					goto again;
 				}
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 0ea66a28b6..561c33ac8a 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -465,8 +465,7 @@ static int files_read_raw_ref(struct ref_store *ref_store,
 	close(fd);
 	strbuf_rtrim(&sb_contents);
 	buf = sb_contents.buf;
-	if (starts_with(buf, "ref:")) {
-		buf += 4;
+	if (skip_prefix(buf, "ref:", &buf)) {
 		while (isspace(*buf))
 			buf++;
 
diff --git a/remote-curl.c b/remote-curl.c
index 350d92a074..8eb96152f5 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -1255,8 +1255,9 @@ static void parse_push(struct strbuf *buf)
 	int ret;
 
 	do {
-		if (starts_with(buf->buf, "push "))
-			argv_array_push(&specs, buf->buf + 5);
+		const char *arg;
+		if (skip_prefix(buf->buf, "push ", &arg))
+			argv_array_push(&specs, arg);
 		else
 			die(_("http transport does not support %s"), buf->buf);
 
diff --git a/sha1-name.c b/sha1-name.c
index 200eb373ad..75d1c32606 100644
--- a/sha1-name.c
+++ b/sha1-name.c
@@ -908,14 +908,9 @@ static int get_oid_basic(struct repository *r, const char *str, int len,
 				real_ref, flags, at_time, nth, oid, 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 {
-					/* detached HEAD */
+				if (!skip_prefix(real_ref, "refs/heads/", &str))
 					str = "HEAD";
-					len = 4;
-				}
+				len = strlen(str);
 			}
 			if (at_time) {
 				if (!(flags & GET_OID_QUIETLY)) {
diff --git a/transport-helper.c b/transport-helper.c
index 413d9d873e..20a7185ec4 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -404,11 +404,12 @@ static int fetch_with_fetch(struct transport *transport,
 	sendline(data, &buf);
 
 	while (1) {
+		const char *name;
+
 		if (recvline(data, &buf))
 			exit(128);
 
-		if (starts_with(buf.buf, "lock ")) {
-			const char *name = buf.buf + 5;
+		if (skip_prefix(buf.buf, "lock ", &name)) {
 			if (transport->pack_lockfile)
 				warning(_("%s also locked %s"), data->name, name);
 			else
-- 
2.25.0-299-g5579215f86




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux