All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Change "refs/" references to symbolic constants
@ 2007-02-19 18:39 Andy Parkins
  2007-02-19 18:55 ` Bill Lear
  2007-02-19 20:07 ` Junio C Hamano
  0 siblings, 2 replies; 30+ messages in thread
From: Andy Parkins @ 2007-02-19 18:39 UTC (permalink / raw)
  To: git

Changed repeated use of the same constants for the ref paths to be
symbolic constants.  I've defined them in refs.h

  refs/ is now PATH_REFS
  refs/heads/ is now PATH_REFS_HEADS
  refs/tags/ is now PATH_REFS_TAGS
  refs/remotes/ is now PATH_REFS_REMOTES

I've changed all references to them and made constants for the string
lengths as well.  This has clarified the code in some places; for
example:

 - len = strlen(refs[i]) + 11;
 + len = strlen(refs[i]) + STRLEN_PATH_REFS_TAGS + 1;

In this case 11 isn't STRLEN_PATH_REFS_HEADS, as it is in most other
cases, it's TAGS + 1.  With the change to symbolic constants it's much
clearer what is happening.

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
 builtin-branch.c        |   28 ++++++++++++++--------------
 builtin-describe.c      |    2 +-
 builtin-fmt-merge-msg.c |    5 +++--
 builtin-fsck.c          |    2 +-
 builtin-init-db.c       |   15 ++++++++-------
 builtin-name-rev.c      |   10 +++++-----
 builtin-pack-refs.c     |    2 +-
 builtin-push.c          |    8 ++++----
 builtin-show-branch.c   |   34 +++++++++++++++++-----------------
 builtin-show-ref.c      |    6 +++---
 connect.c               |   18 +++++++++---------
 fetch-pack.c            |    6 +++---
 http-fetch.c            |    7 ++++---
 http-push.c             |    4 ++--
 local-fetch.c           |    3 ++-
 path.c                  |    5 +++--
 receive-pack.c          |    4 ++--
 reflog-walk.c           |    6 +++---
 refs.c                  |   26 +++++++++++++-------------
 refs.h                  |   17 +++++++++++++++++
 setup.c                 |    5 +++--
 sha1_name.c             |   10 +++++-----
 wt-status.c             |    5 +++--
 23 files changed, 126 insertions(+), 102 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index d0e7209..928f1fe 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -85,12 +85,12 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
 
 	switch (kinds) {
 	case REF_REMOTE_BRANCH:
-		fmt = "refs/remotes/%s";
+		fmt = PATH_REFS_REMOTES "%s";
 		remote = "remote ";
 		force = 1;
 		break;
 	case REF_LOCAL_BRANCH:
-		fmt = "refs/heads/%s";
+		fmt = PATH_REFS_HEADS "%s";
 		remote = "";
 		break;
 	default:
@@ -178,15 +178,15 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,
 	int len;
 
 	/* Detect kind */
-	if (!strncmp(refname, "refs/heads/", 11)) {
+	if (!strncmp(refname, PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS)) {
 		kind = REF_LOCAL_BRANCH;
-		refname += 11;
-	} else if (!strncmp(refname, "refs/remotes/", 13)) {
+		refname += STRLEN_PATH_REFS_HEADS;
+	} else if (!strncmp(refname, PATH_REFS_REMOTES, STRLEN_PATH_REFS_REMOTES)) {
 		kind = REF_REMOTE_BRANCH;
-		refname += 13;
-	} else if (!strncmp(refname, "refs/tags/", 10)) {
+		refname += STRLEN_PATH_REFS_REMOTES;
+	} else if (!strncmp(refname, PATH_REFS_TAGS, STRLEN_PATH_REFS_TAGS)) {
 		kind = REF_TAG;
-		refname += 10;
+		refname += STRLEN_PATH_REFS_TAGS;
 	}
 
 	/* Don't add types the caller doesn't want */
@@ -318,7 +318,7 @@ static void create_branch(const char *name, const char *start_name,
 	char ref[PATH_MAX], msg[PATH_MAX + 20];
 	int forcing = 0;
 
-	snprintf(ref, sizeof ref, "refs/heads/%s", name);
+	snprintf(ref, sizeof ref, PATH_REFS_HEADS "%s", name);
 	if (check_ref_format(ref))
 		die("'%s' is not a valid branch name.", name);
 
@@ -366,13 +366,13 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 	if (!oldname)
 		die("cannot rename the current branch while not on any.");
 
-	if (snprintf(oldref, sizeof(oldref), "refs/heads/%s", oldname) > sizeof(oldref))
+	if (snprintf(oldref, sizeof(oldref), PATH_REFS_HEADS "%s", oldname) > sizeof(oldref))
 		die("Old branchname too long");
 
 	if (check_ref_format(oldref))
 		die("Invalid branch name: %s", oldref);
 
-	if (snprintf(newref, sizeof(newref), "refs/heads/%s", newname) > sizeof(newref))
+	if (snprintf(newref, sizeof(newref), PATH_REFS_HEADS "%s", newname) > sizeof(newref))
 		die("New branchname too long");
 
 	if (check_ref_format(newref))
@@ -476,9 +476,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		detached = 1;
 	}
 	else {
-		if (strncmp(head, "refs/heads/", 11))
-			die("HEAD not found below refs/heads!");
-		head += 11;
+		if (strncmp(head, PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS))
+			die("HEAD not found below " PATH_REFS_HEADS "!");
+		head += STRLEN_PATH_REFS_HEADS;
 	}
 
 	if (delete)
diff --git a/builtin-describe.c b/builtin-describe.c
index bcc6456..0f78363 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -52,7 +52,7 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
 	 * If --tags, then any tags are used.
 	 * Otherwise only annotated tags are used.
 	 */
-	if (!strncmp(path, "refs/tags/", 10)) {
+	if (!strncmp(path, PATH_TAGS, STRLEN_PATH_TAGS)) {
 		if (object->type == OBJ_TAG)
 			prio = 2;
 		else
diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index 87d3d63..d0615b5 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -4,6 +4,7 @@
 #include "diff.h"
 #include "revision.h"
 #include "tag.h"
+#include "refs.h"
 
 static const char *fmt_merge_msg_usage =
 	"git-fmt-merge-msg [--summary] [--no-summary] [--file <file>]";
@@ -280,8 +281,8 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 	current_branch = resolve_ref("HEAD", head_sha1, 1, NULL);
 	if (!current_branch)
 		die("No current branch");
-	if (!strncmp(current_branch, "refs/heads/", 11))
-		current_branch += 11;
+	if (!strncmp(current_branch, PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS))
+		current_branch += STRLEN_PATH_REFS_HEADS;
 
 	while (fgets(line, sizeof(line), in)) {
 		i++;
diff --git a/builtin-fsck.c b/builtin-fsck.c
index 6da3814..0109816 100644
--- a/builtin-fsck.c
+++ b/builtin-fsck.c
@@ -546,7 +546,7 @@ static int fsck_head_link(void)
 
 	if (!head_points_at || !(flag & REF_ISSYMREF))
 		return error("HEAD is not a symbolic ref");
-	if (strncmp(head_points_at, "refs/heads/", 11))
+	if (strncmp(head_points_at, PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS))
 		return error("HEAD points to something strange (%s)",
 			     head_points_at);
 	if (is_null_sha1(sha1))
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 12e43d0..4e5c881 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -5,6 +5,7 @@
  */
 #include "cache.h"
 #include "builtin.h"
+#include "refs.h"
 
 #ifndef DEFAULT_GIT_TEMPLATE_DIR
 #define DEFAULT_GIT_TEMPLATE_DIR "/usr/share/git-core/templates/"
@@ -193,11 +194,11 @@ static int create_default_files(const char *git_dir, const char *template_path)
 	/*
 	 * Create .git/refs/{heads,tags}
 	 */
-	strcpy(path + len, "refs");
+	strcpy(path + len, PATH_REFS);
 	safe_create_dir(path, 1);
-	strcpy(path + len, "refs/heads");
+	strcpy(path + len, PATH_REFS_HEADS);
 	safe_create_dir(path, 1);
-	strcpy(path + len, "refs/tags");
+	strcpy(path + len, PATH_REFS_TAGS);
 	safe_create_dir(path, 1);
 
 	/* First copy the templates -- we might have the default
@@ -216,11 +217,11 @@ static int create_default_files(const char *git_dir, const char *template_path)
 	if (shared_repository) {
 		path[len] = 0;
 		adjust_shared_perm(path);
-		strcpy(path + len, "refs");
+		strcpy(path + len, PATH_REFS);
 		adjust_shared_perm(path);
-		strcpy(path + len, "refs/heads");
+		strcpy(path + len, PATH_REFS_HEADS);
 		adjust_shared_perm(path);
-		strcpy(path + len, "refs/tags");
+		strcpy(path + len, PATH_REFS_TAGS);
 		adjust_shared_perm(path);
 	}
 
@@ -231,7 +232,7 @@ static int create_default_files(const char *git_dir, const char *template_path)
 	strcpy(path + len, "HEAD");
 	reinit = !read_ref("HEAD", sha1);
 	if (!reinit) {
-		if (create_symref("HEAD", "refs/heads/master", NULL) < 0)
+		if (create_symref("HEAD", PATH_REFS_HEADS "master", NULL) < 0)
 			exit(1);
 	}
 
diff --git a/builtin-name-rev.c b/builtin-name-rev.c
index 36f1ba6..47a0f6d 100644
--- a/builtin-name-rev.c
+++ b/builtin-name-rev.c
@@ -85,7 +85,7 @@ static int name_ref(const char *path, const unsigned char *sha1, int flags, void
 	struct name_ref_data *data = cb_data;
 	int deref = 0;
 
-	if (data->tags_only && strncmp(path, "refs/tags/", 10))
+	if (data->tags_only && strncmp(path, PATH_REFS_TAGS, STRLEN_PATH_REFS_TAGS))
 		return 0;
 
 	if (data->ref_filter && fnmatch(data->ref_filter, path, 0))
@@ -101,10 +101,10 @@ static int name_ref(const char *path, const unsigned char *sha1, int flags, void
 	if (o && o->type == OBJ_COMMIT) {
 		struct commit *commit = (struct commit *)o;
 
-		if (!strncmp(path, "refs/heads/", 11))
-			path = path + 11;
-		else if (!strncmp(path, "refs/", 5))
-			path = path + 5;
+		if (!strncmp(path, PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS))
+			path = path + STRLEN_PATH_REFS_HEADS;
+		else if (!strncmp(path, PATH_REFS, STRLEN_PATH_REFS))
+			path = path + STRLEN_PATH_REFS;
 
 		name_rev(commit, xstrdup(path), 0, 0, deref);
 	}
diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index 3de9b3e..ac7543d 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -36,7 +36,7 @@ static int handle_one_ref(const char *path, const unsigned char *sha1,
 	/* Do not pack the symbolic refs */
 	if ((flags & REF_ISSYMREF))
 		return 0;
-	is_tag_ref = !strncmp(path, "refs/tags/", 10);
+	is_tag_ref = !strncmp(path, PATH_REFS_TAGS, STRLEN_PATH_REFS_TAGS);
 
 	/* ALWAYS pack refs that were already packed or are tags */
 	if (!cb->all && !is_tag_ref && !(flags & REF_ISPACKED))
diff --git a/builtin-push.c b/builtin-push.c
index c45649e..caeb9ae 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -30,9 +30,9 @@ static void add_refspec(const char *ref)
 static int expand_one_ref(const char *ref, const unsigned char *sha1, int flag, void *cb_data)
 {
 	/* Ignore the "refs/" at the beginning of the refname */
-	ref += 5;
+	ref += STRLEN_PATH_REFS;
 
-	if (!strncmp(ref, "tags/", 5))
+	if (!strncmp(ref, PATH_TAGS, STRLEN_PATH_TAGS))
 		add_refspec(xstrdup(ref));
 	return 0;
 }
@@ -123,9 +123,9 @@ static void set_refspecs(const char **refs, int nr)
 				int len;
 				if (nr <= ++i)
 					die("tag shorthand without <tag>");
-				len = strlen(refs[i]) + 11;
+				len = strlen(refs[i]) + STRLEN_PATH_REFS_TAGS + 1;
 				tag = xmalloc(len);
-				strcpy(tag, "refs/tags/");
+				strcpy(tag, PATH_REFS_TAGS);
 				strcat(tag, refs[i]);
 				ref = tag;
 			}
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 0d94e40..9995780 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -377,36 +377,36 @@ static int append_ref(const char *refname, const unsigned char *sha1,
 static int append_head_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	unsigned char tmp[20];
-	int ofs = 11;
-	if (strncmp(refname, "refs/heads/", ofs))
+	int ofs = STRLEN_PATH_REFS_HEADS;
+	if (strncmp(refname, PATH_REFS_HEADS, ofs))
 		return 0;
 	/* If both heads/foo and tags/foo exists, get_sha1 would
 	 * get confused.
 	 */
 	if (get_sha1(refname + ofs, tmp) || hashcmp(tmp, sha1))
-		ofs = 5;
+		ofs = STRLEN_PATH_REFS;
 	return append_ref(refname + ofs, sha1, 0);
 }
 
 static int append_remote_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	unsigned char tmp[20];
-	int ofs = 13;
-	if (strncmp(refname, "refs/remotes/", ofs))
+	int ofs = STRLEN_PATH_REFS_REMOTES;
+	if (strncmp(refname, PATH_REFS_REMOTES, ofs))
 		return 0;
 	/* If both heads/foo and tags/foo exists, get_sha1 would
 	 * get confused.
 	 */
 	if (get_sha1(refname + ofs, tmp) || hashcmp(tmp, sha1))
-		ofs = 5;
+		ofs = STRLEN_PATH_REFS;
 	return append_ref(refname + ofs, sha1, 0);
 }
 
 static int append_tag_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
-	if (strncmp(refname, "refs/tags/", 10))
+	if (strncmp(refname, PATH_REFS_TAGS, STRLEN_PATH_REFS_TAGS))
 		return 0;
-	return append_ref(refname + 5, sha1, 0);
+	return append_ref(refname + STRLEN_PATH_REFS, sha1, 0);
 }
 
 static const char *match_ref_pattern = NULL;
@@ -435,9 +435,9 @@ static int append_matching_ref(const char *refname, const unsigned char *sha1, i
 		return 0;
 	if (fnmatch(match_ref_pattern, tail, 0))
 		return 0;
-	if (!strncmp("refs/heads/", refname, 11))
+	if (!strncmp(PATH_REFS_HEADS, refname, STRLEN_PATH_REFS_HEADS))
 		return append_head_ref(refname, sha1, flag, cb_data);
-	if (!strncmp("refs/tags/", refname, 10))
+	if (!strncmp(PATH_REFS_TAGS, refname, STRLEN_PATH_REFS_TAGS))
 		return append_tag_ref(refname, sha1, flag, cb_data);
 	return append_ref(refname, sha1, 0);
 }
@@ -462,12 +462,12 @@ static int rev_is_head(char *head, int headlen, char *name,
 	if ((!head[0]) ||
 	    (head_sha1 && sha1 && hashcmp(head_sha1, sha1)))
 		return 0;
-	if (!strncmp(head, "refs/heads/", 11))
-		head += 11;
-	if (!strncmp(name, "refs/heads/", 11))
-		name += 11;
-	else if (!strncmp(name, "heads/", 6))
-		name += 6;
+	if (!strncmp(head, PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS))
+		head += STRLEN_PATH_REFS_HEADS;
+	if (!strncmp(name, PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS))
+		name += STRLEN_PATH_REFS_HEADS;
+	else if (!strncmp(name, PATH_HEADS, STRLEN_PATH_HEADS))
+		name += STRLEN_PATH_HEADS;
 	return !strcmp(head, name);
 }
 
@@ -777,7 +777,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 				has_head++;
 		}
 		if (!has_head) {
-			int pfxlen = strlen("refs/heads/");
+			int pfxlen = STRLEN_PATH_REFS_HEADS;
 			append_one_rev(head + pfxlen);
 		}
 	}
diff --git a/builtin-show-ref.c b/builtin-show-ref.c
index 853f13f..2c20cde 100644
--- a/builtin-show-ref.c
+++ b/builtin-show-ref.c
@@ -28,8 +28,8 @@ static int show_ref(const char *refname, const unsigned char *sha1, int flag, vo
 	if (tags_only || heads_only) {
 		int match;
 
-		match = heads_only && !strncmp(refname, "refs/heads/", 11);
-		match |= tags_only && !strncmp(refname, "refs/tags/", 10);
+		match = heads_only && !strncmp(refname, PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS);
+		match |= tags_only && !strncmp(refname, PATH_REFS_TAGS, STRLEN_PATH_REFS_TAGS);
 		if (!match)
 			return 0;
 	}
@@ -224,7 +224,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
 		unsigned char sha1[20];
 
 		while (*pattern) {
-			if (!strncmp(*pattern, "refs/", 5) &&
+			if (!strncmp(*pattern, PATH_REFS, STRLEN_PATH_REFS) &&
 			    resolve_ref(*pattern, sha1, 1, NULL)) {
 				if (!quiet)
 					show_one(*pattern, sha1);
diff --git a/connect.c b/connect.c
index 7844888..8701de0 100644
--- a/connect.c
+++ b/connect.c
@@ -11,23 +11,23 @@ static int check_ref(const char *name, int len, unsigned int flags)
 	if (!flags)
 		return 1;
 
-	if (len < 5 || memcmp(name, "refs/", 5))
+	if (len < STRLEN_PATH_REFS || memcmp(name, PATH_REFS, STRLEN_PATH_REFS))
 		return 0;
 
 	/* Skip the "refs/" part */
-	name += 5;
-	len -= 5;
+	name += STRLEN_PATH_REFS;
+	len -= STRLEN_PATH_REFS;
 
 	/* REF_NORMAL means that we don't want the magic fake tag refs */
 	if ((flags & REF_NORMAL) && check_ref_format(name) < 0)
 		return 0;
 
 	/* REF_HEADS means that we want regular branch heads */
-	if ((flags & REF_HEADS) && !memcmp(name, "heads/", 6))
+	if ((flags & REF_HEADS) && !memcmp(name, PATH_HEADS, STRLEN_PATH_HEADS))
 		return 1;
 
 	/* REF_TAGS means that we want tags */
-	if ((flags & REF_TAGS) && !memcmp(name, "tags/", 5))
+	if ((flags & REF_TAGS) && !memcmp(name, PATH_TAGS, STRLEN_PATH_TAGS))
 		return 1;
 
 	/* All type bits clear means that we are ok with anything */
@@ -196,8 +196,8 @@ static int count_refspec_match(const char *pattern,
 		 */
 		if (namelen != patlen &&
 		    patlen != namelen - 5 &&
-		    strncmp(name, "refs/heads/", 11) &&
-		    strncmp(name, "refs/tags/", 10)) {
+		    strncmp(name, PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS) &&
+		    strncmp(name, PATH_REFS_TAGS, STRLEN_PATH_REFS_TAGS)) {
 			/* We want to catch the case where only weak
 			 * matches are found and there are multiple
 			 * matches, and where more than one strong
@@ -284,7 +284,7 @@ static int match_explicit_refs(struct ref *src, struct ref *dst,
 		case 1:
 			break;
 		case 0:
-			if (!memcmp(rs[i].dst, "refs/", 5)) {
+			if (!memcmp(rs[i].dst, PATH_REFS, STRLEN_PATH_REFS)) {
 				int len = strlen(rs[i].dst) + 1;
 				matched_dst = xcalloc(1, sizeof(*dst) + len);
 				memcpy(matched_dst->name, rs[i].dst, len);
@@ -305,7 +305,7 @@ static int match_explicit_refs(struct ref *src, struct ref *dst,
 				errs = 1;
 				error("dst refspec %s does not match any "
 				      "existing ref on the remote and does "
-				      "not start with refs/.", rs[i].dst);
+				      "not start with " PATH_REFS ".", rs[i].dst);
 			}
 			break;
 		default:
diff --git a/fetch-pack.c b/fetch-pack.c
index c787106..14a74d2 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -342,11 +342,11 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
 
 	for (ref = *refs; ref; ref = next) {
 		next = ref->next;
-		if (!memcmp(ref->name, "refs/", 5) &&
-		    check_ref_format(ref->name + 5))
+		if (!memcmp(ref->name, PATH_REFS, STRLEN_PATH_REFS) &&
+		    check_ref_format(ref->name + STRLEN_PATH_REFS))
 			; /* trash */
 		else if (fetch_all &&
-			 (!depth || strncmp(ref->name, "refs/tags/", 10) )) {
+			 (!depth || strncmp(ref->name, PATH_REFS_TAGS, STRLEN_PATH_REFS_TAGS) )) {
 			*newtail = ref;
 			ref->next = NULL;
 			newtail = &ref->next;
diff --git a/http-fetch.c b/http-fetch.c
index 9f790a0..79ef85a 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -3,6 +3,7 @@
 #include "pack.h"
 #include "fetch.h"
 #include "http.h"
+#include "refs.h"
 
 #define PREV_BUF_SIZE 4096
 #define RANGE_HEADER_SIZE 30
@@ -938,14 +939,14 @@ static char *quote_ref_url(const char *base, const char *ref)
 	int len, baselen, ch;
 
 	baselen = strlen(base);
-	len = baselen + 6; /* "refs/" + NUL */
+	len = baselen + STRLEN_PATH_REFS + 1; /* "refs/" + NUL */
 	for (cp = ref; (ch = *cp) != 0; cp++, len++)
 		if (needs_quote(ch))
 			len += 2; /* extra two hex plus replacement % */
 	qref = xmalloc(len);
 	memcpy(qref, base, baselen);
-	memcpy(qref + baselen, "refs/", 5);
-	for (cp = ref, dp = qref + baselen + 5; (ch = *cp) != 0; cp++) {
+	memcpy(qref + baselen, PATH_REFS, STRLEN_PATH_REFS);
+	for (cp = ref, dp = qref + baselen + STRLEN_PATH_REFS; (ch = *cp) != 0; cp++) {
 		if (needs_quote(ch)) {
 			*dp++ = '%';
 			*dp++ = hex((ch >> 4) & 0xF);
diff --git a/http-push.c b/http-push.c
index b128c01..37d2f50 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1922,7 +1922,7 @@ static void get_local_heads(void)
 static void get_dav_remote_heads(void)
 {
 	remote_tail = &remote_refs;
-	remote_ls("refs/", (PROCESS_FILES | PROCESS_DIRS | RECURSIVE), process_ls_ref, NULL);
+	remote_ls(PATH_REFS, (PROCESS_FILES | PROCESS_DIRS | RECURSIVE), process_ls_ref, NULL);
 }
 
 static int is_zero_sha1(const unsigned char *sha1)
@@ -2066,7 +2066,7 @@ static void update_remote_info_refs(struct remote_lock *lock)
 	buffer.buffer = xcalloc(1, 4096);
 	buffer.size = 4096;
 	buffer.posn = 0;
-	remote_ls("refs/", (PROCESS_FILES | RECURSIVE),
+	remote_ls(PATH_REFS, (PROCESS_FILES | RECURSIVE),
 		  add_remote_info_ref, &buffer);
 	if (!aborted) {
 		if_header = xmalloc(strlen(lock->token) + 25);
diff --git a/local-fetch.c b/local-fetch.c
index 7cfe8b3..7b80a31 100644
--- a/local-fetch.c
+++ b/local-fetch.c
@@ -4,6 +4,7 @@
 #include "cache.h"
 #include "commit.h"
 #include "fetch.h"
+#include "refs.h"
 
 static int use_link;
 static int use_symlink;
@@ -174,7 +175,7 @@ int fetch_ref(char *ref, unsigned char *sha1)
 	int ifd;
 
 	if (ref_name_start < 0) {
-		sprintf(filename, "%s/refs/", path);
+		sprintf(filename, "%s/" PATH_REFS, path);
 		ref_name_start = strlen(filename);
 	}
 	strcpy(filename + ref_name_start, ref);
diff --git a/path.c b/path.c
index c5d25a4..dfbae16 100644
--- a/path.c
+++ b/path.c
@@ -11,6 +11,7 @@
  * which is what it's designed for.
  */
 #include "cache.h"
+#include "refs.h"
 
 static char bad_path[] = "/bad-path/";
 
@@ -103,7 +104,7 @@ int validate_headref(const char *path)
 	/* Make sure it is a "refs/.." symlink */
 	if (S_ISLNK(st.st_mode)) {
 		len = readlink(path, buffer, sizeof(buffer)-1);
-		if (len >= 5 && !memcmp("refs/", buffer, 5))
+		if (len >= 5 && !memcmp(PATH_REFS, buffer, STRLEN_PATH_REFS))
 			return 0;
 		return -1;
 	}
@@ -127,7 +128,7 @@ int validate_headref(const char *path)
 		len -= 4;
 		while (len && isspace(*buf))
 			buf++, len--;
-		if (len >= 5 && !memcmp("refs/", buf, 5))
+		if (len >= STRLEN_PATH_REFS && !memcmp(PATH_REFS, buf, STRLEN_PATH_REFS))
 			return 0;
 	}
 
diff --git a/receive-pack.c b/receive-pack.c
index 7311c82..6fd9f60 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -109,7 +109,7 @@ static int update(struct command *cmd)
 	struct ref_lock *lock;
 
 	cmd->error_string = NULL;
-	if (!strncmp(name, "refs/", 5) && check_ref_format(name + 5)) {
+	if (!strncmp(name, PATH_REFS, STRLEN_PATH_REFS) && check_ref_format(name + STRLEN_PATH_REFS)) {
 		cmd->error_string = "funny refname";
 		return error("refusing to create funny ref '%s' locally",
 			     name);
@@ -125,7 +125,7 @@ static int update(struct command *cmd)
 	}
 	if (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&
 	    !is_null_sha1(old_sha1) &&
-	    !strncmp(name, "refs/heads/", 11)) {
+	    !strncmp(name, PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS)) {
 		struct commit *old_commit, *new_commit;
 		struct commit_list *bases, *ent;
 
diff --git a/reflog-walk.c b/reflog-walk.c
index c983858..c05a404 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -55,11 +55,11 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)
 	}
 	if (reflogs->nr == 0) {
 		int len = strlen(ref);
-		char *refname = xmalloc(len + 12);
-		sprintf(refname, "refs/%s", ref);
+		char *refname = xmalloc(len + STRLEN_PATH_REFS_HEADS + 1);
+		sprintf(refname, PATH_REFS "%s", ref);
 		for_each_reflog_ent(refname, read_one_reflog, reflogs);
 		if (reflogs->nr == 0) {
-			sprintf(refname, "refs/heads/%s", ref);
+			sprintf(refname, PATH_REFS_HEADS "%s", ref);
 			for_each_reflog_ent(refname, read_one_reflog, reflogs);
 		}
 		free(refname);
diff --git a/refs.c b/refs.c
index 6387703..997b360 100644
--- a/refs.c
+++ b/refs.c
@@ -261,7 +261,7 @@ const char *resolve_ref(const char *ref, unsigned char *sha1, int reading, int *
 		/* Follow "normalized" - ie "refs/.." symlinks by hand */
 		if (S_ISLNK(st.st_mode)) {
 			len = readlink(path, buffer, sizeof(buffer)-1);
-			if (len >= 5 && !memcmp("refs/", buffer, 5)) {
+			if (len >= STRLEN_PATH_REFS && !memcmp(PATH_REFS, buffer, STRLEN_PATH_REFS)) {
 				buffer[len] = 0;
 				strcpy(ref_buffer, buffer);
 				ref = ref_buffer;
@@ -413,22 +413,22 @@ int head_ref(each_ref_fn fn, void *cb_data)
 
 int for_each_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref("refs/", fn, 0, cb_data);
+	return do_for_each_ref(PATH_REFS, fn, 0, cb_data);
 }
 
 int for_each_tag_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref("refs/tags/", fn, 10, cb_data);
+	return do_for_each_ref(PATH_REFS_TAGS, fn, STRLEN_PATH_REFS_TAGS, cb_data);
 }
 
 int for_each_branch_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref("refs/heads/", fn, 11, cb_data);
+	return do_for_each_ref(PATH_REFS_HEADS, fn, STRLEN_PATH_REFS_HEADS, cb_data);
 }
 
 int for_each_remote_ref(each_ref_fn fn, void *cb_data)
 {
-	return do_for_each_ref("refs/remotes/", fn, 13, cb_data);
+	return do_for_each_ref(PATH_REFS_REMOTES, fn, STRLEN_PATH_REFS_REMOTES, cb_data);
 }
 
 /* NEEDSWORK: This is only used by ssh-upload and it should go; the
@@ -440,7 +440,7 @@ int get_ref_sha1(const char *ref, unsigned char *sha1)
 {
 	if (check_ref_format(ref))
 		return -1;
-	return read_ref(mkpath("refs/%s", ref), sha1);
+	return read_ref(mkpath(PATH_REFS "%s", ref), sha1);
 }
 
 /*
@@ -657,7 +657,7 @@ struct ref_lock *lock_ref_sha1(const char *ref, const unsigned char *old_sha1)
 	char refpath[PATH_MAX];
 	if (check_ref_format(ref))
 		return NULL;
-	strcpy(refpath, mkpath("refs/%s", ref));
+	strcpy(refpath, mkpath(PATH_REFS "%s", ref));
 	return lock_ref_sha1_basic(refpath, old_sha1, NULL);
 }
 
@@ -828,12 +828,12 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 		goto rollback;
 	}
 
-	if (!strncmp(oldref, "refs/heads/", 11) &&
-			!strncmp(newref, "refs/heads/", 11)) {
+	if (!strncmp(oldref, PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS) &&
+			!strncmp(newref, PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS)) {
 		char oldsection[1024], newsection[1024];
 
-		snprintf(oldsection, 1024, "branch.%s", oldref + 11);
-		snprintf(newsection, 1024, "branch.%s", newref + 11);
+		snprintf(oldsection, 1024, "branch.%s", oldref + STRLEN_PATH_REFS_HEADS);
+		snprintf(newsection, 1024, "branch.%s", newref + STRLEN_PATH_REFS_HEADS);
 		if (git_config_rename_section(oldsection, newsection) < 0)
 			return 1;
 	}
@@ -894,8 +894,8 @@ static int log_ref_write(const char *ref_name, const unsigned char *old_sha1,
 	log_file = git_path("logs/%s", ref_name);
 
 	if (log_all_ref_updates &&
-	    (!strncmp(ref_name, "refs/heads/", 11) ||
-	     !strncmp(ref_name, "refs/remotes/", 13) ||
+	    (!strncmp(ref_name,  PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS) ||
+	     !strncmp(ref_name, PATH_REFS_REMOTES, STRLEN_PATH_REFS_HEADS) ||
 	     !strcmp(ref_name, "HEAD"))) {
 		if (safe_create_leading_directories(log_file) < 0)
 			return error("unable to create directory for %s",
diff --git a/refs.h b/refs.h
index acedffc..a986b42 100644
--- a/refs.h
+++ b/refs.h
@@ -13,6 +13,23 @@ struct ref_lock {
 #define REF_ISSYMREF 01
 #define REF_ISPACKED 02
 
+#define PATH_OBJECTS             "objects/"
+#define STRLEN_PATH_OBJECTS      8
+#define PATH_REFS                "refs/"
+#define STRLEN_PATH_REFS         5
+#define PATH_HEADS               "heads/"
+#define STRLEN_PATH_HEADS        6
+#define PATH_TAGS                "tags/"
+#define STRLEN_PATH_TAGS         5
+#define PATH_REMOTES             "remotes/"
+#define STRLEN_PATH_REMOTES      8
+#define PATH_REFS_HEADS          PATH_REFS PATH_HEADS
+#define STRLEN_PATH_REFS_HEADS   (STRLEN_PATH_REFS+STRLEN_PATH_HEADS)
+#define PATH_REFS_TAGS           PATH_REFS PATH_TAGS
+#define STRLEN_PATH_REFS_TAGS    (STRLEN_PATH_REFS+STRLEN_PATH_TAGS)
+#define PATH_REFS_REMOTES        PATH_REFS PATH_REMOTES
+#define STRLEN_PATH_REFS_REMOTES (STRLEN_PATH_REFS+STRLEN_PATH_REMOTES)
+
 /*
  * Calls the specified function for each ref file until it returns nonzero,
  * and returns the value
diff --git a/setup.c b/setup.c
index e9d3f5a..782fc07 100644
--- a/setup.c
+++ b/setup.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "refs.h"
 
 const char *prefix_path(const char *prefix, int len, const char *path)
 {
@@ -154,12 +155,12 @@ static int is_git_directory(const char *suspect)
 			return 0;
 	}
 	else {
-		strcpy(path + len, "/objects");
+		strcpy(path + len, "/" PATH_OBJECTS);
 		if (access(path, X_OK))
 			return 0;
 	}
 
-	strcpy(path + len, "/refs");
+	strcpy(path + len, "/" PATH_REFS);
 	if (access(path, X_OK))
 		return 0;
 
diff --git a/sha1_name.c b/sha1_name.c
index a7efa96..add6123 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -237,11 +237,11 @@ static int ambiguous_path(const char *path, int len)
 
 static const char *ref_fmt[] = {
 	"%.*s",
-	"refs/%.*s",
-	"refs/tags/%.*s",
-	"refs/heads/%.*s",
-	"refs/remotes/%.*s",
-	"refs/remotes/%.*s/HEAD",
+	PATH_REFS "%.*s",
+	PATH_REFS_TAGS "%.*s",
+	PATH_REFS_HEADS "%.*s",
+	PATH_REFS_REMOTES "%.*s",
+	PATH_REFS_REMOTES "%.*s/HEAD",
 	NULL
 };
 
diff --git a/wt-status.c b/wt-status.c
index 2879c3d..9f2705c 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -7,6 +7,7 @@
 #include "diff.h"
 #include "revision.h"
 #include "diffcore.h"
+#include "refs.h"
 
 int wt_status_use_color = 0;
 static char wt_status_colors[][COLOR_MAXLEN] = {
@@ -298,8 +299,8 @@ void wt_status_print(struct wt_status *s)
 	if (s->branch) {
 		const char *on_what = "On branch ";
 		const char *branch_name = s->branch;
-		if (!strncmp(branch_name, "refs/heads/", 11))
-			branch_name += 11;
+		if (!strncmp(branch_name, PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS))
+			branch_name += STRLEN_PATH_REFS_HEADS;
 		else if (!strcmp(branch_name, "HEAD")) {
 			branch_name = "";
 			on_what = "Not currently on any branch.";
-- 
1.5.0.rc4.gb4d2

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-19 18:39 [PATCH] Change "refs/" references to symbolic constants Andy Parkins
@ 2007-02-19 18:55 ` Bill Lear
  2007-02-19 20:01   ` [PATCH] Replace literal STRLEN_ #defines in refs.h with compiler evaluated expressions Andy Parkins
  2007-02-19 20:50   ` [PATCH] Change "refs/" references to symbolic constants Krzysztof Halasa
  2007-02-19 20:07 ` Junio C Hamano
  1 sibling, 2 replies; 30+ messages in thread
From: Bill Lear @ 2007-02-19 18:55 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git

On Monday, February 19, 2007 at 18:39:05 (+0000) Andy Parkins writes:
>...
>+#define PATH_REMOTES             "remotes/"
>+#define STRLEN_PATH_REMOTES      8
>...

Would this be less error-prone, and just as efficient?:

#define PATH_REMOTES "remotes/"
#define LIT_STRLEN(S) ((sizeof(S) / sizeof(S[0])) -1)
#define STRLEN_PATH_REMOTES LIT_STRLEN(PATH_REMOTES)


Bill

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH] Replace literal STRLEN_ #defines in refs.h with compiler evaluated expressions
  2007-02-19 18:55 ` Bill Lear
@ 2007-02-19 20:01   ` Andy Parkins
  2007-02-19 20:50   ` [PATCH] Change "refs/" references to symbolic constants Krzysztof Halasa
  1 sibling, 0 replies; 30+ messages in thread
From: Andy Parkins @ 2007-02-19 20:01 UTC (permalink / raw)
  To: git

Bill Lear pointed out that the following:

 #define PATH_REMOTES             "remotes/"
 #define STRLEN_PATH_REMOTES      8

Could be replaced by the less error-prone

 #define PATH_REMOTES "remotes/"
 #define LIT_STRLEN(S) ((sizeof(S) / sizeof(S[0])) -1)
 #define STRLEN_PATH_REMOTES LIT_STRLEN(PATH_REMOTES)

which is what this patch does.

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---

On top of my previous patch.


 refs.h |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/refs.h b/refs.h
index a986b42..6761095 100644
--- a/refs.h
+++ b/refs.h
@@ -13,16 +13,17 @@ struct ref_lock {
 #define REF_ISSYMREF 01
 #define REF_ISPACKED 02
 
+#define LIT_STRLEN(S)            ((sizeof(S) / sizeof(S[0])) -1)
 #define PATH_OBJECTS             "objects/"
-#define STRLEN_PATH_OBJECTS      8
+#define STRLEN_PATH_OBJECTS      LIT_STRLEN(PATH_OBJECTS)
 #define PATH_REFS                "refs/"
-#define STRLEN_PATH_REFS         5
+#define STRLEN_PATH_REFS         LIT_STRLEN(PATH_REFS)
 #define PATH_HEADS               "heads/"
-#define STRLEN_PATH_HEADS        6
+#define STRLEN_PATH_HEADS        LIT_STRLEN(PATH_HEADS)
 #define PATH_TAGS                "tags/"
-#define STRLEN_PATH_TAGS         5
+#define STRLEN_PATH_TAGS         LIT_STRLEN(PATH_TAGS)
 #define PATH_REMOTES             "remotes/"
-#define STRLEN_PATH_REMOTES      8
+#define STRLEN_PATH_REMOTES      LIT_STRLEN(PATH_REMOTES)
 #define PATH_REFS_HEADS          PATH_REFS PATH_HEADS
 #define STRLEN_PATH_REFS_HEADS   (STRLEN_PATH_REFS+STRLEN_PATH_HEADS)
 #define PATH_REFS_TAGS           PATH_REFS PATH_TAGS
-- 
1.5.0.rc4.gb4d2

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-19 18:39 [PATCH] Change "refs/" references to symbolic constants Andy Parkins
  2007-02-19 18:55 ` Bill Lear
@ 2007-02-19 20:07 ` Junio C Hamano
  2007-02-19 20:12   ` Shawn O. Pearce
                     ` (3 more replies)
  1 sibling, 4 replies; 30+ messages in thread
From: Junio C Hamano @ 2007-02-19 20:07 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git

Andy Parkins <andyparkins@gmail.com> writes:

> Changed repeated use of the same constants for the ref paths to be
> symbolic constants.  I've defined them in refs.h
>
>   refs/ is now PATH_REFS
>   refs/heads/ is now PATH_REFS_HEADS
>   refs/tags/ is now PATH_REFS_TAGS
>   refs/remotes/ is now PATH_REFS_REMOTES

Your example:

> ...  This has clarified the code in some places; for
> example:
>
>  - len = strlen(refs[i]) + 11;
>  + len = strlen(refs[i]) + STRLEN_PATH_REFS_TAGS + 1;

shows that you've carefully looked at what the code does,
instead of mindlessly replacing, which is a very good sign, but
how much testing has this seen, I wonder.

> diff --git a/builtin-describe.c b/builtin-describe.c
> index bcc6456..0f78363 100644
> --- a/builtin-describe.c
> +++ b/builtin-describe.c
> @@ -52,7 +52,7 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
>  	 * If --tags, then any tags are used.
>  	 * Otherwise only annotated tags are used.
>  	 */
> -	if (!strncmp(path, "refs/tags/", 10)) {
> +	if (!strncmp(path, PATH_TAGS, STRLEN_PATH_TAGS)) {
>  		if (object->type == OBJ_TAG)
>  			prio = 2;
>  		else

This is PATH_REFS_TAGS isn't it?

> @@ -231,7 +232,7 @@ static int create_default_files(const char *git_dir, const char *template_path)
>  	strcpy(path + len, "HEAD");
>  	reinit = !read_ref("HEAD", sha1);
>  	if (!reinit) {
> -		if (create_symref("HEAD", "refs/heads/master", NULL) < 0)
> +		if (create_symref("HEAD", PATH_REFS_HEADS "master", NULL) < 0)
>  			exit(1);
>  	}
>  

I mildly mind this one, as it hurts grep-ability.  I know this is one
of the the only two places in git that 'master' branch is treated
specially (and I think we would like to keep it that way --- that's
why I want to be able to grep for "refs/heads/master" and see very few
hits), so introducing PATH_REFS_HEADS_MASTER is probably not very
productive either, but...  hmmmm.

> diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
> index 3de9b3e..ac7543d 100644
> --- a/builtin-pack-refs.c
> +++ b/builtin-pack-refs.c
> @@ -36,7 +36,7 @@ static int handle_one_ref(const char *path, const unsigned char *sha1,
>  	/* Do not pack the symbolic refs */
>  	if ((flags & REF_ISSYMREF))
>  		return 0;
> -	is_tag_ref = !strncmp(path, "refs/tags/", 10);
> +	is_tag_ref = !strncmp(path, PATH_REFS_TAGS, STRLEN_PATH_REFS_TAGS);

These repeated strncmp(p, X, STRLEN_X) almost makes me wonder if we
want to introduce:

	inline int prefixcmp(a, b)
        {
        	return (strncmp(a, b, strlen(b));
        }

with clever preprocessor optimization to have compiler do strlen()
when b is a string literal.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-19 20:07 ` Junio C Hamano
@ 2007-02-19 20:12   ` Shawn O. Pearce
  2007-02-19 22:08   ` Johannes Schindelin
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 30+ messages in thread
From: Shawn O. Pearce @ 2007-02-19 20:12 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andy Parkins, git

Junio C Hamano <junkio@cox.net> wrote:
> > diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
> > index 3de9b3e..ac7543d 100644
> > --- a/builtin-pack-refs.c
> > +++ b/builtin-pack-refs.c
> > @@ -36,7 +36,7 @@ static int handle_one_ref(const char *path, const unsigned char *sha1,
> >  	/* Do not pack the symbolic refs */
> >  	if ((flags & REF_ISSYMREF))
> >  		return 0;
> > -	is_tag_ref = !strncmp(path, "refs/tags/", 10);
> > +	is_tag_ref = !strncmp(path, PATH_REFS_TAGS, STRLEN_PATH_REFS_TAGS);
> 
> These repeated strncmp(p, X, STRLEN_X) almost makes me wonder if we
> want to introduce:
> 
> 	inline int prefixcmp(a, b)
>         {
>         	return (strncmp(a, b, strlen(b));
>         }
> 
> with clever preprocessor optimization to have compiler do strlen()
> when b is a string literal.

This may be worthwhile.  We use strncmp so often in Git that I
tend to write strncmp even when I mean strcmp.  Yes, my fingers are
trained to write strncmp, and only strncmp...  *sigh* At least the
compiler checks for me.  ;-)

-- 
Shawn.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-19 18:55 ` Bill Lear
  2007-02-19 20:01   ` [PATCH] Replace literal STRLEN_ #defines in refs.h with compiler evaluated expressions Andy Parkins
@ 2007-02-19 20:50   ` Krzysztof Halasa
  2007-02-19 20:56     ` Shawn O. Pearce
  1 sibling, 1 reply; 30+ messages in thread
From: Krzysztof Halasa @ 2007-02-19 20:50 UTC (permalink / raw)
  To: Bill Lear; +Cc: Andy Parkins, git

Bill Lear <rael@zopyra.com> writes:

> Would this be less error-prone, and just as efficient?:
>
> #define PATH_REMOTES "remotes/"
> #define LIT_STRLEN(S) ((sizeof(S) / sizeof(S[0])) -1)
> #define STRLEN_PATH_REMOTES LIT_STRLEN(PATH_REMOTES)

sizeof(char) is always 1.
-- 
Krzysztof Halasa

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-19 20:50   ` [PATCH] Change "refs/" references to symbolic constants Krzysztof Halasa
@ 2007-02-19 20:56     ` Shawn O. Pearce
  0 siblings, 0 replies; 30+ messages in thread
From: Shawn O. Pearce @ 2007-02-19 20:56 UTC (permalink / raw)
  To: Krzysztof Halasa; +Cc: Bill Lear, Andy Parkins, git

Krzysztof Halasa <khc@pm.waw.pl> wrote:
> Bill Lear <rael@zopyra.com> writes:
> 
> > Would this be less error-prone, and just as efficient?:
> >
> > #define PATH_REMOTES "remotes/"
> > #define LIT_STRLEN(S) ((sizeof(S) / sizeof(S[0])) -1)
> > #define STRLEN_PATH_REMOTES LIT_STRLEN(PATH_REMOTES)
> 
> sizeof(char) is always 1.

Except when someone does something silly, like say:

	#define char double

:-)

-- 
Shawn.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-19 20:07 ` Junio C Hamano
  2007-02-19 20:12   ` Shawn O. Pearce
@ 2007-02-19 22:08   ` Johannes Schindelin
  2007-02-20  8:41   ` Andy Parkins
  2007-02-20  9:42   ` Andy Parkins
  3 siblings, 0 replies; 30+ messages in thread
From: Johannes Schindelin @ 2007-02-19 22:08 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andy Parkins, git

Hi,

On Mon, 19 Feb 2007, Junio C Hamano wrote:

> These repeated strncmp(p, X, STRLEN_X) almost makes me wonder if we want 
> to introduce:
> 
> 	inline int prefixcmp(a, b)
>         {
>         	return (strncmp(a, b, strlen(b));
>         }
> 
> with clever preprocessor optimization to have compiler do strlen() when 
> b is a string literal.

I am in favour of that. BTW I remember that Han-Wen suggested this some 
time ago, too.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-19 20:07 ` Junio C Hamano
  2007-02-19 20:12   ` Shawn O. Pearce
  2007-02-19 22:08   ` Johannes Schindelin
@ 2007-02-20  8:41   ` Andy Parkins
  2007-02-20  9:04     ` Junio C Hamano
  2007-02-20  9:42   ` Andy Parkins
  3 siblings, 1 reply; 30+ messages in thread
From: Andy Parkins @ 2007-02-20  8:41 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

On Monday 2007 February 19 20:07, Junio C Hamano wrote:

> >  - len = strlen(refs[i]) + 11;
> >  + len = strlen(refs[i]) + STRLEN_PATH_REFS_TAGS + 1;
>
> shows that you've carefully looked at what the code does,
> instead of mindlessly replacing, which is a very good sign, but
> how much testing has this seen, I wonder.

Not huge quantities.  The patch is so wide-spread, that I've checked it 
compiles, and I've run with it for a bit.  However, I could not swear to 
having checked every path...


> > diff --git a/builtin-describe.c b/builtin-describe.c
> > index bcc6456..0f78363 100644
> > --- a/builtin-describe.c
> > +++ b/builtin-describe.c
> > @@ -52,7 +52,7 @@ static int get_name(const char *path, const unsigned
> > char *sha1, int flag, void * If --tags, then any tags are used.
> >  	 * Otherwise only annotated tags are used.
> >  	 */
> > -	if (!strncmp(path, "refs/tags/", 10)) {
> > +	if (!strncmp(path, PATH_TAGS, STRLEN_PATH_TAGS)) {
> >  		if (object->type == OBJ_TAG)
> >  			prio = 2;
> >  		else
>
> This is PATH_REFS_TAGS isn't it?

<shame> Yes.

> I mildly mind this one, as it hurts grep-ability.  I know this is one
> of the the only two places in git that 'master' branch is treated
> specially (and I think we would like to keep it that way --- that's

The output of 
 $ git-grep "master" -- '*.c' '*.h'
Is not that big, so those special treatments can be easily found irrespective 
of my patch.

> why I want to be able to grep for "refs/heads/master" and see very few
> hits), so introducing PATH_REFS_HEADS_MASTER is probably not very
> productive either, but...  hmmmm.

As I've said before, I'm against /any/ special treatment of master other than 
as a default branch name in a newly initialised repository.  
PATH_REFS_HEADS_MASTER is closer to master not being special than without so 
I'd be in favour of that.

> These repeated strncmp(p, X, STRLEN_X) almost makes me wonder if we
> want to introduce:
>
> 	inline int prefixcmp(a, b)
>         {
>         	return (strncmp(a, b, strlen(b));
>         }
>
> with clever preprocessor optimization to have compiler do strlen()
> when b is a string literal.

Wow; that would be clever - regardless of whether this patch is acceptable or 
not.


Andy
-- 
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@gmail.com

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-20  8:41   ` Andy Parkins
@ 2007-02-20  9:04     ` Junio C Hamano
  0 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2007-02-20  9:04 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git

Andy Parkins <andyparkins@gmail.com> writes:

> As I've said before, I'm against /any/ special treatment of
> master other than as a default branch name in a newly
> initialised repository.  PATH_REFS_HEADS_MASTER is closer to
> master not being special than without so I'd be in favour of
> that.

Ok.

>> These repeated strncmp(p, X, STRLEN_X) almost makes me wonder if we
>> want to introduce:
>>
>> 	inline int prefixcmp(a, b)
>>         {
>>         	return (strncmp(a, b, strlen(b));
>>         }
>>
>> with clever preprocessor optimization to have compiler do strlen()
>> when b is a string literal.
>
> Wow; that would be clever - regardless of whether this patch
> is acceptable or not.

Actually GCC seems to be clever enough.  It appears that I do
not even have to do prefixcmp_0() below; prefixcmp_1() seems to
generate good code, with GCC 4.1.2 prerelease on my x86_64.

-- >8 --
#include <string.h>
static inline int prefixcmp_0(const char *a, const char *b)
{
	if (__builtin_constant_p(b))
		return strncmp(a, b, sizeof(b) - 1);
	else
		return strncmp(a, b, strlen(b));
}

static inline prefixcmp_1(const char *a, const char *b)
{
	return strncmp(a, b, strlen(b));
}

void foo(const char *s, const char *t, int *a, int *b, int *c, int *d)
{
	*a = prefixcmp_0(s, "abcdefg");
	*b = prefixcmp_0(s, t);
	*c = prefixcmp_1(s, "ABCDEFGH");
	*d = prefixcmp_1(s, t);
}
-- 8< --

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-19 20:07 ` Junio C Hamano
                     ` (2 preceding siblings ...)
  2007-02-20  8:41   ` Andy Parkins
@ 2007-02-20  9:42   ` Andy Parkins
  2007-02-20  9:50     ` Junio C Hamano
                       ` (4 more replies)
  3 siblings, 5 replies; 30+ messages in thread
From: Andy Parkins @ 2007-02-20  9:42 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

On Monday 2007 February 19 20:07, Junio C Hamano wrote:

> > -	if (!strncmp(path, "refs/tags/", 10)) {
> > +	if (!strncmp(path, PATH_TAGS, STRLEN_PATH_TAGS)) {
> >  		if (object->type == OBJ_TAG)
> >  			prio = 2;
> >  		else
>
> This is PATH_REFS_TAGS isn't it?

I'm uncomfortable that this one managed to get through; in view of this 
mistake if you are thinking of applying this - don't.  I'm going to review 
the patch itself line by line.

Also - I should learn how to run the tests - is "make test" good enough or is 
there something special I should do?


Andy
-- 
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@gmail.com

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-20  9:42   ` Andy Parkins
@ 2007-02-20  9:50     ` Junio C Hamano
  2007-02-20 10:21       ` Andy Parkins
  2007-02-20  9:51     ` [PATCH 1/4] Add prefixcmp() Junio C Hamano
                       ` (3 subsequent siblings)
  4 siblings, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2007-02-20  9:50 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git

Andy Parkins <andyparkins@gmail.com> writes:

> On Monday 2007 February 19 20:07, Junio C Hamano wrote:
>
>> > -	if (!strncmp(path, "refs/tags/", 10)) {
>> > +	if (!strncmp(path, PATH_TAGS, STRLEN_PATH_TAGS)) {
>> >  		if (object->type == OBJ_TAG)
>> >  			prio = 2;
>> >  		else
>>
>> This is PATH_REFS_TAGS isn't it?
>
> I'm uncomfortable that this one managed to get through; in view of this 
> mistake if you are thinking of applying this - don't.  I'm going to review 
> the patch itself line by line.

I'd send the prefixcmp() patches first, as yours would touch the
same lines.

> Also - I should learn how to run the tests - is "make test"
> good enough or is there something special I should do?

Should be enough.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH 1/4] Add prefixcmp()
  2007-02-20  9:42   ` Andy Parkins
  2007-02-20  9:50     ` Junio C Hamano
@ 2007-02-20  9:51     ` Junio C Hamano
  2007-02-20 10:04       ` David Kågedal
  2007-02-20  9:53     ` [PATCH 2/4] Mechanical conversion to use prefixcmp() Junio C Hamano
                       ` (2 subsequent siblings)
  4 siblings, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2007-02-20  9:51 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git

We have too many strncmp(a, b, strlen(b)).

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 git-compat-util.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/git-compat-util.h b/git-compat-util.h
index 9863cf6..0a9ac56 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -279,4 +279,9 @@ static inline int sane_case(int x, int high)
 	return x;
 }
 
+static inline int prefixcmp(const char *a, const char *b)
+{
+	return strncmp(a, b, strlen(b));
+}
+
 #endif
-- 
1.5.0.1.571.ge5a1a

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 2/4] Mechanical conversion to use prefixcmp()
  2007-02-20  9:42   ` Andy Parkins
  2007-02-20  9:50     ` Junio C Hamano
  2007-02-20  9:51     ` [PATCH 1/4] Add prefixcmp() Junio C Hamano
@ 2007-02-20  9:53     ` Junio C Hamano
  2007-02-20 10:19       ` Junio C Hamano
  2007-02-20 11:53       ` Johannes Schindelin
  2007-02-20  9:54     ` [PATCH 3/4] prefixcmp(): fix-up mechanical conversion Junio C Hamano
  2007-02-20  9:55     ` [PATCH 4/4] prefixcmp(): fix-up leftover strncmp() Junio C Hamano
  4 siblings, 2 replies; 30+ messages in thread
From: Junio C Hamano @ 2007-02-20  9:53 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git

This mechanically converts strncmp() to use prefixcmp(), but only when
the parameters match specific patterns, so that they can be verified
easily.  Leftover from this will be fixed in a separate step, including
idiotic conversions like

	if (!strncmp("foo", arg, 3))

  =>

	if (!(-prefixcmp(arg, "foo")))

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 * This was done by using this script in px.perl

   #!/usr/bin/perl -i.bak -p
   if (/strncmp\(([^,]+), "([^\\"]*)", (\d+)\)/ && (length($2) == $3)) {
           s|strncmp\(([^,]+), "([^\\"]*)", (\d+)\)|prefixcmp($1, "$2")|;
   }
   if (/strncmp\("([^\\"]*)", ([^,]+), (\d+)\)/ && (length($1) == $3)) {
           s|strncmp\("([^\\"]*)", ([^,]+), (\d+)\)|(-prefixcmp($2, "$1"))|;
   }

   and running:

   $ git grep -l strncmp -- '*.c' | xargs perl px.perl

 builtin-apply.c          |   12 ++++++------
 builtin-archive.c        |   10 +++++-----
 builtin-blame.c          |    6 +++---
 builtin-branch.c         |   12 ++++++------
 builtin-checkout-index.c |    4 ++--
 builtin-describe.c       |    6 +++---
 builtin-fmt-merge-msg.c  |   10 +++++-----
 builtin-for-each-ref.c   |    6 +++---
 builtin-fsck.c           |    2 +-
 builtin-grep.c           |    6 +++---
 builtin-init-db.c        |    4 ++--
 builtin-log.c            |   12 ++++++------
 builtin-ls-files.c       |    8 ++++----
 builtin-mailinfo.c       |    2 +-
 builtin-name-rev.c       |    8 ++++----
 builtin-pack-objects.c   |    6 +++---
 builtin-pack-refs.c      |    2 +-
 builtin-push.c           |   18 +++++++++---------
 builtin-read-tree.c      |    4 ++--
 builtin-reflog.c         |    4 ++--
 builtin-rerere.c         |    6 +++---
 builtin-rev-parse.c      |   10 +++++-----
 builtin-shortlog.c       |    6 +++---
 builtin-show-branch.c    |   20 ++++++++++----------
 builtin-show-ref.c       |   12 ++++++------
 builtin-tar-tree.c       |    2 +-
 builtin-unpack-objects.c |    2 +-
 builtin-write-tree.c     |    2 +-
 connect.c                |    6 +++---
 daemon.c                 |   32 ++++++++++++++++----------------
 diff.c                   |   30 +++++++++++++++---------------
 exec_cmd.c               |    2 +-
 fast-import.c            |   42 +++++++++++++++++++++---------------------
 fetch-pack.c             |   12 ++++++------
 git.c                    |   10 +++++-----
 help.c                   |    4 ++--
 http-fetch.c             |    2 +-
 http-push.c              |    8 ++++----
 index-pack.c             |    4 ++--
 peek-remote.c            |    4 ++--
 receive-pack.c           |    4 ++--
 refs.c                   |    8 ++++----
 revision.c               |   30 +++++++++++++++---------------
 send-pack.c              |    4 ++--
 setup.c                  |    2 +-
 shell.c                  |    2 +-
 upload-pack.c            |   10 +++++-----
 wt-status.c              |    4 ++--
 48 files changed, 211 insertions(+), 211 deletions(-)

diff --git a/builtin-apply.c b/builtin-apply.c
index abe3538..d678178 100644
--- a/builtin-apply.c
+++ b/builtin-apply.c
@@ -1129,11 +1129,11 @@ static struct fragment *parse_binary_hunk(char **buf_p,
 
 	*status_p = 0;
 
-	if (!strncmp(buffer, "delta ", 6)) {
+	if (!prefixcmp(buffer, "delta ")) {
 		patch_method = BINARY_DELTA_DEFLATED;
 		origlen = strtoul(buffer + 6, NULL, 10);
 	}
-	else if (!strncmp(buffer, "literal ", 8)) {
+	else if (!prefixcmp(buffer, "literal ")) {
 		patch_method = BINARY_LITERAL_DEFLATED;
 		origlen = strtoul(buffer + 8, NULL, 10);
 	}
@@ -2608,14 +2608,14 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 			read_stdin = 0;
 			continue;
 		}
-		if (!strncmp(arg, "--exclude=", 10)) {
+		if (!prefixcmp(arg, "--exclude=")) {
 			struct excludes *x = xmalloc(sizeof(*x));
 			x->path = arg + 10;
 			x->next = excludes;
 			excludes = x;
 			continue;
 		}
-		if (!strncmp(arg, "-p", 2)) {
+		if (!prefixcmp(arg, "-p")) {
 			p_value = atoi(arg + 2);
 			continue;
 		}
@@ -2669,13 +2669,13 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 			line_termination = 0;
 			continue;
 		}
-		if (!strncmp(arg, "-C", 2)) {
+		if (!prefixcmp(arg, "-C")) {
 			p_context = strtoul(arg + 2, &end, 0);
 			if (*end != '\0')
 				die("unrecognized context count '%s'", arg + 2);
 			continue;
 		}
-		if (!strncmp(arg, "--whitespace=", 13)) {
+		if (!prefixcmp(arg, "--whitespace=")) {
 			whitespace_option = arg + 13;
 			parse_whitespace_option(arg + 13);
 			continue;
diff --git a/builtin-archive.c b/builtin-archive.c
index f613ac2..0c56de0 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -35,7 +35,7 @@ static int run_remote_archiver(const char *remote, int argc,
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
-		if (!strncmp("--exec=", arg, 7)) {
+		if (!(-prefixcmp(arg, "--exec="))) {
 			if (exec_at)
 				die("multiple --exec specified");
 			exec = arg + 7;
@@ -62,7 +62,7 @@ static int run_remote_archiver(const char *remote, int argc,
 	if (buf[len-1] == '\n')
 		buf[--len] = 0;
 	if (strcmp(buf, "ACK")) {
-		if (len > 5 && !strncmp(buf, "NACK ", 5))
+		if (len > 5 && !prefixcmp(buf, "NACK "))
 			die("git-archive: NACK %s", buf + 5);
 		die("git-archive: protocol error");
 	}
@@ -166,11 +166,11 @@ int parse_archive_args(int argc, const char **argv, struct archiver *ar)
 			verbose = 1;
 			continue;
 		}
-		if (!strncmp(arg, "--format=", 9)) {
+		if (!prefixcmp(arg, "--format=")) {
 			format = arg + 9;
 			continue;
 		}
-		if (!strncmp(arg, "--prefix=", 9)) {
+		if (!prefixcmp(arg, "--prefix=")) {
 			base = arg + 9;
 			continue;
 		}
@@ -218,7 +218,7 @@ static const char *extract_remote_arg(int *ac, const char **av)
 		if (!strcmp(arg, "--"))
 			no_more_options = 1;
 		if (!no_more_options) {
-			if (!strncmp(arg, "--remote=", 9)) {
+			if (!prefixcmp(arg, "--remote=")) {
 				if (remote)
 					die("Multiple --remote specified");
 				remote = arg + 9;
diff --git a/builtin-blame.c b/builtin-blame.c
index 5669a16..db311bf 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -2097,17 +2097,17 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 			output_option |= OUTPUT_LONG_OBJECT_NAME;
 		else if (!strcmp("-S", arg) && ++i < argc)
 			revs_file = argv[i];
-		else if (!strncmp("-M", arg, 2)) {
+		else if (!(-prefixcmp(arg, "-M"))) {
 			opt |= PICKAXE_BLAME_MOVE;
 			blame_move_score = parse_score(arg+2);
 		}
-		else if (!strncmp("-C", arg, 2)) {
+		else if (!(-prefixcmp(arg, "-C"))) {
 			if (opt & PICKAXE_BLAME_COPY)
 				opt |= PICKAXE_BLAME_COPY_HARDER;
 			opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;
 			blame_copy_score = parse_score(arg+2);
 		}
-		else if (!strncmp("-L", arg, 2)) {
+		else if (!(-prefixcmp(arg, "-L"))) {
 			if (!arg[2]) {
 				if (++i >= argc)
 					usage(blame_usage);
diff --git a/builtin-branch.c b/builtin-branch.c
index d0e7209..d0179b0 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -59,7 +59,7 @@ int git_branch_config(const char *var, const char *value)
 		branch_use_color = git_config_colorbool(var, value);
 		return 0;
 	}
-	if (!strncmp(var, "color.branch.", 13)) {
+	if (!prefixcmp(var, "color.branch.")) {
 		int slot = parse_branch_color_slot(var, 13);
 		color_parse(value, var, branch_colors[slot]);
 		return 0;
@@ -178,13 +178,13 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,
 	int len;
 
 	/* Detect kind */
-	if (!strncmp(refname, "refs/heads/", 11)) {
+	if (!prefixcmp(refname, "refs/heads/")) {
 		kind = REF_LOCAL_BRANCH;
 		refname += 11;
-	} else if (!strncmp(refname, "refs/remotes/", 13)) {
+	} else if (!prefixcmp(refname, "refs/remotes/")) {
 		kind = REF_REMOTE_BRANCH;
 		refname += 13;
-	} else if (!strncmp(refname, "refs/tags/", 10)) {
+	} else if (!prefixcmp(refname, "refs/tags/")) {
 		kind = REF_TAG;
 		refname += 10;
 	}
@@ -446,7 +446,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 			reflog = 1;
 			continue;
 		}
-		if (!strncmp(arg, "--abbrev=", 9)) {
+		if (!prefixcmp(arg, "--abbrev=")) {
 			abbrev = atoi(arg+9);
 			continue;
 		}
@@ -476,7 +476,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		detached = 1;
 	}
 	else {
-		if (strncmp(head, "refs/heads/", 11))
+		if (prefixcmp(head, "refs/heads/"))
 			die("HEAD not found below refs/heads!");
 		head += 11;
 	}
diff --git a/builtin-checkout-index.c b/builtin-checkout-index.c
index b097c88..afe4b0e 100644
--- a/builtin-checkout-index.c
+++ b/builtin-checkout-index.c
@@ -223,12 +223,12 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
 			to_tempfile = 1;
 			continue;
 		}
-		if (!strncmp(arg, "--prefix=", 9)) {
+		if (!prefixcmp(arg, "--prefix=")) {
 			state.base_dir = arg+9;
 			state.base_dir_len = strlen(state.base_dir);
 			continue;
 		}
-		if (!strncmp(arg, "--stage=", 8)) {
+		if (!prefixcmp(arg, "--stage=")) {
 			if (!strcmp(arg + 8, "all")) {
 				to_tempfile = 1;
 				checkout_stage = CHECKOUT_ALL;
diff --git a/builtin-describe.c b/builtin-describe.c
index bcc6456..165917e 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -52,7 +52,7 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
 	 * If --tags, then any tags are used.
 	 * Otherwise only annotated tags are used.
 	 */
-	if (!strncmp(path, "refs/tags/", 10)) {
+	if (!prefixcmp(path, "refs/tags/")) {
 		if (object->type == OBJ_TAG)
 			prio = 2;
 		else
@@ -254,12 +254,12 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
 			all = 1;
 		else if (!strcmp(arg, "--tags"))
 			tags = 1;
-		else if (!strncmp(arg, "--abbrev=", 9)) {
+		else if (!prefixcmp(arg, "--abbrev=")) {
 			abbrev = strtoul(arg + 9, NULL, 10);
 			if (abbrev != 0 && (abbrev < MINIMUM_ABBREV || 40 < abbrev))
 				abbrev = DEFAULT_ABBREV;
 		}
-		else if (!strncmp(arg, "--candidates=", 13)) {
+		else if (!prefixcmp(arg, "--candidates=")) {
 			max_candidates = strtoul(arg + 13, NULL, 10);
 			if (max_candidates < 1)
 				max_candidates = 1;
diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index 87d3d63..1489883 100644
--- a/builtin-fmt-merge-msg.c
+++ b/builtin-fmt-merge-msg.c
@@ -81,7 +81,7 @@ static int handle_line(char *line)
 	if (len < 43 || line[40] != '\t')
 		return 1;
 
-	if (!strncmp(line + 41, "not-for-merge", 13))
+	if (!prefixcmp(line + 41, "not-for-merge"))
 		return 0;
 
 	if (line[41] != '\t')
@@ -119,15 +119,15 @@ static int handle_line(char *line)
 	if (pulling_head) {
 		origin = xstrdup(src);
 		src_data->head_status |= 1;
-	} else if (!strncmp(line, "branch ", 7)) {
+	} else if (!prefixcmp(line, "branch ")) {
 		origin = xstrdup(line + 7);
 		append_to_list(&src_data->branch, origin, NULL);
 		src_data->head_status |= 2;
-	} else if (!strncmp(line, "tag ", 4)) {
+	} else if (!prefixcmp(line, "tag ")) {
 		origin = line;
 		append_to_list(&src_data->tag, xstrdup(origin + 4), NULL);
 		src_data->head_status |= 2;
-	} else if (!strncmp(line, "remote branch ", 14)) {
+	} else if (!prefixcmp(line, "remote branch ")) {
 		origin = xstrdup(line + 14);
 		append_to_list(&src_data->r_branch, origin, NULL);
 		src_data->head_status |= 2;
@@ -280,7 +280,7 @@ int cmd_fmt_merge_msg(int argc, const char **argv, const char *prefix)
 	current_branch = resolve_ref("HEAD", head_sha1, 1, NULL);
 	if (!current_branch)
 		die("No current branch");
-	if (!strncmp(current_branch, "refs/heads/", 11))
+	if (!prefixcmp(current_branch, "refs/heads/"))
 		current_branch += 11;
 
 	while (fgets(line, sizeof(line), in)) {
diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 16c785f..ac0b9f6 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -814,7 +814,7 @@ int cmd_for_each_ref(int ac, const char **av, char *prefix)
 			i++;
 			break;
 		}
-		if (!strncmp(arg, "--format=", 9)) {
+		if (!prefixcmp(arg, "--format=")) {
 			if (format)
 				die("more than one --format?");
 			format = arg + 9;
@@ -844,7 +844,7 @@ int cmd_for_each_ref(int ac, const char **av, char *prefix)
 			quote_style = QUOTE_TCL;
 			continue;
 		}
-		if (!strncmp(arg, "--count=", 8)) {
+		if (!prefixcmp(arg, "--count=")) {
 			if (maxcount)
 				die("more than one --count?");
 			maxcount = atoi(arg + 8);
@@ -852,7 +852,7 @@ int cmd_for_each_ref(int ac, const char **av, char *prefix)
 				die("The number %s did not parse", arg);
 			continue;
 		}
-		if (!strncmp(arg, "--sort=", 7)) {
+		if (!prefixcmp(arg, "--sort=")) {
 			struct ref_sort *s = xcalloc(1, sizeof(*s));
 			int len;
 
diff --git a/builtin-fsck.c b/builtin-fsck.c
index 6da3814..6abf498 100644
--- a/builtin-fsck.c
+++ b/builtin-fsck.c
@@ -546,7 +546,7 @@ static int fsck_head_link(void)
 
 	if (!head_points_at || !(flag & REF_ISSYMREF))
 		return error("HEAD is not a symbolic ref");
-	if (strncmp(head_points_at, "refs/heads/", 11))
+	if (prefixcmp(head_points_at, "refs/heads/"))
 		return error("HEAD points to something strange (%s)",
 			     head_points_at);
 	if (is_null_sha1(sha1))
diff --git a/builtin-grep.c b/builtin-grep.c
index 2bfbdb7..cec2204 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -527,9 +527,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			opt.word_regexp = 1;
 			continue;
 		}
-		if (!strncmp("-A", arg, 2) ||
-		    !strncmp("-B", arg, 2) ||
-		    !strncmp("-C", arg, 2) ||
+		if (!(-prefixcmp(arg, "-A")) ||
+		    !(-prefixcmp(arg, "-B")) ||
+		    !(-prefixcmp(arg, "-C")) ||
 		    (arg[0] == '-' && '1' <= arg[1] && arg[1] <= '9')) {
 			unsigned num;
 			const char *scan;
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 12e43d0..4df9fd0 100644
--- a/builtin-init-db.c
+++ b/builtin-init-db.c
@@ -283,11 +283,11 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
 
 	for (i = 1; i < argc; i++, argv++) {
 		const char *arg = argv[1];
-		if (!strncmp(arg, "--template=", 11))
+		if (!prefixcmp(arg, "--template="))
 			template_dir = arg+11;
 		else if (!strcmp(arg, "--shared"))
 			shared_repository = PERM_GROUP;
-		else if (!strncmp(arg, "--shared=", 9))
+		else if (!prefixcmp(arg, "--shared="))
 			shared_repository = git_config_perm("arg", arg+9);
 		else
 			usage(init_db_usage);
diff --git a/builtin-log.c b/builtin-log.c
index af2de54..ad1e8c0 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -32,7 +32,7 @@ static void cmd_log_init(int argc, const char **argv, const char *prefix,
 		rev->always_show_header = 0;
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
-		if (!strncmp(arg, "--encoding=", 11)) {
+		if (!prefixcmp(arg, "--encoding=")) {
 			arg += 11;
 			if (strcmp(arg, "none"))
 				git_log_output_encoding = strdup(arg);
@@ -287,7 +287,7 @@ static void reopen_stdout(struct commit *commit, int nr, int keep_subject)
 
 		sol += 2;
 		/* strip [PATCH] or [PATCH blabla] */
-		if (!keep_subject && !strncmp(sol, "[PATCH", 6)) {
+		if (!keep_subject && !prefixcmp(sol, "[PATCH")) {
 			char *eos = strchr(sol + 6, ']');
 			if (eos) {
 				while (isspace(*eos))
@@ -435,7 +435,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		else if (!strcmp(argv[i], "-n") ||
 				!strcmp(argv[i], "--numbered"))
 			numbered = 1;
-		else if (!strncmp(argv[i], "--start-number=", 15))
+		else if (!prefixcmp(argv[i], "--start-number="))
 			start_number = strtol(argv[i] + 15, NULL, 10);
 		else if (!strcmp(argv[i], "--start-number")) {
 			i++;
@@ -471,13 +471,13 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 		}
 		else if (!strcmp(argv[i], "--attach"))
 			rev.mime_boundary = git_version_string;
-		else if (!strncmp(argv[i], "--attach=", 9))
+		else if (!prefixcmp(argv[i], "--attach="))
 			rev.mime_boundary = argv[i] + 9;
 		else if (!strcmp(argv[i], "--ignore-if-in-upstream"))
 			ignore_if_in_upstream = 1;
 		else if (!strcmp(argv[i], "--thread"))
 			thread = 1;
-		else if (!strncmp(argv[i], "--in-reply-to=", 14))
+		else if (!prefixcmp(argv[i], "--in-reply-to="))
 			in_reply_to = argv[i] + 14;
 		else if (!strcmp(argv[i], "--in-reply-to")) {
 			i++;
@@ -485,7 +485,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 				die("Need a Message-Id for --in-reply-to");
 			in_reply_to = argv[i];
 		}
-		else if (!strncmp(argv[i], "--suffix=", 9))
+		else if (!prefixcmp(argv[i], "--suffix="))
 			fmt_patch_suffix = argv[i] + 9;
 		else
 			argv[j++] = argv[i];
diff --git a/builtin-ls-files.c b/builtin-ls-files.c
index ac89eb2..4e1d5af 100644
--- a/builtin-ls-files.c
+++ b/builtin-ls-files.c
@@ -406,7 +406,7 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 			add_exclude(argv[++i], "", 0, &dir.exclude_list[EXC_CMDL]);
 			continue;
 		}
-		if (!strncmp(arg, "--exclude=", 10)) {
+		if (!prefixcmp(arg, "--exclude=")) {
 			exc_given = 1;
 			add_exclude(arg+10, "", 0, &dir.exclude_list[EXC_CMDL]);
 			continue;
@@ -416,12 +416,12 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 			add_excludes_from_file(&dir, argv[++i]);
 			continue;
 		}
-		if (!strncmp(arg, "--exclude-from=", 15)) {
+		if (!prefixcmp(arg, "--exclude-from=")) {
 			exc_given = 1;
 			add_excludes_from_file(&dir, arg+15);
 			continue;
 		}
-		if (!strncmp(arg, "--exclude-per-directory=", 24)) {
+		if (!prefixcmp(arg, "--exclude-per-directory=")) {
 			exc_given = 1;
 			dir.exclude_per_dir = arg + 24;
 			continue;
@@ -434,7 +434,7 @@ int cmd_ls_files(int argc, const char **argv, const char *prefix)
 			error_unmatch = 1;
 			continue;
 		}
-		if (!strncmp(arg, "--abbrev=", 9)) {
+		if (!prefixcmp(arg, "--abbrev=")) {
 			abbrev = strtoul(arg+9, NULL, 10);
 			if (abbrev && abbrev < MINIMUM_ABBREV)
 				abbrev = MINIMUM_ABBREV;
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index 583da38..6ee6b0b 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -811,7 +811,7 @@ int cmd_mailinfo(int argc, const char **argv, const char *prefix)
 			metainfo_charset = def_charset;
 		else if (!strcmp(argv[1], "-n"))
 			metainfo_charset = NULL;
-		else if (!strncmp(argv[1], "--encoding=", 11))
+		else if (!prefixcmp(argv[1], "--encoding="))
 			metainfo_charset = argv[1] + 11;
 		else
 			usage(mailinfo_usage);
diff --git a/builtin-name-rev.c b/builtin-name-rev.c
index 36f1ba6..2c3d14c 100644
--- a/builtin-name-rev.c
+++ b/builtin-name-rev.c
@@ -85,7 +85,7 @@ static int name_ref(const char *path, const unsigned char *sha1, int flags, void
 	struct name_ref_data *data = cb_data;
 	int deref = 0;
 
-	if (data->tags_only && strncmp(path, "refs/tags/", 10))
+	if (data->tags_only && prefixcmp(path, "refs/tags/"))
 		return 0;
 
 	if (data->ref_filter && fnmatch(data->ref_filter, path, 0))
@@ -101,9 +101,9 @@ static int name_ref(const char *path, const unsigned char *sha1, int flags, void
 	if (o && o->type == OBJ_COMMIT) {
 		struct commit *commit = (struct commit *)o;
 
-		if (!strncmp(path, "refs/heads/", 11))
+		if (!prefixcmp(path, "refs/heads/"))
 			path = path + 11;
-		else if (!strncmp(path, "refs/", 5))
+		else if (!prefixcmp(path, "refs/"))
 			path = path + 5;
 
 		name_rev(commit, xstrdup(path), 0, 0, deref);
@@ -156,7 +156,7 @@ int cmd_name_rev(int argc, const char **argv, const char *prefix)
 			} else if (!strcmp(*argv, "--tags")) {
 				data.tags_only = 1;
 				continue;
-			} else  if (!strncmp(*argv, "--refs=", 7)) {
+			} else  if (!prefixcmp(*argv, "--refs=")) {
 				data.ref_filter = *argv + 7;
 				continue;
 			} else if (!strcmp(*argv, "--all")) {
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 3824ee3..71113d8 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1579,14 +1579,14 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			incremental = 1;
 			continue;
 		}
-		if (!strncmp("--window=", arg, 9)) {
+		if (!(-prefixcmp(arg, "--window="))) {
 			char *end;
 			window = strtoul(arg+9, &end, 0);
 			if (!arg[9] || *end)
 				usage(pack_usage);
 			continue;
 		}
-		if (!strncmp("--depth=", arg, 8)) {
+		if (!(-prefixcmp(arg, "--depth="))) {
 			char *end;
 			depth = strtoul(arg+8, &end, 0);
 			if (!arg[8] || *end)
@@ -1622,7 +1622,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!strcmp("--unpacked", arg) ||
-		    !strncmp("--unpacked=", arg, 11) ||
+		    !(-prefixcmp(arg, "--unpacked=")) ||
 		    !strcmp("--reflog", arg) ||
 		    !strcmp("--all", arg)) {
 			use_internal_rev_list = 1;
diff --git a/builtin-pack-refs.c b/builtin-pack-refs.c
index 3de9b3e..d080e30 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -36,7 +36,7 @@ static int handle_one_ref(const char *path, const unsigned char *sha1,
 	/* Do not pack the symbolic refs */
 	if ((flags & REF_ISSYMREF))
 		return 0;
-	is_tag_ref = !strncmp(path, "refs/tags/", 10);
+	is_tag_ref = !prefixcmp(path, "refs/tags/");
 
 	/* ALWAYS pack refs that were already packed or are tags */
 	if (!cb->all && !is_tag_ref && !(flags & REF_ISPACKED))
diff --git a/builtin-push.c b/builtin-push.c
index c45649e..2b98ba3 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -32,7 +32,7 @@ static int expand_one_ref(const char *ref, const unsigned char *sha1, int flag,
 	/* Ignore the "refs/" at the beginning of the refname */
 	ref += 5;
 
-	if (!strncmp(ref, "tags/", 5))
+	if (!prefixcmp(ref, "tags/"))
 		add_refspec(xstrdup(ref));
 	return 0;
 }
@@ -149,10 +149,10 @@ static int get_remotes_uri(const char *repo, const char *uri[MAX_URI])
 		int is_refspec;
 		char *s, *p;
 
-		if (!strncmp("URL:", buffer, 4)) {
+		if (!(-prefixcmp(buffer, "URL:"))) {
 			is_refspec = 0;
 			s = buffer + 4;
-		} else if (!strncmp("Push:", buffer, 5)) {
+		} else if (!(-prefixcmp(buffer, "Push:"))) {
 			is_refspec = 1;
 			s = buffer + 5;
 		} else
@@ -195,7 +195,7 @@ static int config_get_receivepack;
 
 static int get_remote_config(const char* key, const char* value)
 {
-	if (!strncmp(key, "remote.", 7) &&
+	if (!prefixcmp(key, "remote.") &&
 	    !strncmp(key + 7, config_repo, config_repo_len)) {
 		if (!strcmp(key + 7 + config_repo_len, ".url")) {
 			if (config_current_uri < MAX_URI)
@@ -324,8 +324,8 @@ static int do_push(const char *repo)
 		const char **dest_refspec = refspec;
 		const char *dest = uri[i];
 		const char *sender = "git-send-pack";
-		if (!strncmp(dest, "http://", 7) ||
-		    !strncmp(dest, "https://", 8))
+		if (!prefixcmp(dest, "http://") ||
+		    !prefixcmp(dest, "https://"))
 			sender = "git-http-push";
 		else if (thin)
 			argv[dest_argc++] = "--thin";
@@ -373,7 +373,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 			verbose=1;
 			continue;
 		}
-		if (!strncmp(arg, "--repo=", 7)) {
+		if (!prefixcmp(arg, "--repo=")) {
 			repo = arg+7;
 			continue;
 		}
@@ -397,11 +397,11 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 			thin = 0;
 			continue;
 		}
-		if (!strncmp(arg, "--receive-pack=", 15)) {
+		if (!prefixcmp(arg, "--receive-pack=")) {
 			receivepack = arg;
 			continue;
 		}
-		if (!strncmp(arg, "--exec=", 7)) {
+		if (!prefixcmp(arg, "--exec=")) {
 			receivepack = arg;
 			continue;
 		}
diff --git a/builtin-read-tree.c b/builtin-read-tree.c
index 8ba436d..e477155 100644
--- a/builtin-read-tree.c
+++ b/builtin-read-tree.c
@@ -133,7 +133,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 		 *  entries and put the entries from the tree under the
 		 * given subdirectory.
 		 */
-		if (!strncmp(arg, "--prefix=", 9)) {
+		if (!prefixcmp(arg, "--prefix=")) {
 			if (stage || opts.merge || opts.prefix)
 				usage(read_tree_usage);
 			opts.prefix = arg + 9;
@@ -179,7 +179,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
 			continue;
 		}
 
-		if (!strncmp(arg, "--exclude-per-directory=", 24)) {
+		if (!prefixcmp(arg, "--exclude-per-directory=")) {
 			struct dir_struct *dir;
 
 			if (opts.dir)
diff --git a/builtin-reflog.c b/builtin-reflog.c
index 3415551..cefb40d 100644
--- a/builtin-reflog.c
+++ b/builtin-reflog.c
@@ -321,9 +321,9 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 		const char *arg = argv[i];
 		if (!strcmp(arg, "--dry-run") || !strcmp(arg, "-n"))
 			cb.dry_run = 1;
-		else if (!strncmp(arg, "--expire=", 9))
+		else if (!prefixcmp(arg, "--expire="))
 			cb.expire_total = approxidate(arg + 9);
-		else if (!strncmp(arg, "--expire-unreachable=", 21))
+		else if (!prefixcmp(arg, "--expire-unreachable="))
 			cb.expire_unreachable = approxidate(arg + 21);
 		else if (!strcmp(arg, "--stale-fix"))
 			cb.stalefix = 1;
diff --git a/builtin-rerere.c b/builtin-rerere.c
index 318d959..978105b 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -105,11 +105,11 @@ static int handle_file(const char *path,
 		SHA1_Init(&ctx);
 
 	while (fgets(buf, sizeof(buf), f)) {
-		if (!strncmp("<<<<<<< ", buf, 8))
+		if (!(-prefixcmp(buf, "<<<<<<< ")))
 			hunk = 1;
-		else if (!strncmp("=======", buf, 7))
+		else if (!(-prefixcmp(buf, "=======")))
 			hunk = 2;
-		else if (!strncmp(">>>>>>> ", buf, 8)) {
+		else if (!(-prefixcmp(buf, ">>>>>>> "))) {
 			hunk_no++;
 			hunk = 0;
 			if (memcmp(one->ptr, two->ptr, one->nr < two->nr ?
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index d53deaa..a1c3411 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -274,7 +274,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!strcmp(arg, "--short") ||
-			    !strncmp(arg, "--short=", 8)) {
+			    !prefixcmp(arg, "--short=")) {
 				filter &= ~(DO_FLAGS|DO_NOREV);
 				verify = 1;
 				abbrev = DEFAULT_ABBREV;
@@ -352,19 +352,19 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 						: "false");
 				continue;
 			}
-			if (!strncmp(arg, "--since=", 8)) {
+			if (!prefixcmp(arg, "--since=")) {
 				show_datestring("--max-age=", arg+8);
 				continue;
 			}
-			if (!strncmp(arg, "--after=", 8)) {
+			if (!prefixcmp(arg, "--after=")) {
 				show_datestring("--max-age=", arg+8);
 				continue;
 			}
-			if (!strncmp(arg, "--before=", 9)) {
+			if (!prefixcmp(arg, "--before=")) {
 				show_datestring("--min-age=", arg+9);
 				continue;
 			}
-			if (!strncmp(arg, "--until=", 8)) {
+			if (!prefixcmp(arg, "--until=")) {
 				show_datestring("--min-age=", arg+8);
 				continue;
 			}
diff --git a/builtin-shortlog.c b/builtin-shortlog.c
index edb4042..2f71a2a 100644
--- a/builtin-shortlog.c
+++ b/builtin-shortlog.c
@@ -124,7 +124,7 @@ static void insert_author_oneline(struct path_list *list,
 	else
 		free(buffer);
 
-	if (!strncmp(oneline, "[PATCH", 6)) {
+	if (!prefixcmp(oneline, "[PATCH")) {
 		char *eob = strchr(oneline, ']');
 
 		if (eob) {
@@ -179,7 +179,7 @@ static void read_from_stdin(struct path_list *list)
 	while (fgets(buffer, sizeof(buffer), stdin) != NULL) {
 		char *bob;
 		if ((buffer[0] == 'A' || buffer[0] == 'a') &&
-				!strncmp(buffer + 1, "uthor: ", 7) &&
+				!prefixcmp(buffer + 1, "uthor: ") &&
 				(bob = strchr(buffer + 7, '<')) != NULL) {
 			char buffer2[1024], offset = 0;
 
@@ -230,7 +230,7 @@ static void get_from_rev(struct rev_info *rev, struct path_list *list)
 			else
 				eol++;
 
-			if (!strncmp(buffer, "author ", 7)) {
+			if (!prefixcmp(buffer, "author ")) {
 				char *bracket = strchr(buffer, '<');
 
 				if (bracket == NULL || bracket > eol)
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 0d94e40..bf6aee4 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -266,7 +266,7 @@ static void show_one_commit(struct commit *commit, int no_name)
 				    pretty, sizeof(pretty), 0, NULL, NULL, 0);
 	else
 		strcpy(pretty, "(unavailable)");
-	if (!strncmp(pretty, "[PATCH] ", 8))
+	if (!prefixcmp(pretty, "[PATCH] "))
 		cp = pretty + 8;
 	else
 		cp = pretty;
@@ -404,7 +404,7 @@ static int append_remote_ref(const char *refname, const unsigned char *sha1, int
 
 static int append_tag_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
-	if (strncmp(refname, "refs/tags/", 10))
+	if (prefixcmp(refname, "refs/tags/"))
 		return 0;
 	return append_ref(refname + 5, sha1, 0);
 }
@@ -435,9 +435,9 @@ static int append_matching_ref(const char *refname, const unsigned char *sha1, i
 		return 0;
 	if (fnmatch(match_ref_pattern, tail, 0))
 		return 0;
-	if (!strncmp("refs/heads/", refname, 11))
+	if (!(-prefixcmp(refname, "refs/heads/")))
 		return append_head_ref(refname, sha1, flag, cb_data);
-	if (!strncmp("refs/tags/", refname, 10))
+	if (!(-prefixcmp(refname, "refs/tags/")))
 		return append_tag_ref(refname, sha1, flag, cb_data);
 	return append_ref(refname, sha1, 0);
 }
@@ -462,11 +462,11 @@ static int rev_is_head(char *head, int headlen, char *name,
 	if ((!head[0]) ||
 	    (head_sha1 && sha1 && hashcmp(head_sha1, sha1)))
 		return 0;
-	if (!strncmp(head, "refs/heads/", 11))
+	if (!prefixcmp(head, "refs/heads/"))
 		head += 11;
-	if (!strncmp(name, "refs/heads/", 11))
+	if (!prefixcmp(name, "refs/heads/"))
 		name += 11;
-	else if (!strncmp(name, "heads/", 6))
+	else if (!prefixcmp(name, "heads/"))
 		name += 6;
 	return !strcmp(head, name);
 }
@@ -635,7 +635,7 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 			with_current_branch = 1;
 		else if (!strcmp(arg, "--sha1-name"))
 			sha1_name = 1;
-		else if (!strncmp(arg, "--more=", 7))
+		else if (!prefixcmp(arg, "--more="))
 			extra = atoi(arg + 7);
 		else if (!strcmp(arg, "--merge-base"))
 			merge_base = 1;
@@ -652,9 +652,9 @@ int cmd_show_branch(int ac, const char **av, const char *prefix)
 		else if (!strcmp(arg, "--reflog") || !strcmp(arg, "-g")) {
 			reflog = DEFAULT_REFLOG;
 		}
-		else if (!strncmp(arg, "--reflog=", 9))
+		else if (!prefixcmp(arg, "--reflog="))
 			parse_reflog_param(arg + 9, &reflog, &reflog_base);
-		else if (!strncmp(arg, "-g=", 3))
+		else if (!prefixcmp(arg, "-g="))
 			parse_reflog_param(arg + 3, &reflog, &reflog_base);
 		else
 			usage(show_branch_usage);
diff --git a/builtin-show-ref.c b/builtin-show-ref.c
index 853f13f..ae0eddd 100644
--- a/builtin-show-ref.c
+++ b/builtin-show-ref.c
@@ -28,8 +28,8 @@ static int show_ref(const char *refname, const unsigned char *sha1, int flag, vo
 	if (tags_only || heads_only) {
 		int match;
 
-		match = heads_only && !strncmp(refname, "refs/heads/", 11);
-		match |= tags_only && !strncmp(refname, "refs/tags/", 10);
+		match = heads_only && !prefixcmp(refname, "refs/heads/");
+		match |= tags_only && !prefixcmp(refname, "refs/tags/");
 		if (!match)
 			return 0;
 	}
@@ -178,8 +178,8 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
 			hash_only = 1;
 			continue;
 		}
-		if (!strncmp(arg, "--hash=", 7) ||
-		    (!strncmp(arg, "--abbrev", 8) &&
+		if (!prefixcmp(arg, "--hash=") ||
+		    (!prefixcmp(arg, "--abbrev") &&
 		     (arg[8] == '=' || arg[8] == '\0'))) {
 			if (arg[2] != 'h' && !arg[8])
 				/* --abbrev only */
@@ -215,7 +215,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
 		}
 		if (!strcmp(arg, "--exclude-existing"))
 			return exclude_existing(NULL);
-		if (!strncmp(arg, "--exclude-existing=", 19))
+		if (!prefixcmp(arg, "--exclude-existing="))
 			return exclude_existing(arg + 19);
 		usage(show_ref_usage);
 	}
@@ -224,7 +224,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
 		unsigned char sha1[20];
 
 		while (*pattern) {
-			if (!strncmp(*pattern, "refs/", 5) &&
+			if (!prefixcmp(*pattern, "refs/") &&
 			    resolve_ref(*pattern, sha1, 1, NULL)) {
 				if (!quiet)
 					show_one(*pattern, sha1);
diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
index 8055dda..28f8c1c 100644
--- a/builtin-tar-tree.c
+++ b/builtin-tar-tree.c
@@ -31,7 +31,7 @@ int cmd_tar_tree(int argc, const char **argv, const char *prefix)
 	nargv[nargc++] = "git-archive";
 	nargv[nargc++] = "--format=tar";
 
-	if (2 <= argc && !strncmp("--remote=", argv[1], 9)) {
+	if (2 <= argc && !(-prefixcmp(argv[1], "--remote="))) {
 		nargv[nargc++] = argv[1];
 		argv++;
 		argc--;
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index d351e02..8f8e898 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -369,7 +369,7 @@ int cmd_unpack_objects(int argc, const char **argv, const char *prefix)
 				recover = 1;
 				continue;
 			}
-			if (!strncmp(arg, "--pack_header=", 14)) {
+			if (!prefixcmp(arg, "--pack_header=")) {
 				struct pack_header *hdr;
 				char *c;
 
diff --git a/builtin-write-tree.c b/builtin-write-tree.c
index 50670dc..90fc1cf 100644
--- a/builtin-write-tree.c
+++ b/builtin-write-tree.c
@@ -70,7 +70,7 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
 		const char *arg = argv[1];
 		if (!strcmp(arg, "--missing-ok"))
 			missing_ok = 1;
-		else if (!strncmp(arg, "--prefix=", 9))
+		else if (!prefixcmp(arg, "--prefix="))
 			prefix = arg + 9;
 		else
 			usage(write_tree_usage);
diff --git a/connect.c b/connect.c
index 7844888..8a8a13b 100644
--- a/connect.c
+++ b/connect.c
@@ -96,7 +96,7 @@ int get_ack(int fd, unsigned char *result_sha1)
 		line[--len] = 0;
 	if (!strcmp(line, "NAK"))
 		return 0;
-	if (!strncmp(line, "ACK ", 4)) {
+	if (!prefixcmp(line, "ACK ")) {
 		if (!get_sha1_hex(line+4, result_sha1)) {
 			if (strstr(line+45, "continue"))
 				return 2;
@@ -196,8 +196,8 @@ static int count_refspec_match(const char *pattern,
 		 */
 		if (namelen != patlen &&
 		    patlen != namelen - 5 &&
-		    strncmp(name, "refs/heads/", 11) &&
-		    strncmp(name, "refs/tags/", 10)) {
+		    prefixcmp(name, "refs/heads/") &&
+		    prefixcmp(name, "refs/tags/")) {
 			/* We want to catch the case where only weak
 			 * matches are found and there are multiple
 			 * matches, and where more than one strong
diff --git a/daemon.c b/daemon.c
index 66f8d6f..cdbc23f 100644
--- a/daemon.c
+++ b/daemon.c
@@ -286,7 +286,7 @@ static int service_enabled;
 
 static int git_daemon_config(const char *var, const char *value)
 {
-	if (!strncmp(var, "daemon.", 7) &&
+	if (!prefixcmp(var, "daemon.") &&
 	    !strcmp(var + 7, service_looking_at->config_name)) {
 		service_enabled = git_config_bool(var, value);
 		return 0;
@@ -562,7 +562,7 @@ static int execute(struct sockaddr *addr)
 	for (i = 0; i < ARRAY_SIZE(daemon_service); i++) {
 		struct daemon_service *s = &(daemon_service[i]);
 		int namelen = strlen(s->name);
-		if (!strncmp("git-", line, 4) &&
+		if (!(-prefixcmp(line, "git-")) &&
 		    !strncmp(s->name, line + 4, namelen) &&
 		    line[namelen + 4] == ' ') {
 			/*
@@ -1011,7 +1011,7 @@ int main(int argc, char **argv)
 	for (i = 1; i < argc; i++) {
 		char *arg = argv[i];
 
-		if (!strncmp(arg, "--listen=", 9)) {
+		if (!prefixcmp(arg, "--listen=")) {
 		    char *p = arg + 9;
 		    char *ph = listen_addr = xmalloc(strlen(arg + 9) + 1);
 		    while (*p)
@@ -1019,7 +1019,7 @@ int main(int argc, char **argv)
 		    *ph = 0;
 		    continue;
 		}
-		if (!strncmp(arg, "--port=", 7)) {
+		if (!prefixcmp(arg, "--port=")) {
 			char *end;
 			unsigned long n;
 			n = strtoul(arg+7, &end, 0);
@@ -1045,11 +1045,11 @@ int main(int argc, char **argv)
 			export_all_trees = 1;
 			continue;
 		}
-		if (!strncmp(arg, "--timeout=", 10)) {
+		if (!prefixcmp(arg, "--timeout=")) {
 			timeout = atoi(arg+10);
 			continue;
 		}
-		if (!strncmp(arg, "--init-timeout=", 15)) {
+		if (!prefixcmp(arg, "--init-timeout=")) {
 			init_timeout = atoi(arg+15);
 			continue;
 		}
@@ -1057,11 +1057,11 @@ int main(int argc, char **argv)
 			strict_paths = 1;
 			continue;
 		}
-		if (!strncmp(arg, "--base-path=", 12)) {
+		if (!prefixcmp(arg, "--base-path=")) {
 			base_path = arg+12;
 			continue;
 		}
-		if (!strncmp(arg, "--interpolated-path=", 20)) {
+		if (!prefixcmp(arg, "--interpolated-path=")) {
 			interpolated_path = arg+20;
 			continue;
 		}
@@ -1073,11 +1073,11 @@ int main(int argc, char **argv)
 			user_path = "";
 			continue;
 		}
-		if (!strncmp(arg, "--user-path=", 12)) {
+		if (!prefixcmp(arg, "--user-path=")) {
 			user_path = arg + 12;
 			continue;
 		}
-		if (!strncmp(arg, "--pid-file=", 11)) {
+		if (!prefixcmp(arg, "--pid-file=")) {
 			pid_file = arg + 11;
 			continue;
 		}
@@ -1086,27 +1086,27 @@ int main(int argc, char **argv)
 			log_syslog = 1;
 			continue;
 		}
-		if (!strncmp(arg, "--user=", 7)) {
+		if (!prefixcmp(arg, "--user=")) {
 			user_name = arg + 7;
 			continue;
 		}
-		if (!strncmp(arg, "--group=", 8)) {
+		if (!prefixcmp(arg, "--group=")) {
 			group_name = arg + 8;
 			continue;
 		}
-		if (!strncmp(arg, "--enable=", 9)) {
+		if (!prefixcmp(arg, "--enable=")) {
 			enable_service(arg + 9, 1);
 			continue;
 		}
-		if (!strncmp(arg, "--disable=", 10)) {
+		if (!prefixcmp(arg, "--disable=")) {
 			enable_service(arg + 10, 0);
 			continue;
 		}
-		if (!strncmp(arg, "--allow-override=", 17)) {
+		if (!prefixcmp(arg, "--allow-override=")) {
 			make_service_overridable(arg + 17, 1);
 			continue;
 		}
-		if (!strncmp(arg, "--forbid-override=", 18)) {
+		if (!prefixcmp(arg, "--forbid-override=")) {
 			make_service_overridable(arg + 18, 0);
 			continue;
 		}
diff --git a/diff.c b/diff.c
index 07589c3..fad13ab 100644
--- a/diff.c
+++ b/diff.c
@@ -77,7 +77,7 @@ int git_diff_ui_config(const char *var, const char *value)
 			diff_detect_rename_default = DIFF_DETECT_RENAME;
 		return 0;
 	}
-	if (!strncmp(var, "diff.color.", 11) || !strncmp(var, "color.diff.", 11)) {
+	if (!prefixcmp(var, "diff.color.") || !strncmp(var, "color.diff.", 11)) {
 		int slot = parse_diff_color_slot(var, 11);
 		color_parse(value, var, diff_colors[slot]);
 		return 0;
@@ -1119,9 +1119,9 @@ static void builtin_diff(const char *name_a,
 		xecfg.flags = XDL_EMIT_FUNCNAMES;
 		if (!diffopts)
 			;
-		else if (!strncmp(diffopts, "--unified=", 10))
+		else if (!prefixcmp(diffopts, "--unified="))
 			xecfg.ctxlen = strtoul(diffopts + 10, NULL, 10);
-		else if (!strncmp(diffopts, "-u", 2))
+		else if (!prefixcmp(diffopts, "-u"))
 			xecfg.ctxlen = strtoul(diffopts + 2, NULL, 10);
 		ecb.outf = xdiff_outf;
 		ecb.priv = &ecbdata;
@@ -1957,7 +1957,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 	else if (!strcmp(arg, "--shortstat")) {
 		options->output_format |= DIFF_FORMAT_SHORTSTAT;
 	}
-	else if (!strncmp(arg, "--stat", 6)) {
+	else if (!prefixcmp(arg, "--stat")) {
 		char *end;
 		int width = options->stat_width;
 		int name_width = options->stat_name_width;
@@ -1966,9 +1966,9 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 
 		switch (*arg) {
 		case '-':
-			if (!strncmp(arg, "-width=", 7))
+			if (!prefixcmp(arg, "-width="))
 				width = strtoul(arg + 7, &end, 10);
-			else if (!strncmp(arg, "-name-width=", 12))
+			else if (!prefixcmp(arg, "-name-width="))
 				name_width = strtoul(arg + 12, &end, 10);
 			break;
 		case '=':
@@ -1993,7 +1993,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 	}
 	else if (!strcmp(arg, "-z"))
 		options->line_termination = 0;
-	else if (!strncmp(arg, "-l", 2))
+	else if (!prefixcmp(arg, "-l"))
 		options->rename_limit = strtoul(arg+2, NULL, 10);
 	else if (!strcmp(arg, "--full-index"))
 		options->full_index = 1;
@@ -2010,31 +2010,31 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->output_format |= DIFF_FORMAT_NAME_STATUS;
 	else if (!strcmp(arg, "-R"))
 		options->reverse_diff = 1;
-	else if (!strncmp(arg, "-S", 2))
+	else if (!prefixcmp(arg, "-S"))
 		options->pickaxe = arg + 2;
 	else if (!strcmp(arg, "-s")) {
 		options->output_format |= DIFF_FORMAT_NO_OUTPUT;
 	}
-	else if (!strncmp(arg, "-O", 2))
+	else if (!prefixcmp(arg, "-O"))
 		options->orderfile = arg + 2;
-	else if (!strncmp(arg, "--diff-filter=", 14))
+	else if (!prefixcmp(arg, "--diff-filter="))
 		options->filter = arg + 14;
 	else if (!strcmp(arg, "--pickaxe-all"))
 		options->pickaxe_opts = DIFF_PICKAXE_ALL;
 	else if (!strcmp(arg, "--pickaxe-regex"))
 		options->pickaxe_opts = DIFF_PICKAXE_REGEX;
-	else if (!strncmp(arg, "-B", 2)) {
+	else if (!prefixcmp(arg, "-B")) {
 		if ((options->break_opt =
 		     diff_scoreopt_parse(arg)) == -1)
 			return -1;
 	}
-	else if (!strncmp(arg, "-M", 2)) {
+	else if (!prefixcmp(arg, "-M")) {
 		if ((options->rename_score =
 		     diff_scoreopt_parse(arg)) == -1)
 			return -1;
 		options->detect_rename = DIFF_DETECT_RENAME;
 	}
-	else if (!strncmp(arg, "-C", 2)) {
+	else if (!prefixcmp(arg, "-C")) {
 		if ((options->rename_score =
 		     diff_scoreopt_parse(arg)) == -1)
 			return -1;
@@ -2044,7 +2044,7 @@ int diff_opt_parse(struct diff_options *options, const char **av, int ac)
 		options->find_copies_harder = 1;
 	else if (!strcmp(arg, "--abbrev"))
 		options->abbrev = DEFAULT_ABBREV;
-	else if (!strncmp(arg, "--abbrev=", 9)) {
+	else if (!prefixcmp(arg, "--abbrev=")) {
 		options->abbrev = strtoul(arg + 9, NULL, 10);
 		if (options->abbrev < MINIMUM_ABBREV)
 			options->abbrev = MINIMUM_ABBREV;
@@ -2553,7 +2553,7 @@ static void patch_id_consume(void *priv, char *line, unsigned long len)
 	int new_len;
 
 	/* Ignore line numbers when computing the SHA1 of the patch */
-	if (!strncmp(line, "@@ -", 4))
+	if (!prefixcmp(line, "@@ -"))
 		return;
 
 	new_len = remove_space(line, len);
diff --git a/exec_cmd.c b/exec_cmd.c
index 3996bce..9b74ed2 100644
--- a/exec_cmd.c
+++ b/exec_cmd.c
@@ -56,7 +56,7 @@ int execv_git_cmd(const char **argv)
 			len = strlen(git_command);
 
 			/* Trivial cleanup */
-			while (!strncmp(exec_dir, "./", 2)) {
+			while (!prefixcmp(exec_dir, "./")) {
 				exec_dir += 2;
 				while (*exec_dir == '/')
 					exec_dir++;
diff --git a/fast-import.c b/fast-import.c
index ad32300..ee7c04c 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1397,7 +1397,7 @@ static void read_next_command(void)
 
 static void cmd_mark(void)
 {
-	if (!strncmp("mark :", command_buf.buf, 6)) {
+	if (!(-prefixcmp(command_buf.buf, "mark :"))) {
 		next_mark = strtoumax(command_buf.buf + 6, NULL, 10);
 		read_next_command();
 	}
@@ -1410,10 +1410,10 @@ static void *cmd_data (size_t *size)
 	size_t length;
 	char *buffer;
 
-	if (strncmp("data ", command_buf.buf, 5))
+	if ((-prefixcmp(command_buf.buf, "data ")))
 		die("Expected 'data n' command, found: %s", command_buf.buf);
 
-	if (!strncmp("<<", command_buf.buf + 5, 2)) {
+	if (!(-prefixcmp(command_buf.buf + 5, "<<"))) {
 		char *term = xstrdup(command_buf.buf + 5 + 2);
 		size_t sz = 8192, term_len = command_buf.len - 5 - 2;
 		length = 0;
@@ -1600,7 +1600,7 @@ static void file_change_m(struct branch *b)
 		oe = find_mark(strtoumax(p + 1, &x, 10));
 		hashcpy(sha1, oe->sha1);
 		p = x;
-	} else if (!strncmp("inline", p, 6)) {
+	} else if (!(-prefixcmp(p, "inline"))) {
 		inline_data = 1;
 		p += 6;
 	} else {
@@ -1673,7 +1673,7 @@ static void cmd_from(struct branch *b)
 	const char *from;
 	struct branch *s;
 
-	if (strncmp("from ", command_buf.buf, 5))
+	if ((-prefixcmp(command_buf.buf, "from ")))
 		return;
 
 	if (b->branch_tree.tree) {
@@ -1739,7 +1739,7 @@ static struct hash_list *cmd_merge(unsigned int *count)
 	struct branch *s;
 
 	*count = 0;
-	while (!strncmp("merge ", command_buf.buf, 6)) {
+	while (!(-prefixcmp(command_buf.buf, "merge "))) {
 		from = strchr(command_buf.buf, ' ') + 1;
 		n = xmalloc(sizeof(*n));
 		s = lookup_branch(from);
@@ -1785,11 +1785,11 @@ static void cmd_new_commit(void)
 
 	read_next_command();
 	cmd_mark();
-	if (!strncmp("author ", command_buf.buf, 7)) {
+	if (!(-prefixcmp(command_buf.buf, "author "))) {
 		author = parse_ident(command_buf.buf + 7);
 		read_next_command();
 	}
-	if (!strncmp("committer ", command_buf.buf, 10)) {
+	if (!(-prefixcmp(command_buf.buf, "committer "))) {
 		committer = parse_ident(command_buf.buf + 10);
 		read_next_command();
 	}
@@ -1810,9 +1810,9 @@ static void cmd_new_commit(void)
 	for (;;) {
 		if (1 == command_buf.len)
 			break;
-		else if (!strncmp("M ", command_buf.buf, 2))
+		else if (!(-prefixcmp(command_buf.buf, "M ")))
 			file_change_m(b);
-		else if (!strncmp("D ", command_buf.buf, 2))
+		else if (!(-prefixcmp(command_buf.buf, "D ")))
 			file_change_d(b);
 		else if (!strcmp("deleteall", command_buf.buf))
 			file_change_deleteall(b);
@@ -1882,7 +1882,7 @@ static void cmd_new_tag(void)
 	read_next_command();
 
 	/* from ... */
-	if (strncmp("from ", command_buf.buf, 5))
+	if ((-prefixcmp(command_buf.buf, "from ")))
 		die("Expected from command, got %s", command_buf.buf);
 	from = strchr(command_buf.buf, ' ') + 1;
 	s = lookup_branch(from);
@@ -1909,7 +1909,7 @@ static void cmd_new_tag(void)
 	read_next_command();
 
 	/* tagger ... */
-	if (strncmp("tagger ", command_buf.buf, 7))
+	if ((-prefixcmp(command_buf.buf, "tagger ")))
 		die("Expected tagger command, got %s", command_buf.buf);
 	tagger = parse_ident(command_buf.buf + 7);
 
@@ -1986,7 +1986,7 @@ int main(int argc, const char **argv)
 
 		if (*a != '-' || !strcmp(a, "--"))
 			break;
-		else if (!strncmp(a, "--date-format=", 14)) {
+		else if (!prefixcmp(a, "--date-format=")) {
 			const char *fmt = a + 14;
 			if (!strcmp(fmt, "raw"))
 				whenspec = WHENSPEC_RAW;
@@ -1997,15 +1997,15 @@ int main(int argc, const char **argv)
 			else
 				die("unknown --date-format argument %s", fmt);
 		}
-		else if (!strncmp(a, "--max-pack-size=", 16))
+		else if (!prefixcmp(a, "--max-pack-size="))
 			max_packsize = strtoumax(a + 16, NULL, 0) * 1024 * 1024;
-		else if (!strncmp(a, "--depth=", 8))
+		else if (!prefixcmp(a, "--depth="))
 			max_depth = strtoul(a + 8, NULL, 0);
-		else if (!strncmp(a, "--active-branches=", 18))
+		else if (!prefixcmp(a, "--active-branches="))
 			max_active_branches = strtoul(a + 18, NULL, 0);
-		else if (!strncmp(a, "--export-marks=", 15))
+		else if (!prefixcmp(a, "--export-marks="))
 			mark_file = a + 15;
-		else if (!strncmp(a, "--export-pack-edges=", 20)) {
+		else if (!prefixcmp(a, "--export-pack-edges=")) {
 			if (pack_edges)
 				fclose(pack_edges);
 			pack_edges = fopen(a + 20, "a");
@@ -2038,11 +2038,11 @@ int main(int argc, const char **argv)
 			break;
 		else if (!strcmp("blob", command_buf.buf))
 			cmd_new_blob();
-		else if (!strncmp("commit ", command_buf.buf, 7))
+		else if (!(-prefixcmp(command_buf.buf, "commit ")))
 			cmd_new_commit();
-		else if (!strncmp("tag ", command_buf.buf, 4))
+		else if (!(-prefixcmp(command_buf.buf, "tag ")))
 			cmd_new_tag();
-		else if (!strncmp("reset ", command_buf.buf, 6))
+		else if (!(-prefixcmp(command_buf.buf, "reset ")))
 			cmd_reset_branch();
 		else if (!strcmp("checkpoint", command_buf.buf))
 			cmd_checkpoint();
diff --git a/fetch-pack.c b/fetch-pack.c
index c787106..1fd2c3a 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -198,13 +198,13 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 		int len;
 
 		while ((len = packet_read_line(fd[0], line, sizeof(line)))) {
-			if (!strncmp("shallow ", line, 8)) {
+			if (!(-prefixcmp(line, "shallow "))) {
 				if (get_sha1_hex(line + 8, sha1))
 					die("invalid shallow line: %s", line);
 				register_shallow(sha1);
 				continue;
 			}
-			if (!strncmp("unshallow ", line, 10)) {
+			if (!(-prefixcmp(line, "unshallow "))) {
 				if (get_sha1_hex(line + 10, sha1))
 					die("invalid unshallow line: %s", line);
 				if (!lookup_object(sha1))
@@ -346,7 +346,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
 		    check_ref_format(ref->name + 5))
 			; /* trash */
 		else if (fetch_all &&
-			 (!depth || strncmp(ref->name, "refs/tags/", 10) )) {
+			 (!depth || prefixcmp(ref->name, "refs/tags/") )) {
 			*newtail = ref;
 			ref->next = NULL;
 			newtail = &ref->next;
@@ -683,11 +683,11 @@ int main(int argc, char **argv)
 		char *arg = argv[i];
 
 		if (*arg == '-') {
-			if (!strncmp("--upload-pack=", arg, 14)) {
+			if (!(-prefixcmp(arg, "--upload-pack="))) {
 				uploadpack = arg + 14;
 				continue;
 			}
-			if (!strncmp("--exec=", arg, 7)) {
+			if (!(-prefixcmp(arg, "--exec="))) {
 				uploadpack = arg + 7;
 				continue;
 			}
@@ -712,7 +712,7 @@ int main(int argc, char **argv)
 				verbose = 1;
 				continue;
 			}
-			if (!strncmp("--depth=", arg, 8)) {
+			if (!(-prefixcmp(arg, "--depth="))) {
 				depth = strtol(arg + 8, NULL, 0);
 				if (stat(git_path("shallow"), &st))
 					st.st_mtime = 0;
diff --git a/git.c b/git.c
index 4dd1967..1fad852 100644
--- a/git.c
+++ b/git.c
@@ -48,7 +48,7 @@ static int handle_options(const char*** argv, int* argc)
 		/*
 		 * Check remaining flags.
 		 */
-		if (!strncmp(cmd, "--exec-path", 11)) {
+		if (!prefixcmp(cmd, "--exec-path")) {
 			cmd += 11;
 			if (*cmd == '=')
 				git_set_exec_path(cmd + 1);
@@ -66,7 +66,7 @@ static int handle_options(const char*** argv, int* argc)
 			setenv(GIT_DIR_ENVIRONMENT, (*argv)[1], 1);
 			(*argv)++;
 			(*argc)--;
-		} else if (!strncmp(cmd, "--git-dir=", 10)) {
+		} else if (!prefixcmp(cmd, "--git-dir=")) {
 			setenv(GIT_DIR_ENVIRONMENT, cmd + 10, 1);
 		} else if (!strcmp(cmd, "--bare")) {
 			static char git_dir[PATH_MAX+1];
@@ -88,7 +88,7 @@ static char *alias_string;
 
 static int git_alias_config(const char *var, const char *value)
 {
-	if (!strncmp(var, "alias.", 6) && !strcmp(var + 6, alias_command)) {
+	if (!prefixcmp(var, "alias.") && !strcmp(var + 6, alias_command)) {
 		alias_string = xstrdup(value);
 	}
 	return 0;
@@ -348,7 +348,7 @@ int main(int argc, const char **argv, char **envp)
 	 * So we just directly call the internal command handler, and
 	 * die if that one cannot handle it.
 	 */
-	if (!strncmp(cmd, "git-", 4)) {
+	if (!prefixcmp(cmd, "git-")) {
 		cmd += 4;
 		argv[0] = cmd;
 		handle_internal_command(argc, argv, envp);
@@ -360,7 +360,7 @@ int main(int argc, const char **argv, char **envp)
 	argc--;
 	handle_options(&argv, &argc);
 	if (argc > 0) {
-		if (!strncmp(argv[0], "--", 2))
+		if (!prefixcmp(argv[0], "--"))
 			argv[0] += 2;
 	} else {
 		/* Default command: "help" */
diff --git a/help.c b/help.c
index b667463..0893fea 100644
--- a/help.c
+++ b/help.c
@@ -130,7 +130,7 @@ static void list_commands(const char *exec_path, const char *pattern)
 		struct stat st;
 		int entlen;
 
-		if (strncmp(de->d_name, "git-", 4))
+		if (prefixcmp(de->d_name, "git-"))
 			continue;
 		strcpy(path+dirlen, de->d_name);
 		if (stat(path, &st) || /* stat, not lstat */
@@ -179,7 +179,7 @@ static void show_man_page(const char *git_cmd)
 {
 	const char *page;
 
-	if (!strncmp(git_cmd, "git", 3))
+	if (!prefixcmp(git_cmd, "git"))
 		page = git_cmd;
 	else {
 		int page_len = strlen(git_cmd) + 4;
diff --git a/http-fetch.c b/http-fetch.c
index 9f790a0..d9a4561 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -717,7 +717,7 @@ static int fetch_indices(struct alt_base *repo)
 		case 'P':
 			i++;
 			if (i + 52 <= buffer.posn &&
-			    !strncmp(data + i, " pack-", 6) &&
+			    !prefixcmp(data + i, " pack-") &&
 			    !strncmp(data + i + 46, ".pack\n", 6)) {
 				get_sha1_hex(data + i + 6, sha1);
 				setup_index(repo, sha1);
diff --git a/http-push.c b/http-push.c
index b128c01..eb77c9a 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1060,7 +1060,7 @@ static int fetch_indices(void)
 		case 'P':
 			i++;
 			if (i + 52 < buffer.posn &&
-			    !strncmp(data + i, " pack-", 6) &&
+			    !prefixcmp(data + i, " pack-") &&
 			    !strncmp(data + i + 46, ".pack\n", 6)) {
 				get_sha1_hex(data + i + 6, sha1);
 				setup_index(sha1);
@@ -1206,11 +1206,11 @@ 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 (!strncmp(ctx->cdata, "Second-", 7))
+			if (!prefixcmp(ctx->cdata, "Second-"))
 				lock->timeout =
 					strtol(ctx->cdata + 7, NULL, 10);
 		} else if (!strcmp(ctx->name, DAV_ACTIVELOCK_TOKEN)) {
-			if (!strncmp(ctx->cdata, "opaquelocktoken:", 16)) {
+			if (!prefixcmp(ctx->cdata, "opaquelocktoken:")) {
 				lock->token = xmalloc(strlen(ctx->cdata) - 15);
 				strcpy(lock->token, ctx->cdata + 16);
 			}
@@ -2168,7 +2168,7 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1)
 		return;
 
 	/* If it's a symref, set the refname; otherwise try for a sha1 */
-	if (!strncmp((char *)buffer.buffer, "ref: ", 5)) {
+	if (!prefixcmp((char *)buffer.buffer, "ref: ")) {
 		*symref = xmalloc(buffer.posn - 5);
 		strlcpy(*symref, (char *)buffer.buffer + 5, buffer.posn - 5);
 	} else {
diff --git a/index-pack.c b/index-pack.c
index 72e0962..fa9a0e7 100644
--- a/index-pack.c
+++ b/index-pack.c
@@ -849,9 +849,9 @@ int main(int argc, char **argv)
 				fix_thin_pack = 1;
 			} else if (!strcmp(arg, "--keep")) {
 				keep_msg = "";
-			} else if (!strncmp(arg, "--keep=", 7)) {
+			} else if (!prefixcmp(arg, "--keep=")) {
 				keep_msg = arg + 7;
-			} else if (!strncmp(arg, "--pack_header=", 14)) {
+			} else if (!prefixcmp(arg, "--pack_header=")) {
 				struct pack_header *hdr;
 				char *c;
 
diff --git a/peek-remote.c b/peek-remote.c
index ef3c76c..7b66228 100644
--- a/peek-remote.c
+++ b/peek-remote.c
@@ -35,11 +35,11 @@ int main(int argc, char **argv)
 		char *arg = argv[i];
 
 		if (*arg == '-') {
-			if (!strncmp("--upload-pack=", arg, 14)) {
+			if (!(-prefixcmp(arg, "--upload-pack="))) {
 				uploadpack = arg + 14;
 				continue;
 			}
-			if (!strncmp("--exec=", arg, 7)) {
+			if (!(-prefixcmp(arg, "--exec="))) {
 				uploadpack = arg + 7;
 				continue;
 			}
diff --git a/receive-pack.c b/receive-pack.c
index 7311c82..7f1dcc0 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -109,7 +109,7 @@ static int update(struct command *cmd)
 	struct ref_lock *lock;
 
 	cmd->error_string = NULL;
-	if (!strncmp(name, "refs/", 5) && check_ref_format(name + 5)) {
+	if (!prefixcmp(name, "refs/") && check_ref_format(name + 5)) {
 		cmd->error_string = "funny refname";
 		return error("refusing to create funny ref '%s' locally",
 			     name);
@@ -125,7 +125,7 @@ static int update(struct command *cmd)
 	}
 	if (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&
 	    !is_null_sha1(old_sha1) &&
-	    !strncmp(name, "refs/heads/", 11)) {
+	    !prefixcmp(name, "refs/heads/")) {
 		struct commit *old_commit, *new_commit;
 		struct commit_list *bases, *ent;
 
diff --git a/refs.c b/refs.c
index 6387703..d347876 100644
--- a/refs.c
+++ b/refs.c
@@ -828,8 +828,8 @@ int rename_ref(const char *oldref, const char *newref, const char *logmsg)
 		goto rollback;
 	}
 
-	if (!strncmp(oldref, "refs/heads/", 11) &&
-			!strncmp(newref, "refs/heads/", 11)) {
+	if (!prefixcmp(oldref, "refs/heads/") &&
+			!prefixcmp(newref, "refs/heads/")) {
 		char oldsection[1024], newsection[1024];
 
 		snprintf(oldsection, 1024, "branch.%s", oldref + 11);
@@ -894,8 +894,8 @@ static int log_ref_write(const char *ref_name, const unsigned char *old_sha1,
 	log_file = git_path("logs/%s", ref_name);
 
 	if (log_all_ref_updates &&
-	    (!strncmp(ref_name, "refs/heads/", 11) ||
-	     !strncmp(ref_name, "refs/remotes/", 13) ||
+	    (!prefixcmp(ref_name, "refs/heads/") ||
+	     !prefixcmp(ref_name, "refs/remotes/") ||
 	     !strcmp(ref_name, "HEAD"))) {
 		if (safe_create_leading_directories(log_file) < 0)
 			return error("unable to create directory for %s",
diff --git a/revision.c b/revision.c
index 5b1794b..abab3b9 100644
--- a/revision.c
+++ b/revision.c
@@ -813,11 +813,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 		const char *arg = argv[i];
 		if (*arg == '-') {
 			int opts;
-			if (!strncmp(arg, "--max-count=", 12)) {
+			if (!prefixcmp(arg, "--max-count=")) {
 				revs->max_count = atoi(arg + 12);
 				continue;
 			}
-			if (!strncmp(arg, "--skip=", 7)) {
+			if (!prefixcmp(arg, "--skip=")) {
 				revs->skip_count = atoi(arg + 7);
 				continue;
 			}
@@ -836,27 +836,27 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				revs->max_count = atoi(arg + 2);
 				continue;
 			}
-			if (!strncmp(arg, "--max-age=", 10)) {
+			if (!prefixcmp(arg, "--max-age=")) {
 				revs->max_age = atoi(arg + 10);
 				continue;
 			}
-			if (!strncmp(arg, "--since=", 8)) {
+			if (!prefixcmp(arg, "--since=")) {
 				revs->max_age = approxidate(arg + 8);
 				continue;
 			}
-			if (!strncmp(arg, "--after=", 8)) {
+			if (!prefixcmp(arg, "--after=")) {
 				revs->max_age = approxidate(arg + 8);
 				continue;
 			}
-			if (!strncmp(arg, "--min-age=", 10)) {
+			if (!prefixcmp(arg, "--min-age=")) {
 				revs->min_age = atoi(arg + 10);
 				continue;
 			}
-			if (!strncmp(arg, "--before=", 9)) {
+			if (!prefixcmp(arg, "--before=")) {
 				revs->min_age = approxidate(arg + 9);
 				continue;
 			}
-			if (!strncmp(arg, "--until=", 8)) {
+			if (!prefixcmp(arg, "--until=")) {
 				revs->min_age = approxidate(arg + 8);
 				continue;
 			}
@@ -944,7 +944,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				revs->num_ignore_packed = 0;
 				continue;
 			}
-			if (!strncmp(arg, "--unpacked=", 11)) {
+			if (!prefixcmp(arg, "--unpacked=")) {
 				revs->unpacked = 1;
 				add_ignore_packed(revs, arg+11);
 				continue;
@@ -980,7 +980,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				revs->verbose_header = 1;
 				continue;
 			}
-			if (!strncmp(arg, "--pretty", 8)) {
+			if (!prefixcmp(arg, "--pretty")) {
 				revs->verbose_header = 1;
 				revs->commit_format = get_commit_format(arg+8);
 				continue;
@@ -1005,7 +1005,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				revs->abbrev = DEFAULT_ABBREV;
 				continue;
 			}
-			if (!strncmp(arg, "--abbrev=", 9)) {
+			if (!prefixcmp(arg, "--abbrev=")) {
 				revs->abbrev = strtoul(arg + 9, NULL, 10);
 				if (revs->abbrev < MINIMUM_ABBREV)
 					revs->abbrev = MINIMUM_ABBREV;
@@ -1034,15 +1034,15 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 			/*
 			 * Grepping the commit log
 			 */
-			if (!strncmp(arg, "--author=", 9)) {
+			if (!prefixcmp(arg, "--author=")) {
 				add_header_grep(revs, "author", arg+9);
 				continue;
 			}
-			if (!strncmp(arg, "--committer=", 12)) {
+			if (!prefixcmp(arg, "--committer=")) {
 				add_header_grep(revs, "committer", arg+12);
 				continue;
 			}
-			if (!strncmp(arg, "--grep=", 7)) {
+			if (!prefixcmp(arg, "--grep=")) {
 				add_message_grep(revs, arg+7);
 				continue;
 			}
@@ -1050,7 +1050,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				all_match = 1;
 				continue;
 			}
-			if (!strncmp(arg, "--encoding=", 11)) {
+			if (!prefixcmp(arg, "--encoding=")) {
 				arg += 11;
 				if (strcmp(arg, "none"))
 					git_log_output_encoding = strdup(arg);
diff --git a/send-pack.c b/send-pack.c
index 33e69db..512b660 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -379,11 +379,11 @@ int main(int argc, char **argv)
 		char *arg = *argv;
 
 		if (*arg == '-') {
-			if (!strncmp(arg, "--receive-pack=", 15)) {
+			if (!prefixcmp(arg, "--receive-pack=")) {
 				receivepack = arg + 15;
 				continue;
 			}
-			if (!strncmp(arg, "--exec=", 7)) {
+			if (!prefixcmp(arg, "--exec=")) {
 				receivepack = arg + 7;
 				continue;
 			}
diff --git a/setup.c b/setup.c
index e9d3f5a..dda67d2 100644
--- a/setup.c
+++ b/setup.c
@@ -251,7 +251,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	offset++;
 	cwd[len++] = '/';
 	cwd[len] = 0;
-	inside_git_dir = !strncmp(cwd + offset, ".git/", 5);
+	inside_git_dir = !prefixcmp(cwd + offset, ".git/");
 	return cwd + offset;
 }
 
diff --git a/shell.c b/shell.c
index 8c08cf0..c983fc7 100644
--- a/shell.c
+++ b/shell.c
@@ -8,7 +8,7 @@ static int do_generic_cmd(const char *me, char *arg)
 
 	if (!arg || !(arg = sq_dequote(arg)))
 		die("bad argument");
-	if (strncmp(me, "git-", 4))
+	if (prefixcmp(me, "git-"))
 		die("bad command");
 
 	my_argv[0] = me + 4;
diff --git a/upload-pack.c b/upload-pack.c
index 3648aae..d7876ca 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -455,7 +455,7 @@ static int get_common_commits(void)
 			continue;
 		}
 		len = strip(line, len);
-		if (!strncmp(line, "have ", 5)) {
+		if (!prefixcmp(line, "have ")) {
 			switch (got_sha1(line+5, sha1)) {
 			case -1: /* they have what we do not */
 				if (multi_ack && ok_to_give_up())
@@ -502,7 +502,7 @@ static void receive_needs(void)
 		if (!len)
 			break;
 
-		if (!strncmp("shallow ", line, 8)) {
+		if (!(-prefixcmp(line, "shallow "))) {
 			unsigned char sha1[20];
 			struct object *object;
 			use_thin_pack = 0;
@@ -515,7 +515,7 @@ static void receive_needs(void)
 			add_object_array(object, NULL, &shallows);
 			continue;
 		}
-		if (!strncmp("deepen ", line, 7)) {
+		if (!(-prefixcmp(line, "deepen "))) {
 			char *end;
 			use_thin_pack = 0;
 			depth = strtol(line + 7, &end, 0);
@@ -523,7 +523,7 @@ static void receive_needs(void)
 				die("Invalid deepen: %s", line);
 			continue;
 		}
-		if (strncmp("want ", line, 5) ||
+		if ((-prefixcmp(line, "want ")) ||
 		    get_sha1_hex(line+5, sha1_buf))
 			die("git-upload-pack: protocol error, "
 			    "expected to get sha, not '%s'", line);
@@ -656,7 +656,7 @@ int main(int argc, char **argv)
 			strict = 1;
 			continue;
 		}
-		if (!strncmp(arg, "--timeout=", 10)) {
+		if (!prefixcmp(arg, "--timeout=")) {
 			timeout = atoi(arg+10);
 			continue;
 		}
diff --git a/wt-status.c b/wt-status.c
index 2879c3d..d17a6ba 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -298,7 +298,7 @@ void wt_status_print(struct wt_status *s)
 	if (s->branch) {
 		const char *on_what = "On branch ";
 		const char *branch_name = s->branch;
-		if (!strncmp(branch_name, "refs/heads/", 11))
+		if (!prefixcmp(branch_name, "refs/heads/"))
 			branch_name += 11;
 		else if (!strcmp(branch_name, "HEAD")) {
 			branch_name = "";
@@ -344,7 +344,7 @@ int git_status_config(const char *k, const char *v)
 		wt_status_use_color = git_config_colorbool(k, v);
 		return 0;
 	}
-	if (!strncmp(k, "status.color.", 13) || !strncmp(k, "color.status.", 13)) {
+	if (!prefixcmp(k, "status.color.") || !strncmp(k, "color.status.", 13)) {
 		int slot = parse_status_slot(k, 13);
 		color_parse(v, k, wt_status_colors[slot]);
 	}
-- 
1.5.0.1.571.ge5a1a

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 3/4] prefixcmp(): fix-up mechanical conversion.
  2007-02-20  9:42   ` Andy Parkins
                       ` (2 preceding siblings ...)
  2007-02-20  9:53     ` [PATCH 2/4] Mechanical conversion to use prefixcmp() Junio C Hamano
@ 2007-02-20  9:54     ` Junio C Hamano
  2007-02-20  9:55     ` [PATCH 4/4] prefixcmp(): fix-up leftover strncmp() Junio C Hamano
  4 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2007-02-20  9:54 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git

Previous step converted use of strncmp() with literal string
mechanically even when the result is only used as a boolean (not
comparison):

	if (!strncmp("foo", arg, 3)) ==> if (!(-prefixcmp(arg, "foo")))

This step manually cleans them up.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 builtin-archive.c      |    2 +-
 builtin-blame.c        |    6 +++---
 builtin-grep.c         |    6 +++---
 builtin-pack-objects.c |    6 +++---
 builtin-push.c         |    4 ++--
 builtin-rerere.c       |    6 +++---
 builtin-show-branch.c  |    4 ++--
 builtin-tar-tree.c     |    2 +-
 daemon.c               |    2 +-
 fast-import.c          |   30 +++++++++++++++---------------
 fetch-pack.c           |   10 +++++-----
 peek-remote.c          |    4 ++--
 upload-pack.c          |    6 +++---
 13 files changed, 44 insertions(+), 44 deletions(-)

diff --git a/builtin-archive.c b/builtin-archive.c
index 0c56de0..8ea6cb1 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -35,7 +35,7 @@ static int run_remote_archiver(const char *remote, int argc,
 
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
-		if (!(-prefixcmp(arg, "--exec="))) {
+		if (!prefixcmp(arg, "--exec=")) {
 			if (exec_at)
 				die("multiple --exec specified");
 			exec = arg + 7;
diff --git a/builtin-blame.c b/builtin-blame.c
index db311bf..530b97f 100644
--- a/builtin-blame.c
+++ b/builtin-blame.c
@@ -2097,17 +2097,17 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
 			output_option |= OUTPUT_LONG_OBJECT_NAME;
 		else if (!strcmp("-S", arg) && ++i < argc)
 			revs_file = argv[i];
-		else if (!(-prefixcmp(arg, "-M"))) {
+		else if (!prefixcmp(arg, "-M")) {
 			opt |= PICKAXE_BLAME_MOVE;
 			blame_move_score = parse_score(arg+2);
 		}
-		else if (!(-prefixcmp(arg, "-C"))) {
+		else if (!prefixcmp(arg, "-C")) {
 			if (opt & PICKAXE_BLAME_COPY)
 				opt |= PICKAXE_BLAME_COPY_HARDER;
 			opt |= PICKAXE_BLAME_COPY | PICKAXE_BLAME_MOVE;
 			blame_copy_score = parse_score(arg+2);
 		}
-		else if (!(-prefixcmp(arg, "-L"))) {
+		else if (!prefixcmp(arg, "-L")) {
 			if (!arg[2]) {
 				if (++i >= argc)
 					usage(blame_usage);
diff --git a/builtin-grep.c b/builtin-grep.c
index cec2204..f35f2d0 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -527,9 +527,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			opt.word_regexp = 1;
 			continue;
 		}
-		if (!(-prefixcmp(arg, "-A")) ||
-		    !(-prefixcmp(arg, "-B")) ||
-		    !(-prefixcmp(arg, "-C")) ||
+		if (!prefixcmp(arg, "-A") ||
+		    !prefixcmp(arg, "-B") ||
+		    !prefixcmp(arg, "-C") ||
 		    (arg[0] == '-' && '1' <= arg[1] && arg[1] <= '9')) {
 			unsigned num;
 			const char *scan;
diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index 71113d8..b5ed9ce 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -1579,14 +1579,14 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			incremental = 1;
 			continue;
 		}
-		if (!(-prefixcmp(arg, "--window="))) {
+		if (!prefixcmp(arg, "--window=")) {
 			char *end;
 			window = strtoul(arg+9, &end, 0);
 			if (!arg[9] || *end)
 				usage(pack_usage);
 			continue;
 		}
-		if (!(-prefixcmp(arg, "--depth="))) {
+		if (!prefixcmp(arg, "--depth=")) {
 			char *end;
 			depth = strtoul(arg+8, &end, 0);
 			if (!arg[8] || *end)
@@ -1622,7 +1622,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!strcmp("--unpacked", arg) ||
-		    !(-prefixcmp(arg, "--unpacked=")) ||
+		    !prefixcmp(arg, "--unpacked=") ||
 		    !strcmp("--reflog", arg) ||
 		    !strcmp("--all", arg)) {
 			use_internal_rev_list = 1;
diff --git a/builtin-push.c b/builtin-push.c
index 2b98ba3..979efcc 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -149,10 +149,10 @@ static int get_remotes_uri(const char *repo, const char *uri[MAX_URI])
 		int is_refspec;
 		char *s, *p;
 
-		if (!(-prefixcmp(buffer, "URL:"))) {
+		if (!prefixcmp(buffer, "URL:")) {
 			is_refspec = 0;
 			s = buffer + 4;
-		} else if (!(-prefixcmp(buffer, "Push:"))) {
+		} else if (!prefixcmp(buffer, "Push:")) {
 			is_refspec = 1;
 			s = buffer + 5;
 		} else
diff --git a/builtin-rerere.c b/builtin-rerere.c
index 978105b..dd1d4c1 100644
--- a/builtin-rerere.c
+++ b/builtin-rerere.c
@@ -105,11 +105,11 @@ static int handle_file(const char *path,
 		SHA1_Init(&ctx);
 
 	while (fgets(buf, sizeof(buf), f)) {
-		if (!(-prefixcmp(buf, "<<<<<<< ")))
+		if (!prefixcmp(buf, "<<<<<<< "))
 			hunk = 1;
-		else if (!(-prefixcmp(buf, "=======")))
+		else if (!prefixcmp(buf, "======="))
 			hunk = 2;
-		else if (!(-prefixcmp(buf, ">>>>>>> "))) {
+		else if (!prefixcmp(buf, ">>>>>>> ")) {
 			hunk_no++;
 			hunk = 0;
 			if (memcmp(one->ptr, two->ptr, one->nr < two->nr ?
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index bf6aee4..402a8f7 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -435,9 +435,9 @@ static int append_matching_ref(const char *refname, const unsigned char *sha1, i
 		return 0;
 	if (fnmatch(match_ref_pattern, tail, 0))
 		return 0;
-	if (!(-prefixcmp(refname, "refs/heads/")))
+	if (!prefixcmp(refname, "refs/heads/"))
 		return append_head_ref(refname, sha1, flag, cb_data);
-	if (!(-prefixcmp(refname, "refs/tags/")))
+	if (!prefixcmp(refname, "refs/tags/"))
 		return append_tag_ref(refname, sha1, flag, cb_data);
 	return append_ref(refname, sha1, 0);
 }
diff --git a/builtin-tar-tree.c b/builtin-tar-tree.c
index 28f8c1c..b04719e 100644
--- a/builtin-tar-tree.c
+++ b/builtin-tar-tree.c
@@ -31,7 +31,7 @@ int cmd_tar_tree(int argc, const char **argv, const char *prefix)
 	nargv[nargc++] = "git-archive";
 	nargv[nargc++] = "--format=tar";
 
-	if (2 <= argc && !(-prefixcmp(argv[1], "--remote="))) {
+	if (2 <= argc && !prefixcmp(argv[1], "--remote=")) {
 		nargv[nargc++] = argv[1];
 		argv++;
 		argc--;
diff --git a/daemon.c b/daemon.c
index cdbc23f..e74ecac 100644
--- a/daemon.c
+++ b/daemon.c
@@ -562,7 +562,7 @@ static int execute(struct sockaddr *addr)
 	for (i = 0; i < ARRAY_SIZE(daemon_service); i++) {
 		struct daemon_service *s = &(daemon_service[i]);
 		int namelen = strlen(s->name);
-		if (!(-prefixcmp(line, "git-")) &&
+		if (!prefixcmp(line, "git-") &&
 		    !strncmp(s->name, line + 4, namelen) &&
 		    line[namelen + 4] == ' ') {
 			/*
diff --git a/fast-import.c b/fast-import.c
index ee7c04c..580eadc 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1397,7 +1397,7 @@ static void read_next_command(void)
 
 static void cmd_mark(void)
 {
-	if (!(-prefixcmp(command_buf.buf, "mark :"))) {
+	if (!prefixcmp(command_buf.buf, "mark :")) {
 		next_mark = strtoumax(command_buf.buf + 6, NULL, 10);
 		read_next_command();
 	}
@@ -1410,10 +1410,10 @@ static void *cmd_data (size_t *size)
 	size_t length;
 	char *buffer;
 
-	if ((-prefixcmp(command_buf.buf, "data ")))
+	if (prefixcmp(command_buf.buf, "data "))
 		die("Expected 'data n' command, found: %s", command_buf.buf);
 
-	if (!(-prefixcmp(command_buf.buf + 5, "<<"))) {
+	if (!prefixcmp(command_buf.buf + 5, "<<")) {
 		char *term = xstrdup(command_buf.buf + 5 + 2);
 		size_t sz = 8192, term_len = command_buf.len - 5 - 2;
 		length = 0;
@@ -1600,7 +1600,7 @@ static void file_change_m(struct branch *b)
 		oe = find_mark(strtoumax(p + 1, &x, 10));
 		hashcpy(sha1, oe->sha1);
 		p = x;
-	} else if (!(-prefixcmp(p, "inline"))) {
+	} else if (!prefixcmp(p, "inline")) {
 		inline_data = 1;
 		p += 6;
 	} else {
@@ -1673,7 +1673,7 @@ static void cmd_from(struct branch *b)
 	const char *from;
 	struct branch *s;
 
-	if ((-prefixcmp(command_buf.buf, "from ")))
+	if (prefixcmp(command_buf.buf, "from "))
 		return;
 
 	if (b->branch_tree.tree) {
@@ -1739,7 +1739,7 @@ static struct hash_list *cmd_merge(unsigned int *count)
 	struct branch *s;
 
 	*count = 0;
-	while (!(-prefixcmp(command_buf.buf, "merge "))) {
+	while (!prefixcmp(command_buf.buf, "merge ")) {
 		from = strchr(command_buf.buf, ' ') + 1;
 		n = xmalloc(sizeof(*n));
 		s = lookup_branch(from);
@@ -1785,11 +1785,11 @@ static void cmd_new_commit(void)
 
 	read_next_command();
 	cmd_mark();
-	if (!(-prefixcmp(command_buf.buf, "author "))) {
+	if (!prefixcmp(command_buf.buf, "author ")) {
 		author = parse_ident(command_buf.buf + 7);
 		read_next_command();
 	}
-	if (!(-prefixcmp(command_buf.buf, "committer "))) {
+	if (!prefixcmp(command_buf.buf, "committer ")) {
 		committer = parse_ident(command_buf.buf + 10);
 		read_next_command();
 	}
@@ -1810,9 +1810,9 @@ static void cmd_new_commit(void)
 	for (;;) {
 		if (1 == command_buf.len)
 			break;
-		else if (!(-prefixcmp(command_buf.buf, "M ")))
+		else if (!prefixcmp(command_buf.buf, "M "))
 			file_change_m(b);
-		else if (!(-prefixcmp(command_buf.buf, "D ")))
+		else if (!prefixcmp(command_buf.buf, "D "))
 			file_change_d(b);
 		else if (!strcmp("deleteall", command_buf.buf))
 			file_change_deleteall(b);
@@ -1882,7 +1882,7 @@ static void cmd_new_tag(void)
 	read_next_command();
 
 	/* from ... */
-	if ((-prefixcmp(command_buf.buf, "from ")))
+	if (prefixcmp(command_buf.buf, "from "))
 		die("Expected from command, got %s", command_buf.buf);
 	from = strchr(command_buf.buf, ' ') + 1;
 	s = lookup_branch(from);
@@ -1909,7 +1909,7 @@ static void cmd_new_tag(void)
 	read_next_command();
 
 	/* tagger ... */
-	if ((-prefixcmp(command_buf.buf, "tagger ")))
+	if (prefixcmp(command_buf.buf, "tagger "))
 		die("Expected tagger command, got %s", command_buf.buf);
 	tagger = parse_ident(command_buf.buf + 7);
 
@@ -2038,11 +2038,11 @@ int main(int argc, const char **argv)
 			break;
 		else if (!strcmp("blob", command_buf.buf))
 			cmd_new_blob();
-		else if (!(-prefixcmp(command_buf.buf, "commit ")))
+		else if (!prefixcmp(command_buf.buf, "commit "))
 			cmd_new_commit();
-		else if (!(-prefixcmp(command_buf.buf, "tag ")))
+		else if (!prefixcmp(command_buf.buf, "tag "))
 			cmd_new_tag();
-		else if (!(-prefixcmp(command_buf.buf, "reset ")))
+		else if (!prefixcmp(command_buf.buf, "reset "))
 			cmd_reset_branch();
 		else if (!strcmp("checkpoint", command_buf.buf))
 			cmd_checkpoint();
diff --git a/fetch-pack.c b/fetch-pack.c
index 1fd2c3a..41bdd27 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -198,13 +198,13 @@ static int find_common(int fd[2], unsigned char *result_sha1,
 		int len;
 
 		while ((len = packet_read_line(fd[0], line, sizeof(line)))) {
-			if (!(-prefixcmp(line, "shallow "))) {
+			if (!prefixcmp(line, "shallow ")) {
 				if (get_sha1_hex(line + 8, sha1))
 					die("invalid shallow line: %s", line);
 				register_shallow(sha1);
 				continue;
 			}
-			if (!(-prefixcmp(line, "unshallow "))) {
+			if (!prefixcmp(line, "unshallow ")) {
 				if (get_sha1_hex(line + 10, sha1))
 					die("invalid unshallow line: %s", line);
 				if (!lookup_object(sha1))
@@ -683,11 +683,11 @@ int main(int argc, char **argv)
 		char *arg = argv[i];
 
 		if (*arg == '-') {
-			if (!(-prefixcmp(arg, "--upload-pack="))) {
+			if (!prefixcmp(arg, "--upload-pack=")) {
 				uploadpack = arg + 14;
 				continue;
 			}
-			if (!(-prefixcmp(arg, "--exec="))) {
+			if (!prefixcmp(arg, "--exec=")) {
 				uploadpack = arg + 7;
 				continue;
 			}
@@ -712,7 +712,7 @@ int main(int argc, char **argv)
 				verbose = 1;
 				continue;
 			}
-			if (!(-prefixcmp(arg, "--depth="))) {
+			if (!prefixcmp(arg, "--depth=")) {
 				depth = strtol(arg + 8, NULL, 0);
 				if (stat(git_path("shallow"), &st))
 					st.st_mtime = 0;
diff --git a/peek-remote.c b/peek-remote.c
index 7b66228..96bfac4 100644
--- a/peek-remote.c
+++ b/peek-remote.c
@@ -35,11 +35,11 @@ int main(int argc, char **argv)
 		char *arg = argv[i];
 
 		if (*arg == '-') {
-			if (!(-prefixcmp(arg, "--upload-pack="))) {
+			if (!prefixcmp(arg, "--upload-pack=")) {
 				uploadpack = arg + 14;
 				continue;
 			}
-			if (!(-prefixcmp(arg, "--exec="))) {
+			if (!prefixcmp(arg, "--exec=")) {
 				uploadpack = arg + 7;
 				continue;
 			}
diff --git a/upload-pack.c b/upload-pack.c
index d7876ca..804bbb6 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -502,7 +502,7 @@ static void receive_needs(void)
 		if (!len)
 			break;
 
-		if (!(-prefixcmp(line, "shallow "))) {
+		if (!prefixcmp(line, "shallow ")) {
 			unsigned char sha1[20];
 			struct object *object;
 			use_thin_pack = 0;
@@ -515,7 +515,7 @@ static void receive_needs(void)
 			add_object_array(object, NULL, &shallows);
 			continue;
 		}
-		if (!(-prefixcmp(line, "deepen "))) {
+		if (!prefixcmp(line, "deepen ")) {
 			char *end;
 			use_thin_pack = 0;
 			depth = strtol(line + 7, &end, 0);
@@ -523,7 +523,7 @@ static void receive_needs(void)
 				die("Invalid deepen: %s", line);
 			continue;
 		}
-		if ((-prefixcmp(line, "want ")) ||
+		if (prefixcmp(line, "want ") ||
 		    get_sha1_hex(line+5, sha1_buf))
 			die("git-upload-pack: protocol error, "
 			    "expected to get sha, not '%s'", line);
-- 
1.5.0.1.571.ge5a1a

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH 4/4] prefixcmp(): fix-up leftover strncmp().
  2007-02-20  9:42   ` Andy Parkins
                       ` (3 preceding siblings ...)
  2007-02-20  9:54     ` [PATCH 3/4] prefixcmp(): fix-up mechanical conversion Junio C Hamano
@ 2007-02-20  9:55     ` Junio C Hamano
  4 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2007-02-20  9:55 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git

There were instances of strncmp() that were formatted improperly
(e.g. whitespace around parameter before closing parenthesis)
that caused the earlier mechanical conversion step to miss
them.  This step cleans them up.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

 * This concludes the series.  I'll go over these line by line
   tomorrow for the last time, and then will apply them.

 builtin-ls-tree.c     |    2 +-
 builtin-rev-parse.c   |    2 +-
 builtin-show-branch.c |    4 ++--
 diff.c                |    2 +-
 http-fetch.c          |    2 +-
 http-push.c           |    2 +-
 imap-send.c           |    8 ++++----
 revision.c            |    2 +-
 wt-status.c           |    2 +-
 9 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/builtin-ls-tree.c b/builtin-ls-tree.c
index 201defd..6472610 100644
--- a/builtin-ls-tree.c
+++ b/builtin-ls-tree.c
@@ -118,7 +118,7 @@ int cmd_ls_tree(int argc, const char **argv, const char *prefix)
 				chomp_prefix = 0;
 				break;
 			}
-			if (!strncmp(argv[1]+2, "abbrev=",7)) {
+			if (!prefixcmp(argv[1]+2, "abbrev=")) {
 				abbrev = strtoul(argv[1]+9, NULL, 10);
 				if (abbrev && abbrev < MINIMUM_ABBREV)
 					abbrev = MINIMUM_ABBREV;
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index a1c3411..37addb2 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -233,7 +233,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 			}
 			continue;
 		}
-		if (!strncmp(arg,"-n",2)) {
+		if (!prefixcmp(arg, "-n")) {
 			if ((filter & DO_FLAGS) && (filter & DO_REVS))
 				show(arg);
 			continue;
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 402a8f7..67ae6ba 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -378,7 +378,7 @@ static int append_head_ref(const char *refname, const unsigned char *sha1, int f
 {
 	unsigned char tmp[20];
 	int ofs = 11;
-	if (strncmp(refname, "refs/heads/", ofs))
+	if (prefixcmp(refname, "refs/heads/"))
 		return 0;
 	/* If both heads/foo and tags/foo exists, get_sha1 would
 	 * get confused.
@@ -392,7 +392,7 @@ static int append_remote_ref(const char *refname, const unsigned char *sha1, int
 {
 	unsigned char tmp[20];
 	int ofs = 13;
-	if (strncmp(refname, "refs/remotes/", ofs))
+	if (prefixcmp(refname, "refs/remotes/"))
 		return 0;
 	/* If both heads/foo and tags/foo exists, get_sha1 would
 	 * get confused.
diff --git a/diff.c b/diff.c
index fad13ab..c3afee2 100644
--- a/diff.c
+++ b/diff.c
@@ -77,7 +77,7 @@ int git_diff_ui_config(const char *var, const char *value)
 			diff_detect_rename_default = DIFF_DETECT_RENAME;
 		return 0;
 	}
-	if (!prefixcmp(var, "diff.color.") || !strncmp(var, "color.diff.", 11)) {
+	if (!prefixcmp(var, "diff.color.") || !prefixcmp(var, "color.diff.")) {
 		int slot = parse_diff_color_slot(var, 11);
 		color_parse(value, var, diff_colors[slot]);
 		return 0;
diff --git a/http-fetch.c b/http-fetch.c
index d9a4561..e6cd11d 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -718,7 +718,7 @@ static int fetch_indices(struct alt_base *repo)
 			i++;
 			if (i + 52 <= buffer.posn &&
 			    !prefixcmp(data + i, " pack-") &&
-			    !strncmp(data + i + 46, ".pack\n", 6)) {
+			    !prefixcmp(data + i + 46, ".pack\n")) {
 				get_sha1_hex(data + i + 6, sha1);
 				setup_index(repo, sha1);
 				i += 51;
diff --git a/http-push.c b/http-push.c
index eb77c9a..9ad6fd0 100644
--- a/http-push.c
+++ b/http-push.c
@@ -1061,7 +1061,7 @@ static int fetch_indices(void)
 			i++;
 			if (i + 52 < buffer.posn &&
 			    !prefixcmp(data + i, " pack-") &&
-			    !strncmp(data + i + 46, ".pack\n", 6)) {
+			    !prefixcmp(data + i + 46, ".pack\n")) {
 				get_sha1_hex(data + i + 6, sha1);
 				setup_index(sha1);
 				i += 51;
diff --git a/imap-send.c b/imap-send.c
index 3eaf025..84df2fa 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1192,7 +1192,7 @@ count_messages( msg_data_t *msg )
 	char *p = msg->data;
 
 	while (1) {
-		if (!strncmp( "From ", p, 5 )) {
+		if (!prefixcmp(p, "From ")) {
 			count++;
 			p += 5;
 		}
@@ -1216,7 +1216,7 @@ split_msg( msg_data_t *all_msgs, msg_data_t *msg, int *ofs )
 	data = &all_msgs->data[ *ofs ];
 	msg->len = all_msgs->len - *ofs;
 
-	if (msg->len < 5 || strncmp( data, "From ", 5 ))
+	if (msg->len < 5 || prefixcmp(data, "From "))
 		return 0;
 
 	p = strchr( data, '\n' );
@@ -1267,12 +1267,12 @@ git_imap_config(const char *key, const char *val)
 		imap_folder = xstrdup( val );
 	} else if (!strcmp( "host", key )) {
 		{
-			if (!strncmp( "imap:", val, 5 ))
+			if (!prefixcmp(val, "imap:"))
 				val += 5;
 			if (!server.port)
 				server.port = 143;
 		}
-		if (!strncmp( "//", val, 2 ))
+		if (!prefixcmp(val, "//"))
 			val += 2;
 		server.host = xstrdup( val );
 	}
diff --git a/revision.c b/revision.c
index abab3b9..622afe3 100644
--- a/revision.c
+++ b/revision.c
@@ -832,7 +832,7 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				revs->max_count = atoi(argv[++i]);
 				continue;
 			}
-			if (!strncmp(arg,"-n",2)) {
+			if (!prefixcmp(arg, "-n")) {
 				revs->max_count = atoi(arg + 2);
 				continue;
 			}
diff --git a/wt-status.c b/wt-status.c
index d17a6ba..035e546 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -344,7 +344,7 @@ int git_status_config(const char *k, const char *v)
 		wt_status_use_color = git_config_colorbool(k, v);
 		return 0;
 	}
-	if (!prefixcmp(k, "status.color.") || !strncmp(k, "color.status.", 13)) {
+	if (!prefixcmp(k, "status.color.") || !prefixcmp(k, "color.status.")) {
 		int slot = parse_status_slot(k, 13);
 		color_parse(v, k, wt_status_colors[slot]);
 	}
-- 
1.5.0.1.571.ge5a1a

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] Add prefixcmp()
  2007-02-20  9:51     ` [PATCH 1/4] Add prefixcmp() Junio C Hamano
@ 2007-02-20 10:04       ` David Kågedal
  2007-02-20 10:20         ` Junio C Hamano
  0 siblings, 1 reply; 30+ messages in thread
From: David Kågedal @ 2007-02-20 10:04 UTC (permalink / raw)
  To: git; +Cc: Junio C. Hamano

Junio C Hamano <junkio@cox.net> writes:

> We have too many strncmp(a, b, strlen(b)).
>
> Signed-off-by: Junio C Hamano <junkio@cox.net>
> ---
>  git-compat-util.h |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index 9863cf6..0a9ac56 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -279,4 +279,9 @@ static inline int sane_case(int x, int high)
>  	return x;
>  }
>  
> +static inline int prefixcmp(const char *a, const char *b)
> +{
> +	return strncmp(a, b, strlen(b));
> +}
> +
>  #endif

Is it just me, or coudln't this be a little more self-documenting.  I
find it annoying to have to read through a functions implementation to
figure out what to pass to it.

If a doc comment is too much, just naming the parameters is often
enough.

+static inline int prefixcmp(const char *s, const char *prefix)

-- 
David Kågedal

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/4] Mechanical conversion to use prefixcmp()
  2007-02-20  9:53     ` [PATCH 2/4] Mechanical conversion to use prefixcmp() Junio C Hamano
@ 2007-02-20 10:19       ` Junio C Hamano
  2007-02-20 11:53       ` Johannes Schindelin
  1 sibling, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2007-02-20 10:19 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git

Junio C Hamano <junkio@cox.net> writes:

> This mechanically converts strncmp() to use prefixcmp(),
> ...
>  * This was done by using this script in px.perl
>
>    #!/usr/bin/perl -i.bak -p
>    if (/strncmp\(([^,]+), "([^\\"]*)", (\d+)\)/ && (length($2) == $3)) {
>            s|strncmp\(([^,]+), "([^\\"]*)", (\d+)\)|prefixcmp($1, "$2")|;
>    }
>    if (/strncmp\("([^\\"]*)", ([^,]+), (\d+)\)/ && (length($1) == $3)) {
>            s|strncmp\("([^\\"]*)", ([^,]+), (\d+)\)|(-prefixcmp($2, "$1"))|;
>    }
>
>    and running:
>
>    $ git grep -l strncmp -- '*.c' | xargs perl px.perl

Two useless comments to add.

 (1) Yes, I have seen the "Oh, I lost my data doing this silly
     thing" thread that mentioned the risk of using xargs ;-).
     In general, piping output from git commands that give list
     of paths (e.g. "grep", "ls-files", "diff --name-only" and
     "ls-tree -r --name-only") to xargs should be a much safer
     practice, and people should get into the habit of doing so,
     instead of using "find | xargs".

 (2) This multi-step "mechanical conversion followed by manual
     fixup" is a trick I picked up from Linus.  The replacement
     regexp quoted above are designed to be stricter than
     necessary to catch only the safe conversion target, while
     accepting false negatives.  Doing the conversion this way,
     I do not have to worry too much about auditing 1800 lines
     of diff in [PATCH 2/4], as long as I make sure the above
     regexp is strict enough (although I did look at all 1800
     lines of diff before committing this).  Manual conversions
     in later steps do need to be looked at much more carefully
     than the result of this step, of course.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 1/4] Add prefixcmp()
  2007-02-20 10:04       ` David Kågedal
@ 2007-02-20 10:20         ` Junio C Hamano
  0 siblings, 0 replies; 30+ messages in thread
From: Junio C Hamano @ 2007-02-20 10:20 UTC (permalink / raw)
  To: David Kågedal; +Cc: git, Junio C. Hamano

David Kågedal <davidk@lysator.liu.se> writes:

> Junio C Hamano <junkio@cox.net> writes:
>
>> We have too many strncmp(a, b, strlen(b)).
>>
>> Signed-off-by: Junio C Hamano <junkio@cox.net>
>> ---
>>  git-compat-util.h |    5 +++++
>>  1 files changed, 5 insertions(+), 0 deletions(-)
>>
>> diff --git a/git-compat-util.h b/git-compat-util.h
>> index 9863cf6..0a9ac56 100644
>> --- a/git-compat-util.h
>> +++ b/git-compat-util.h
>> @@ -279,4 +279,9 @@ static inline int sane_case(int x, int high)
>>  	return x;
>>  }
>>  
>> +static inline int prefixcmp(const char *a, const char *b)
>> +{
>> +	return strncmp(a, b, strlen(b));
>> +}
>> +
>>  #endif
>
> Is it just me, or coudln't this be a little more self-documenting.  I
> find it annoying to have to read through a functions implementation to
> figure out what to pass to it.
>
> If a doc comment is too much, just naming the parameters is often
> enough.
>
> +static inline int prefixcmp(const char *s, const char *prefix)

Thanks.  That is much much better.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-20  9:50     ` Junio C Hamano
@ 2007-02-20 10:21       ` Andy Parkins
  2007-02-20 10:30         ` Junio C Hamano
  0 siblings, 1 reply; 30+ messages in thread
From: Andy Parkins @ 2007-02-20 10:21 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

On Tuesday 2007 February 20 09:50, Junio C Hamano wrote:

> I'd send the prefixcmp() patches first, as yours would touch the
> same lines.

Okay.

Your prefixcmp() point about them being used so regularly made me wonder if 
the following would improve readability:

static inline ref_is_head(const char *a)
{
    return (prefixcmp(a, PATH_REFS_HEADS) == 0);
}
static inline ref_is_tag(const char *a)
{
    return (prefixcmp(a, PATH_REFS_TAGS) == 0);
}
static inline ref_is_remote(const char *a)
{
    return (prefixcmp(a, PATH_REFS_REMOTES) == 0);
}

which would in turn convert:

   if (!strncmp(head, "refs/heads/", 11))
       head += 11;

into

   if (ref_is_head(head))
       head += STRLEN_PATH_REFS_HEADS;

which expresses the intent of the code far more clearly.



Andy

-- 
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@gmail.com

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-20 10:21       ` Andy Parkins
@ 2007-02-20 10:30         ` Junio C Hamano
  2007-02-20 10:57           ` Andy Parkins
  0 siblings, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2007-02-20 10:30 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git

Andy Parkins <andyparkins@gmail.com> writes:

> Your prefixcmp() point about them being used so regularly made me wonder if 
> the following would improve readability:
>
> static inline ref_is_head(const char *a)
> {
>     return (prefixcmp(a, PATH_REFS_HEADS) == 0);
> }
> ...
>    if (ref_is_head(head))
>        head += STRLEN_PATH_REFS_HEADS;
>
> which expresses the intent of the code far more clearly.

If we _were_ doing the inline function, I would actually prefer:

        static inline ref_is_head(const char *ref)
        {
		return !prefixcmp(ref, PATH_REFS_HEADS);
        }

But at least to me,

	if (!prefixcmp(head, PATH_REFS_HEADS))
		head += strlen(PATH_REFS_HEADS);

is easier to follow than:

        if (ref_is_head(head))
                head += STRLEN_PATH_REFS_HEADS;

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-20 10:30         ` Junio C Hamano
@ 2007-02-20 10:57           ` Andy Parkins
  2007-02-20 11:37             ` Johannes Schindelin
  2007-02-20 15:46             ` Nicolas Pitre
  0 siblings, 2 replies; 30+ messages in thread
From: Andy Parkins @ 2007-02-20 10:57 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

On Tuesday 2007 February 20 10:30, Junio C Hamano wrote:

> If we _were_ doing the inline function, I would actually prefer:
>
>         static inline ref_is_head(const char *ref)
>         {
> 		return !prefixcmp(ref, PATH_REFS_HEADS);
>         }

As you brought it up...

I've never really liked "!" on strcmp() lines (but I accept that that is the 
tradition in git) because it implies the the output of prefixcmp is boolean, 
but it's actually ternary.  strcmp() (I think), should be thought of as 
outputting

enum {
 STRING1_LESS_THAN_STRING2,
 STRINGS_EQUAL,
 STRING1_GREATER_THAN_STRING2
}

Given that, it makes me uncomfortable to use !strcmp().  Of course in the case 
of strcmp(), that form is so well known that it makes very little difference 
to the reader.

I have similar feelings about

 if( !something )

being incorrect when you meant

 if( something == NULL )

While they are identical in what they generate, they send a different message 
to someone reading the code.

Regardless, I'm not so stubborn as to refuse to go with the flow...

> But at least to me,
>
> 	if (!prefixcmp(head, PATH_REFS_HEADS))
> 		head += strlen(PATH_REFS_HEADS);
>
> is easier to follow than:
>
>         if (ref_is_head(head))
>                 head += STRLEN_PATH_REFS_HEADS;

Fine.  I don't really mind - and it's less work on my patch :-)

My argument in favour of the ref_is_head() method is that the prefixcmp() 
method requires knowledge from the caller about how you tell whether a given 
ref is a head - the second pushes that information further down the call 
tree, abstracting it out just a little more.

As I say though - it's not a problem for me.



Andy
-- 
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@gmail.com

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-20 10:57           ` Andy Parkins
@ 2007-02-20 11:37             ` Johannes Schindelin
  2007-02-20 12:24               ` Simon 'corecode' Schubert
  2007-02-20 13:26               ` Andy Parkins
  2007-02-20 15:46             ` Nicolas Pitre
  1 sibling, 2 replies; 30+ messages in thread
From: Johannes Schindelin @ 2007-02-20 11:37 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git, Junio C Hamano

Hi,

On Tue, 20 Feb 2007, Andy Parkins wrote:

> I've never really liked "!" on strcmp() lines (but I accept that that is the 
> tradition in git) because it implies the the output of prefixcmp is boolean, 
> but it's actually ternary.

Actually, it's not even ternary, but to the return value should only be 
handled in terms of >0, ==0, <0.

Ah, and if "!" implies a boolean, then why is "!!" a common construct? 
Because "!" really does not imply a boolean.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/4] Mechanical conversion to use prefixcmp()
  2007-02-20  9:53     ` [PATCH 2/4] Mechanical conversion to use prefixcmp() Junio C Hamano
  2007-02-20 10:19       ` Junio C Hamano
@ 2007-02-20 11:53       ` Johannes Schindelin
  2007-02-21  6:43         ` Junio C Hamano
  1 sibling, 1 reply; 30+ messages in thread
From: Johannes Schindelin @ 2007-02-20 11:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andy Parkins, git

Hi,

On Tue, 20 Feb 2007, Junio C Hamano wrote:

>    #!/usr/bin/perl -i.bak -p
>    if (/strncmp\(([^,]+), "([^\\"]*)", (\d+)\)/ && (length($2) == $3)) {
>            s|strncmp\(([^,]+), "([^\\"]*)", (\d+)\)|prefixcmp($1, "$2")|;
>    }
>    if (/strncmp\("([^\\"]*)", ([^,]+), (\d+)\)/ && (length($1) == $3)) {
>            s|strncmp\("([^\\"]*)", ([^,]+), (\d+)\)|(-prefixcmp($2, "$1"))|;
>    }

Ha, I did it by

$ perl -pi.bup -e \
 's/strncmp\( *("[^"]*"), *([^"]*), *[0-9]* *\)/prefixcmp\($2, $1\)/g' \
 $(git ls-files)

and

$ perl -pi.bup -e \
 's/strncmp\( *([^"]*), *("[^"]*"), *[0-9]* *\)/prefixcmp\($1, $2\)/g' \
 $(git-ls-files)

Of course, I missed the two ,ofs ones, but a git grep -n strncmp brought 
these up.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-20 11:37             ` Johannes Schindelin
@ 2007-02-20 12:24               ` Simon 'corecode' Schubert
  2007-02-20 13:26                 ` Johannes Schindelin
  2007-02-20 13:26               ` Andy Parkins
  1 sibling, 1 reply; 30+ messages in thread
From: Simon 'corecode' Schubert @ 2007-02-20 12:24 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Andy Parkins, git, Junio C Hamano

[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

Johannes Schindelin wrote:
>> I've never really liked "!" on strcmp() lines (but I accept that that is the 
>> tradition in git) because it implies the the output of prefixcmp is boolean, 
>> but it's actually ternary.
> 
> Actually, it's not even ternary, but to the return value should only be 
> handled in terms of >0, ==0, <0.
> 
> Ah, and if "!" implies a boolean, then why is "!!" a common construct? 
> Because "!" really does not imply a boolean.

Depends on how you look at it.  I code using semantics which use expressions only as boolean if they are really are.  So NULL pointers are not treated like a boolean, and neither are errno nor strcmp.  For me that's part of good, readable style, but people/groups of course are free to disagree.  Even after so many years of breathing C, I find   "if (!strcmp(foo, bar))" misleading, suggesting "not compare", which translates to "not equal".  Of course I know it, and can work with it, but in my own code I'd never write this.  I don't see any gain except some obfuscation.

cheers
  simon

-- 
Serve - BSD     +++  RENT this banner advert  +++    ASCII Ribbon   /"\
Work - Mac      +++  space for low €€€ NOW!1  +++      Campaign     \ /
Party Enjoy Relax   |   http://dragonflybsd.org      Against  HTML   \
Dude 2c 2 the max   !   http://golden-apple.biz       Mail + News   / \


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-20 12:24               ` Simon 'corecode' Schubert
@ 2007-02-20 13:26                 ` Johannes Schindelin
  0 siblings, 0 replies; 30+ messages in thread
From: Johannes Schindelin @ 2007-02-20 13:26 UTC (permalink / raw)
  To: Simon 'corecode' Schubert; +Cc: Andy Parkins, git, Junio C Hamano

Hi,

On Tue, 20 Feb 2007, Simon 'corecode' Schubert wrote:

> Johannes Schindelin wrote:
> > > I've never really liked "!" on strcmp() lines (but I accept that that is
> > > the tradition in git) because it implies the the output of prefixcmp is
> > > boolean, but it's actually ternary.
> > 
> > Actually, it's not even ternary, but to the return value should only be
> > handled in terms of >0, ==0, <0.
> > 
> > Ah, and if "!" implies a boolean, then why is "!!" a common construct?
> > Because "!" really does not imply a boolean.
> 
> Depends on how you look at it.  I code using semantics which use 
> expressions only as boolean if they are really are.

There are no booleans in C.

Also, you just state that the construct is not common for _you_. It is 
really quite common in C. Why? Because it is a short way to say _exactly_ 
what you want. Like when you say "BTW" instead of "by the way". It is not 
only quicker to type, it is also quicker to read.

> Even after so many years of breathing C, I find "if (!strcmp(foo, bar))" 
> misleading, suggesting "not compare", which translates to "not equal".

No, in plain English "!strcmp("nothing", u)" translates to "nothing 
compares to u.

In mathematics, which is the basis of computer languages, "not compare" 
means something completely different yet: "a" does not compare to "b" 
means that they cannot be compared at all, i.e. the statement "a<b" is 
neither true nor false.

So, we have -- as so often -- a case, where somebody says "it is obvious 
how this expression translates to English", but really, it is not. So, why 
not stay in the context, and interpret it like millions of programmers 
before us? Or do you want to start another Babel?

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-20 11:37             ` Johannes Schindelin
  2007-02-20 12:24               ` Simon 'corecode' Schubert
@ 2007-02-20 13:26               ` Andy Parkins
  1 sibling, 0 replies; 30+ messages in thread
From: Andy Parkins @ 2007-02-20 13:26 UTC (permalink / raw)
  To: git; +Cc: Johannes Schindelin, Junio C Hamano

On Tuesday 2007 February 20 11:37, Johannes Schindelin wrote:

> Actually, it's not even ternary, but to the return value should only be
> handled in terms of >0, ==0, <0.

Apologies, I'm not using ternary in the sense of the number 0, 1 and 2; I'm 
using it in the sense of there being three possible outcomes - which there 
are.  This is similar to how operators are categorised into unary, binary and 
ternary categories.  I use ternary to mean "having three states or elements", 
rather than "having the value 0, 1 or 2".

> Ah, and if "!" implies a boolean, then why is "!!" a common construct?
> Because "!" really does not imply a boolean.

Boolean expressions are those that have two possible states - "true" 
or "false".  There are no real booleans in C, so they are faked.  The two 
boolean states in C are represented by

 False = Equal to zero
 True = Not equal to zero

Now, the !! construction you suggest is perfectly in keeping with this 
definition.  Ironically, the only reason you need the !! construction is 
because of this integer-as-boolean trait of C.

So, to get a boolean in C you use a standard integer (say).  My contention is 
that to improve clarity you should not mix integer-as-boolean and 
integer-as-integer, even though C will accept it when you do.

 i = 10;
 while( i )
   i--;

This is bad, it abuses the fact that C will let you treat an integer as a 
boolean.  while() takes a boolean expression as it's argument, so I think 
that you should always hand it something that would be a boolean output (even 
though C doesn't care if you don't).

 i = 10;
 while( i > 0 )
  i--;

This makes it clear to the reader that i is not boolean.  Obviously this is a 
trivial example; no one would have any trouble understanding either of the 
two examples.  When things start to get bigger and more complicated though, 
more clarity is always better than less clarity.  The principle I try to 
follow is that code is write-once-read-many.  If you save yourself two 
keystrokes at the expense of the clarity you gain for the 100 times you read 
that code, then you have made a false economy.

In the end - I don't care - I was only countering Junio's "I prefer !strcmp", 
with my reasons why I don't like it.  I do not expect git to change to my 
preferred coding style, and I do try to keep to the coding style that the git 
project uses.  To my mind, inconsistency is a worse offence than anything 
else in a project, so it's always better to go with what is established.



Andy

-- 
Dr Andy Parkins, M Eng (hons), MIEE
andyparkins@gmail.com

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-02-20 10:57           ` Andy Parkins
  2007-02-20 11:37             ` Johannes Schindelin
@ 2007-02-20 15:46             ` Nicolas Pitre
  1 sibling, 0 replies; 30+ messages in thread
From: Nicolas Pitre @ 2007-02-20 15:46 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git, Junio C Hamano

On Tue, 20 Feb 2007, Andy Parkins wrote:

> On Tuesday 2007 February 20 10:30, Junio C Hamano wrote:
> 
> > But at least to me,
> >
> > 	if (!prefixcmp(head, PATH_REFS_HEADS))
> > 		head += strlen(PATH_REFS_HEADS);
> >
> > is easier to follow than:
> >
> >         if (ref_is_head(head))
> >                 head += STRLEN_PATH_REFS_HEADS;

Ditto for me.

> Fine.  I don't really mind - and it's less work on my patch :-)
> 
> My argument in favour of the ref_is_head() method is that the prefixcmp() 
> method requires knowledge from the caller about how you tell whether a given 
> ref is a head - the second pushes that information further down the call 
> tree, abstracting it out just a little more.

That's the problem though.  Too much abstraction hides away the purpose.  
With prefixcmp() it shows that the code cares about a string prefix.  
With ref_is_head() you don't know what is happening there since that 
might be many things like a pointer comparison, etc. and you have to 
look ref_is_head() implementation to be sure.


Nicolas

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/4] Mechanical conversion to use prefixcmp()
  2007-02-20 11:53       ` Johannes Schindelin
@ 2007-02-21  6:43         ` Junio C Hamano
  2007-02-21 12:41           ` Johannes Schindelin
  0 siblings, 1 reply; 30+ messages in thread
From: Junio C Hamano @ 2007-02-21  6:43 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Andy Parkins, git

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Ha, I did it by
>
> $ perl -pi.bup -e \
>  's/strncmp\( *("[^"]*"), *([^"]*), *[0-9]* *\)/prefixcmp\($2, $1\)/g' \
>  $(git ls-files)
>
> and
>
> $ perl -pi.bup -e \
>  's/strncmp\( *([^"]*), *("[^"]*"), *[0-9]* *\)/prefixcmp\($1, $2\)/g' \
>  $(git-ls-files)
>
> Of course, I missed the two ,ofs ones, but a git grep -n strncmp brought 
> these up.

I think you totally missed my point.  I wanted to make sure that
things like these do not go unnoticed:

        if (!strncmp(arg, "--foo==", 6))
	if (strncmp(line, "foo\nbar", 8))

Both are probably incorrectly written code in the original, but
probably would _happen_ to be working (for a certain definition
of "working" -- the former probably wanted to make sure the
parameter is of form "--foo=something", and the latter wanted to
check the line has the 7 bytes terminated with NUL).  But your
conversion would make them actually start behaving incorrectly.

And the worst part of this is that the change that caused to
expose these bugs would be literally _buried_ in 1800 lines of
"mechanical conversion" patch which is mind-numbing to audit.

That's why you are better off writing mechanical conversion
script in stricter than seemingly necessary to catch only the
safe conversion target, while accepting false negatives.

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH 2/4] Mechanical conversion to use prefixcmp()
  2007-02-21  6:43         ` Junio C Hamano
@ 2007-02-21 12:41           ` Johannes Schindelin
  0 siblings, 0 replies; 30+ messages in thread
From: Johannes Schindelin @ 2007-02-21 12:41 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andy Parkins, git

Hi,

On Tue, 20 Feb 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Ha, I did it by
> >
> > $ perl -pi.bup -e \
> >  's/strncmp\( *("[^"]*"), *([^"]*), *[0-9]* *\)/prefixcmp\($2, $1\)/g' \
> >  $(git ls-files)
> >
> > and
> >
> > $ perl -pi.bup -e \
> >  's/strncmp\( *([^"]*), *("[^"]*"), *[0-9]* *\)/prefixcmp\($1, $2\)/g' \
> >  $(git-ls-files)
> >
> > Of course, I missed the two ,ofs ones, but a git grep -n strncmp brought 
> > these up.
> 
> I think you totally missed my point.  I wanted to make sure that
> things like these do not go unnoticed:
> 
>         if (!strncmp(arg, "--foo==", 6))
> 	if (strncmp(line, "foo\nbar", 8))
> 
> Both are probably incorrectly written code in the original, but
> probably would _happen_ to be working (for a certain definition
> of "working" -- the former probably wanted to make sure the
> parameter is of form "--foo=something", and the latter wanted to
> check the line has the 7 bytes terminated with NUL).  But your
> conversion would make them actually start behaving incorrectly.
> 
> And the worst part of this is that the change that caused to
> expose these bugs would be literally _buried_ in 1800 lines of
> "mechanical conversion" patch which is mind-numbing to audit.
> 
> That's why you are better off writing mechanical conversion
> script in stricter than seemingly necessary to catch only the
> safe conversion target, while accepting false negatives.

All true. I thought fixing them without checking was fine, but you are 
right: better safe than sorry.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2007-02-21 12:41 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-02-19 18:39 [PATCH] Change "refs/" references to symbolic constants Andy Parkins
2007-02-19 18:55 ` Bill Lear
2007-02-19 20:01   ` [PATCH] Replace literal STRLEN_ #defines in refs.h with compiler evaluated expressions Andy Parkins
2007-02-19 20:50   ` [PATCH] Change "refs/" references to symbolic constants Krzysztof Halasa
2007-02-19 20:56     ` Shawn O. Pearce
2007-02-19 20:07 ` Junio C Hamano
2007-02-19 20:12   ` Shawn O. Pearce
2007-02-19 22:08   ` Johannes Schindelin
2007-02-20  8:41   ` Andy Parkins
2007-02-20  9:04     ` Junio C Hamano
2007-02-20  9:42   ` Andy Parkins
2007-02-20  9:50     ` Junio C Hamano
2007-02-20 10:21       ` Andy Parkins
2007-02-20 10:30         ` Junio C Hamano
2007-02-20 10:57           ` Andy Parkins
2007-02-20 11:37             ` Johannes Schindelin
2007-02-20 12:24               ` Simon 'corecode' Schubert
2007-02-20 13:26                 ` Johannes Schindelin
2007-02-20 13:26               ` Andy Parkins
2007-02-20 15:46             ` Nicolas Pitre
2007-02-20  9:51     ` [PATCH 1/4] Add prefixcmp() Junio C Hamano
2007-02-20 10:04       ` David Kågedal
2007-02-20 10:20         ` Junio C Hamano
2007-02-20  9:53     ` [PATCH 2/4] Mechanical conversion to use prefixcmp() Junio C Hamano
2007-02-20 10:19       ` Junio C Hamano
2007-02-20 11:53       ` Johannes Schindelin
2007-02-21  6:43         ` Junio C Hamano
2007-02-21 12:41           ` Johannes Schindelin
2007-02-20  9:54     ` [PATCH 3/4] prefixcmp(): fix-up mechanical conversion Junio C Hamano
2007-02-20  9:55     ` [PATCH 4/4] prefixcmp(): fix-up leftover strncmp() Junio C Hamano

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.