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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ 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; 42+ 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] 42+ messages in thread

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-10-03  4:05             ` Johannes Schindelin
  2007-10-03  4:30               ` Jeff King
@ 2007-10-03 11:30               ` Andreas Ericsson
  1 sibling, 0 replies; 42+ messages in thread
From: Andreas Ericsson @ 2007-10-03 11:30 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Jeff King, Junio C Hamano, Andy Parkins, git

Johannes Schindelin wrote:
> Hi,
> 
> On Tue, 2 Oct 2007, Jeff King wrote:
> 
>> On Tue, Oct 02, 2007 at 05:22:23PM -0700, Junio C Hamano wrote:
>>
>>>>   strbuf_init(&url);
>>>>   strbuf_addf(&url, "%s/objects/pack/pack-%s.idx", repo->base, hex);
>>> Ugh, this typically calls snprintf() twice doesn't it?
>> Yes, it probably does. However, I think it is considerably easier to
>> read and more maintainable. Are you "ugh"ing because of the performance
>> impact (which should be negligible unless this is in a tight loop) or
>> because of the portability problems associated with va_copy?
> 
> I wonder, I wonder, if
> 
> 	strbuf_addstr(&url, repo->base);
> 	strbuf_addstr(&url, "/objects/pack/pack-");
> 	strbuf_addstr(&url, hex);
> 	strbuf_addstr(&url, ".idx");
> 
> would make anybody else but me happy...

strbuf_addstr_many(&url, repo->base, "/objects/pack/pack-", hex, ".idx", NULL);

is what I'd prefer. It's not overly complicated, requires no *printf(), and doesn't
introduce any new portability issues (va_arg() is C89).

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

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

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-10-03  7:50     ` Andy Parkins
@ 2007-10-03 11:13       ` Andy Parkins
  0 siblings, 0 replies; 42+ messages in thread
From: Andy Parkins @ 2007-10-03 11:13 UTC (permalink / raw)
  To: git; +Cc: Jeff King

On Wednesday 2007 October 03, Andy Parkins wrote:

> I put a comment above the other changes like this in the same file, but
> thought I was being overly verbose putting it in every single time.  I'm
> happy to copy the comment around in the file though.

I'm a liar; I remember writing them, but they're not in the patch.  Perhaps 
they are still in my working tree at home.  Anyway; you're point was correct, 
I'll find the explanatory comments or write new ones and add them to the 
patch.



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

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

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-10-02 19:11   ` Jeff King
  2007-10-02 19:47     ` Junio C Hamano
@ 2007-10-03  7:50     ` Andy Parkins
  2007-10-03 11:13       ` Andy Parkins
  1 sibling, 1 reply; 42+ messages in thread
From: Andy Parkins @ 2007-10-03  7:50 UTC (permalink / raw)
  To: git; +Cc: Jeff King

On Tuesday 2007 October 02, Jeff King wrote:

> > -		if (prefixcmp(head, "refs/heads/"))
> > -			die("HEAD not found below refs/heads!");
> > -		head += 11;
> > +		if (prefixcmp(head, PATH_REFS_HEADS))
> > +			die("HEAD not found below " PATH_REFS_HEADS "!");
> > +		head += STRLEN_PATH_REFS_HEADS;
>
> This slightly changes the message (extra "/"), but I don't think that is
> a big deal...

Actually, I felt that was an improvement.  Personally I always like paths 
shown to a user to have the trailing slash so that they can be clear that it 
is a path not a file.

> > -	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);
>
> ...but here it's not immediately obvious if the extra trailing "/" is
> OK. Looks like the path just gets handed off to system calls trhough
> safe_create_dir, and they are happy with the trailing slash. But it is a
> behavior change.

It's been a while, but at the time I did it I think I checked 
safe_create_dir() to be sure that it was happy with trailing slashes.

> I find the 'PATH_REFS_TAGS "%s"' (with a space) you used earlier a
> little easier to read.

Okay.

> > -	if (len < 5 || memcmp(name, "refs/", 5))
> > +	if (len < STRLEN_PATH_REFS || memcmp(name, PATH_REFS,
> > STRLEN_PATH_REFS))
>
> I imagine this was one of the times you mentioned before where prefixcmp
> would be more readable. I would agree.

It is.   A patch to fix these is in my pending list.

> > -	strcpy(posn, "/objects/");
> > +	strcpy(posn, "/" PATH_OBJECTS);
> >  	posn += 9;
>
> should be posn += 1 + STRLEN_PATH_OBJECTS ?

Well spotted.  Fixed.

> > -	url = xmalloc(strlen(repo->base) + 64);
> > -	sprintf(url, "%s/objects/pack/pack-%s.idx", repo->base, hex);
> > +	url = xmalloc(strlen(repo->base) + STRLEN_PATH_OBJECTS + 56);
> > +	sprintf(url, "%s/" PATH_OBJECTS "pack/pack-%s.idx", repo->base, hex);
>
> The '56' is still quite hard to verify as correct ("/" + "pack/pack-" +
> ".idx" + "\0"). But I wonder if trying to fix that will just make it
> harder to read (perhaps a comment is in order?).

I put a comment above the other changes like this in the same file, but 
thought I was being overly verbose putting it in every single time.  I'm 
happy to copy the comment around in the file though.

> Or maybe using a strbuf here would be much more obviously correct?
>
> > -	url = xmalloc(strlen(base) + 31);
> > -	sprintf(url, "%s/objects/info/http-alternates", base);
> > +	url = xmalloc(strlen(base) + STRLEN_PATH_OBJECTS + 23);
> > +	sprintf(url, "%s/" PATH_OBJECTS "info/http-alternates", base);
>
> Also a potential strbuf. Ther are more of this same form, but I'm not
> going to bother pointing out each one.

I was trying, as far as I could, to leave the code unchanged.  If strbuf would 
be better I think I'd rather do it with another patch after this.

> Man that was tedious. But I think every other change is purely
> syntactic, so there shouldn't be any bugs.

It really was wasn't it?  :-)   While I was trying to find that bug that you 
caught yesterday I thought I was going blind.  I have this to say though: 
thank heaven for git's colourised diffs.  Those that think coloured output is 
just for prettiness sake should try that review with and without.

Thank you for expending so much effort on this, it is appreciated.



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

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

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-10-02 20:48       ` Jeff King
  2007-10-03  0:22         ` Junio C Hamano
@ 2007-10-03  7:37         ` Andy Parkins
  1 sibling, 0 replies; 42+ messages in thread
From: Andy Parkins @ 2007-10-03  7:37 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Junio C Hamano

On Tuesday 2007 October 02, Jeff King wrote:

> Perhaps a better quest would be to eliminate all of those counts
> entirely with code that is obviously correct. I think it is much more
> readable to replace:

I've got a patch replacing every appropriate memcmp() with prefixcmp(), but it 
goes on top of this one, so wanted to get this through review to save 
constantly spamming the list with the same patch slightly modified because of 
changes in a different patch.

>   url = xmalloc(strlen(repo->base) + 64);
>   sprintf(url, "%s/objects/pack/pack-%s.idx", repo->base, hex);
>
> with something like:
>
>   strbuf_init(&url);
>   strbuf_addf(&url, "%s/objects/pack/pack-%s.idx", repo->base, hex);

I've not been following the strbuf() changes, so have missed the appearance of 
these handy new functions.  They would appear to be an improvement for cases 
just like this.

> > constants in CAPITAL_LETTERS_WITH_UNDERSCORE shout too loudly to
>
> Part of the problem is also that they're long. Perhaps REFS_HEADS, while
> being less unique in the C namespace, would look better?

I completely agree with the length and loudness concerns, but my worry was 
polluting the namespace while maintaining some sort of rationality between 
PATH_REFS_HEADS and STRLEN_PATH_REFS_HEADS.  My reasoning was that

 "refs/heads" -> PATH_REFS_HEADS

is only three extra characters, and

 strlen("refs/heads/") -> STRLEN_PATH_REFS_HEADS

is only one extra character.

However I have no strong feelings about changing them.



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

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

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-10-03  4:05             ` Johannes Schindelin
@ 2007-10-03  4:30               ` Jeff King
  2007-10-03 11:30               ` Andreas Ericsson
  1 sibling, 0 replies; 42+ messages in thread
From: Jeff King @ 2007-10-03  4:30 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, Andy Parkins, git

On Wed, Oct 03, 2007 at 05:05:15AM +0100, Johannes Schindelin wrote:

> I wonder, I wonder, if
> 
> 	strbuf_addstr(&url, repo->base);
> 	strbuf_addstr(&url, "/objects/pack/pack-");
> 	strbuf_addstr(&url, hex);
> 	strbuf_addstr(&url, ".idx");
> 
> would make anybody else but me happy...

I actually wrote that originally, and then switched to the formatted
version for readability. But I would be happy with that, as well, if we
are truly concerned about the cost of 2 snprintfs.

-Peff

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

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-10-03  2:58           ` Jeff King
@ 2007-10-03  4:05             ` Johannes Schindelin
  2007-10-03  4:30               ` Jeff King
  2007-10-03 11:30               ` Andreas Ericsson
  0 siblings, 2 replies; 42+ messages in thread
From: Johannes Schindelin @ 2007-10-03  4:05 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Andy Parkins, git

Hi,

On Tue, 2 Oct 2007, Jeff King wrote:

> On Tue, Oct 02, 2007 at 05:22:23PM -0700, Junio C Hamano wrote:
> 
> > >   strbuf_init(&url);
> > >   strbuf_addf(&url, "%s/objects/pack/pack-%s.idx", repo->base, hex);
> > 
> > Ugh, this typically calls snprintf() twice doesn't it?
> 
> Yes, it probably does. However, I think it is considerably easier to
> read and more maintainable. Are you "ugh"ing because of the performance
> impact (which should be negligible unless this is in a tight loop) or
> because of the portability problems associated with va_copy?

I wonder, I wonder, if

	strbuf_addstr(&url, repo->base);
	strbuf_addstr(&url, "/objects/pack/pack-");
	strbuf_addstr(&url, hex);
	strbuf_addstr(&url, ".idx");

would make anybody else but me happy...

Ciao,
Dscho

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

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-10-03  0:22         ` Junio C Hamano
@ 2007-10-03  2:58           ` Jeff King
  2007-10-03  4:05             ` Johannes Schindelin
  0 siblings, 1 reply; 42+ messages in thread
From: Jeff King @ 2007-10-03  2:58 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andy Parkins, git

On Tue, Oct 02, 2007 at 05:22:23PM -0700, Junio C Hamano wrote:

> >   strbuf_init(&url);
> >   strbuf_addf(&url, "%s/objects/pack/pack-%s.idx", repo->base, hex);
> 
> Ugh, this typically calls snprintf() twice doesn't it?

Yes, it probably does. However, I think it is considerably easier to
read and more maintainable. Are you "ugh"ing because of the performance
impact (which should be negligible unless this is in a tight loop) or
because of the portability problems associated with va_copy?

-Peff

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

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-10-02 20:48       ` Jeff King
@ 2007-10-03  0:22         ` Junio C Hamano
  2007-10-03  2:58           ` Jeff King
  2007-10-03  7:37         ` Andy Parkins
  1 sibling, 1 reply; 42+ messages in thread
From: Junio C Hamano @ 2007-10-03  0:22 UTC (permalink / raw)
  To: Jeff King; +Cc: Andy Parkins, git

Jeff King <peff@peff.net> writes:

> Perhaps a better quest would be to eliminate all of those counts
> entirely with code that is obviously correct. I think it is much more
> readable to replace:
>
>   url = xmalloc(strlen(repo->base) + 64);
>   sprintf(url, "%s/objects/pack/pack-%s.idx", repo->base, hex);
>
> with something like:
>
>   strbuf_init(&url);
>   strbuf_addf(&url, "%s/objects/pack/pack-%s.idx", repo->base, hex);

Ugh, this typically calls snprintf() twice doesn't it?

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

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-10-02 19:47     ` Junio C Hamano
@ 2007-10-02 20:48       ` Jeff King
  2007-10-03  0:22         ` Junio C Hamano
  2007-10-03  7:37         ` Andy Parkins
  0 siblings, 2 replies; 42+ messages in thread
From: Jeff King @ 2007-10-02 20:48 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Andy Parkins, git

On Tue, Oct 02, 2007 at 12:47:59PM -0700, Junio C Hamano wrote:

>  - it makes typo harder to make and easier to spot
>    (e.g. "refs/head/");
> 
>  - it makes miscount harder to make and easier to spot (e.g.
>    what is this magic constant 11? Is it strlen("refs/heads/")?);
> 
>  - it makes reviewing the resulting code, and more importantly,
>    future patches on the resulting code, easier.
> [...]
> It however actively hurts on the third count.  These long

Yes, I find some of the substitutions more readable, but some are a bit
less readable. The parts of the patch I found the _most_ improved are
the ones that get rid of a memcmp in favor of a prefixcmp (i.e.,
removing the count entirely).

Perhaps a better quest would be to eliminate all of those counts
entirely with code that is obviously correct. I think it is much more
readable to replace:

  url = xmalloc(strlen(repo->base) + 64);
  sprintf(url, "%s/objects/pack/pack-%s.idx", repo->base, hex);

with something like:

  strbuf_init(&url);
  strbuf_addf(&url, "%s/objects/pack/pack-%s.idx", repo->base, hex);

which has the same number of lines, but no magic numbers at all. Or
since most of the uses of things like PATH_OBJECTS are more or less the
same, maybe something like:

  mkpath_object(&url, "pack/pack-%s.idx", hex);

i.e., rather than fiddling with string constants, wrap them
functionally.

> constants in CAPITAL_LETTERS_WITH_UNDERSCORE shout too loudly to

Part of the problem is also that they're long. Perhaps REFS_HEADS, while
being less unique in the C namespace, would look better?

-Peff

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

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-10-02 19:11   ` Jeff King
@ 2007-10-02 19:47     ` Junio C Hamano
  2007-10-02 20:48       ` Jeff King
  2007-10-03  7:50     ` Andy Parkins
  1 sibling, 1 reply; 42+ messages in thread
From: Junio C Hamano @ 2007-10-02 19:47 UTC (permalink / raw)
  To: Jeff King; +Cc: Andy Parkins, git

Jeff King <peff@peff.net> writes:

> On Tue, Oct 02, 2007 at 07:16:43PM +0100, Andy Parkins wrote:
>
>> Changed repeated use of the same constants for the ref paths to be
>> symbolic constants.  I've defined them in refs.h
>
> I've manually inspected the patch. Comments are below.
>
>> -		if (prefixcmp(head, "refs/heads/"))
>> -			die("HEAD not found below refs/heads!");
>> -		head += 11;
>> +		if (prefixcmp(head, PATH_REFS_HEADS))
>> +			die("HEAD not found below " PATH_REFS_HEADS "!");
>> +		head += STRLEN_PATH_REFS_HEADS;
>
> This slightly changes the message (extra "/"), but I don't think that is
> a big deal...

        die("HEAD not found below %.*%s!",
             PATH_REFS_HEADS, STRLEN_PATH_REFS_HEADS-1)

>> -		if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p)
>> +		if (snprintf(ref, sizeof(ref), PATH_REFS_TAGS"%s", *p)
>
> I find the 'PATH_REFS_TAGS "%s"' (with a space) you used earlier a
> little easier to read.

Even though we all know that PATH_REFS_* do not have any '%' in
them, it is somewhat unnerving to see such an opaque string in
the format specifier part of _any_printf() function.  It just
makes you think twice, disrupting the flow of thoughts.

This applies to die() and friends as well; see my above rewrite.

To me, the valid reasons for this kind of rewrite are if:

 - it makes typo harder to make and easier to spot
   (e.g. "refs/head/");

 - it makes miscount harder to make and easier to spot (e.g.
   what is this magic constant 11? Is it strlen("refs/heads/")?);

 - it makes reviewing the resulting code, and more importantly,
   future patches on the resulting code, easier.

 - it makes it easier for us to later revamp the strings
   wholesale (e.g. "refs/heads/" => "refs/branches/").

 - it saves us repeated instances of the same string constant;
   using C literal string as values for PATH_REFS_HEADS would
   not help and you would need (const char []) strings instead,
   but the compiler may be clever enough to do so.

Unquestionably, this series helps on the first two counts.

It however actively hurts on the third count.  These long
constants in CAPITAL_LETTERS_WITH_UNDERSCORE shout too loudly to
the eye, overwhelming the surrounding code.  I wonder if we can
do anything about this point to resurrect the first two
benefits, which I like very much.

The forth is a myth we shouldn't care about.  If we later would
want to change refs/heads to refs/branches, we would want to
rename PATH_REFS_HEADS to PATH_REFS_BRANCHES at the same time as
well, so the kind of rewrite this patch does does not buy us
anything there.  More importantly, such a change would need to
be made in a backward compatible way (e.g. "if we have heads
then keep using them but in new repositories we favor
branches"), so it won't be straight token replacement anyway.

And the fifth do not apply to us.  This matters only if we were
an embedded application on memory starved machine and string
constants are far smaller matter compared to the amount of other
data we use in-core.

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

* Re: [PATCH] Change "refs/" references to symbolic constants
  2007-10-02 18:16 ` [PATCH] " Andy Parkins
@ 2007-10-02 19:11   ` Jeff King
  2007-10-02 19:47     ` Junio C Hamano
  2007-10-03  7:50     ` Andy Parkins
  0 siblings, 2 replies; 42+ messages in thread
From: Jeff King @ 2007-10-02 19:11 UTC (permalink / raw)
  To: Andy Parkins; +Cc: git

On Tue, Oct 02, 2007 at 07:16:43PM +0100, Andy Parkins wrote:

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

I've manually inspected the patch. Comments are below.

> -		if (prefixcmp(head, "refs/heads/"))
> -			die("HEAD not found below refs/heads!");
> -		head += 11;
> +		if (prefixcmp(head, PATH_REFS_HEADS))
> +			die("HEAD not found below " PATH_REFS_HEADS "!");
> +		head += STRLEN_PATH_REFS_HEADS;

This slightly changes the message (extra "/"), but I don't think that is
a big deal...

> -	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);

...but here it's not immediately obvious if the extra trailing "/" is
OK. Looks like the path just gets handed off to system calls trhough
safe_create_dir, and they are happy with the trailing slash. But it is a
behavior change.

> -		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);

And of course ditto here.

> -		if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p)
> +		if (snprintf(ref, sizeof(ref), PATH_REFS_TAGS"%s", *p)

I find the 'PATH_REFS_TAGS "%s"' (with a space) you used earlier a
little easier to read.

> -	if (len < 5 || memcmp(name, "refs/", 5))
> +	if (len < STRLEN_PATH_REFS || memcmp(name, PATH_REFS, STRLEN_PATH_REFS))

I imagine this was one of the times you mentioned before where prefixcmp
would be more readable. I would agree.

> -	strcpy(posn, "/objects/");
> +	strcpy(posn, "/" PATH_OBJECTS);
>  	posn += 9;

should be posn += 1 + STRLEN_PATH_OBJECTS ?

> -	url = xmalloc(strlen(repo->base) + 64);
> -	sprintf(url, "%s/objects/pack/pack-%s.idx", repo->base, hex);
> +	url = xmalloc(strlen(repo->base) + STRLEN_PATH_OBJECTS + 56);
> +	sprintf(url, "%s/" PATH_OBJECTS "pack/pack-%s.idx", repo->base, hex);

The '56' is still quite hard to verify as correct ("/" + "pack/pack-" +
".idx" + "\0"). But I wonder if trying to fix that will just make it
harder to read (perhaps a comment is in order?).

Or maybe using a strbuf here would be much more obviously correct?

> -	url = xmalloc(strlen(base) + 31);
> -	sprintf(url, "%s/objects/info/http-alternates", base);
> +	url = xmalloc(strlen(base) + STRLEN_PATH_OBJECTS + 23);
> +	sprintf(url, "%s/" PATH_OBJECTS "info/http-alternates", base);

Also a potential strbuf. Ther are more of this same form, but I'm not
going to bother pointing out each one.

> -- 
> 1.5.3.rc5.11.g312e

Man that was tedious. But I think every other change is purely
syntactic, so there shouldn't be any bugs.

-Peff

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

* [PATCH] Change "refs/" references to symbolic constants
  2007-10-02 15:58 [PATCH 1/2] Change "refs/" references to symbolic constants Jeff King
@ 2007-10-02 18:16 ` Andy Parkins
  2007-10-02 19:11   ` Jeff King
  0 siblings, 1 reply; 42+ messages in thread
From: Andy Parkins @ 2007-10-02 18:16 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-fetch--tool.c   |   10 +++++-----
 builtin-fmt-merge-msg.c |    5 +++--
 builtin-fsck.c          |    4 ++--
 builtin-init-db.c       |   15 ++++++++-------
 builtin-name-rev.c      |   14 +++++++-------
 builtin-pack-refs.c     |    2 +-
 builtin-push.c          |    6 +++---
 builtin-show-branch.c   |   34 +++++++++++++++++-----------------
 builtin-show-ref.c      |    6 +++---
 builtin-tag.c           |    4 ++--
 connect.c               |   10 +++++-----
 fetch-pack.c            |    6 +++---
 http-fetch.c            |   31 ++++++++++++++++---------------
 http-push.c             |   42 +++++++++++++++++++++---------------------
 local-fetch.c           |   13 +++++++------
 path.c                  |    5 +++--
 receive-pack.c          |    4 ++--
 reflog-walk.c           |    6 +++---
 refs.c                  |   18 +++++++++---------
 refs.h                  |   17 +++++++++++++++++
 remote.c                |   14 +++++++-------
 setup.c                 |    5 +++--
 sha1_name.c             |   10 +++++-----
 wt-status.c             |    5 +++--
 26 files changed, 170 insertions(+), 146 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index 5f5c182..b203b2a 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -92,12 +92,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:
@@ -189,15 +189,15 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,
 	int len;
 
 	/* Detect kind */
-	if (!prefixcmp(refname, "refs/heads/")) {
+	if (!prefixcmp(refname, PATH_REFS_HEADS)) {
 		kind = REF_LOCAL_BRANCH;
-		refname += 11;
-	} else if (!prefixcmp(refname, "refs/remotes/")) {
+		refname += STRLEN_PATH_REFS_HEADS;
+	} else if (!prefixcmp(refname, PATH_REFS_REMOTES)) {
 		kind = REF_REMOTE_BRANCH;
-		refname += 13;
-	} else if (!prefixcmp(refname, "refs/tags/")) {
+		refname += STRLEN_PATH_REFS_REMOTES;
+	} else if (!prefixcmp(refname, PATH_REFS_TAGS)) {
 		kind = REF_TAG;
-		refname += 10;
+		refname += STRLEN_PATH_REFS_TAGS;
 	}
 
 	/* Don't add types the caller doesn't want */
@@ -400,7 +400,7 @@ static void create_branch(const char *name, const char *start_name,
 	char *real_ref, 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);
 
@@ -469,13 +469,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))
@@ -602,9 +602,9 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 		detached = 1;
 	}
 	else {
-		if (prefixcmp(head, "refs/heads/"))
-			die("HEAD not found below refs/heads!");
-		head += 11;
+		if (prefixcmp(head, 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 669110c..b6f5b45 100644
--- a/builtin-describe.c
+++ b/builtin-describe.c
@@ -53,7 +53,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 (!prefixcmp(path, "refs/tags/")) {
+	if (!prefixcmp(path, PATH_REFS_TAGS)) {
 		if (object->type == OBJ_TAG)
 			prio = 2;
 		else
diff --git a/builtin-fetch--tool.c b/builtin-fetch--tool.c
index 24c7e6f..521b5fc 100644
--- a/builtin-fetch--tool.c
+++ b/builtin-fetch--tool.c
@@ -76,7 +76,7 @@ static int update_local_ref(const char *name,
 		char *msg;
 	just_store:
 		/* new ref */
-		if (!strncmp(name, "refs/tags/", 10))
+		if (!prefixcmp(name, PATH_REFS_TAGS))
 			msg = "storing tag";
 		else
 			msg = "storing head";
@@ -94,7 +94,7 @@ static int update_local_ref(const char *name,
 		return 0;
 	}
 
-	if (!strncmp(name, "refs/tags/", 10)) {
+	if (!prefixcmp(name, PATH_REFS_TAGS)) {
 		fprintf(stderr, "* %s: updating with %s\n", name, note);
 		show_new(type, sha1_new);
 		return update_ref_env("updating tag", name, sha1_new, NULL);
@@ -151,15 +151,15 @@ static int append_fetch_head(FILE *fp,
 		kind = "";
 		what = "";
 	}
-	else if (!strncmp(remote_name, "refs/heads/", 11)) {
+	else if (!prefixcmp(remote_name, PATH_REFS_HEADS)) {
 		kind = "branch";
 		what = remote_name + 11;
 	}
-	else if (!strncmp(remote_name, "refs/tags/", 10)) {
+	else if (!prefixcmp(remote_name, PATH_REFS_TAGS)) {
 		kind = "tag";
 		what = remote_name + 10;
 	}
-	else if (!strncmp(remote_name, "refs/remotes/", 13)) {
+	else if (!prefixcmp(remote_name, PATH_REFS_REMOTES)) {
 		kind = "remote branch";
 		what = remote_name + 13;
 	}
diff --git a/builtin-fmt-merge-msg.c b/builtin-fmt-merge-msg.c
index ae60fcc..8700343 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>]";
@@ -282,8 +283,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 (!prefixcmp(current_branch, "refs/heads/"))
-		current_branch += 11;
+	if (!prefixcmp(current_branch, 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 8d12287..83a2d0c 100644
--- a/builtin-fsck.c
+++ b/builtin-fsck.c
@@ -629,14 +629,14 @@ static int fsck_head_link(void)
 	if (!strcmp(head_points_at, "HEAD"))
 		/* detached HEAD */
 		null_is_error = 1;
-	else if (prefixcmp(head_points_at, "refs/heads/"))
+	else if (prefixcmp(head_points_at, PATH_REFS_HEADS))
 		return error("HEAD points to something strange (%s)",
 			     head_points_at);
 	if (is_null_sha1(sha1)) {
 		if (null_is_error)
 			return error("HEAD: detached HEAD points at nothing");
 		fprintf(stderr, "notice: HEAD points to an unborn branch (%s)\n",
-			head_points_at + 11);
+			head_points_at + STRLEN_PATH_REFS_HEADS);
 	}
 	return 0;
 }
diff --git a/builtin-init-db.c b/builtin-init-db.c
index 763fa55..1f3c0dd 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"
@@ -194,11 +195,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
@@ -217,11 +218,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);
 	}
 
@@ -232,7 +233,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 03083e9..56c13d0 100644
--- a/builtin-name-rev.c
+++ b/builtin-name-rev.c
@@ -96,7 +96,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 && prefixcmp(path, "refs/tags/"))
+	if (data->tags_only && prefixcmp(path, PATH_REFS_TAGS))
 		return 0;
 
 	if (data->ref_filter && fnmatch(data->ref_filter, path, 0))
@@ -112,14 +112,14 @@ 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 (!prefixcmp(path, "refs/heads/"))
-			path = path + 11;
+		if (!prefixcmp(path, PATH_REFS_HEADS))
+			path = path + STRLEN_PATH_REFS_HEADS;
 		else if (data->tags_only
 		    && data->name_only
-		    && !prefixcmp(path, "refs/tags/"))
-			path = path + 10;
-		else if (!prefixcmp(path, "refs/"))
-			path = path + 5;
+		    && !prefixcmp(path, PATH_REFS_TAGS))
+			path = path + STRLEN_PATH_REFS_TAGS;
+		else if (!prefixcmp(path, 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 09df4e1..23b4c4e 100644
--- a/builtin-pack-refs.c
+++ b/builtin-pack-refs.c
@@ -39,7 +39,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 = !prefixcmp(path, "refs/tags/");
+	is_tag_ref = !prefixcmp(path, PATH_REFS_TAGS);
 
 	/* ALWAYS pack refs that were already packed or are tags */
 	if (!(cb->flags & PACK_REFS_ALL) && !is_tag_ref && !(flags & REF_ISPACKED))
diff --git a/builtin-push.c b/builtin-push.c
index 88c5024..2fdae7a 100644
--- a/builtin-push.c
+++ b/builtin-push.c
@@ -33,9 +33,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;
 		}
@@ -148,7 +148,7 @@ int cmd_push(int argc, const char **argv, const char *prefix)
 			continue;
 		}
 		if (!strcmp(arg, "--tags")) {
-			add_refspec("refs/tags/*");
+			add_refspec(PATH_REFS_TAGS "*");
 			continue;
 		}
 		if (!strcmp(arg, "--force") || !strcmp(arg, "-f")) {
diff --git a/builtin-show-branch.c b/builtin-show-branch.c
index 4fa87f6..7e39d60 100644
--- a/builtin-show-branch.c
+++ b/builtin-show-branch.c
@@ -380,36 +380,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 (prefixcmp(refname, "refs/heads/"))
+	int ofs = STRLEN_PATH_REFS_HEADS;
+	if (prefixcmp(refname, PATH_REFS_HEADS))
 		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 (prefixcmp(refname, "refs/remotes/"))
+	int ofs = STRLEN_PATH_REFS_REMOTES;
+	if (prefixcmp(refname, PATH_REFS_REMOTES))
 		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 (prefixcmp(refname, "refs/tags/"))
+	if (prefixcmp(refname, 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;
@@ -438,9 +438,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, PATH_REFS_HEADS))
 		return append_head_ref(refname, sha1, flag, cb_data);
-	if (!prefixcmp(refname, "refs/tags/"))
+	if (!prefixcmp(refname, PATH_REFS_TAGS))
 		return append_tag_ref(refname, sha1, flag, cb_data);
 	return append_ref(refname, sha1, 0);
 }
@@ -465,12 +465,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 (!prefixcmp(head, "refs/heads/"))
-		head += 11;
-	if (!prefixcmp(name, "refs/heads/"))
-		name += 11;
-	else if (!prefixcmp(name, "heads/"))
-		name += 6;
+	if (!prefixcmp(head, PATH_REFS_HEADS))
+		head += STRLEN_PATH_REFS_HEADS;
+	if (!prefixcmp(name, PATH_REFS_HEADS))
+		name += STRLEN_PATH_REFS_HEADS;
+	else if (!prefixcmp(name, PATH_HEADS))
+		name += STRLEN_PATH_HEADS;
 	return !strcmp(head, name);
 }
 
@@ -781,7 +781,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 65051d1..d463d80 100644
--- a/builtin-show-ref.c
+++ b/builtin-show-ref.c
@@ -29,8 +29,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 && !prefixcmp(refname, "refs/heads/");
-		match |= tags_only && !prefixcmp(refname, "refs/tags/");
+		match = heads_only && !prefixcmp(refname, PATH_REFS_HEADS);
+		match |= tags_only && !prefixcmp(refname, PATH_REFS_TAGS);
 		if (!match)
 			return 0;
 	}
@@ -227,7 +227,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
 		while (*pattern) {
 			unsigned char sha1[20];
 
-			if (!prefixcmp(*pattern, "refs/") &&
+			if (!prefixcmp(*pattern, PATH_REFS) &&
 			    resolve_ref(*pattern, sha1, 1, NULL)) {
 				if (!quiet)
 					show_one(*pattern, sha1);
diff --git a/builtin-tag.c b/builtin-tag.c
index 3a9d2ee..800e823 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -146,7 +146,7 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn)
 	unsigned char sha1[20];
 
 	for (p = argv; *p; p++) {
-		if (snprintf(ref, sizeof(ref), "refs/tags/%s", *p)
+		if (snprintf(ref, sizeof(ref), PATH_REFS_TAGS"%s", *p)
 					>= sizeof(ref)) {
 			error("tag name too long: %.*s...", 50, *p);
 			had_error = 1;
@@ -440,7 +440,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (get_sha1(object_ref, object))
 		die("Failed to resolve '%s' as a valid ref.", object_ref);
 
-	if (snprintf(ref, sizeof(ref), "refs/tags/%s", tag) > sizeof(ref) - 1)
+	if (snprintf(ref, sizeof(ref), PATH_REFS_TAGS "%s", tag) > sizeof(ref) - 1)
 		die("tag name too long: %.*s...", 50, tag);
 	if (check_ref_format(ref))
 		die("'%s' is not a valid tag name.", tag);
diff --git a/connect.c b/connect.c
index 8b1e993..9d576bc 100644
--- a/connect.c
+++ b/connect.c
@@ -13,23 +13,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 */
diff --git a/fetch-pack.c b/fetch-pack.c
index 9c81305..c3b7ef6 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -344,11 +344,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 || prefixcmp(ref->name, "refs/tags/") )) {
+			 (!depth || prefixcmp(ref->name, PATH_REFS_TAGS) )) {
 			*newtail = ref;
 			ref->next = NULL;
 			newtail = &ref->next;
diff --git a/http-fetch.c b/http-fetch.c
index 202fae0..85c46a8 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
@@ -157,11 +158,11 @@ static void start_object_request(struct object_request *obj_req)
 
 	SHA1_Init(&obj_req->c);
 
-	url = xmalloc(strlen(obj_req->repo->base) + 51);
-	obj_req->url = xmalloc(strlen(obj_req->repo->base) + 51);
+	url = xmalloc(strlen(obj_req->repo->base) + STRLEN_PATH_OBJECTS + 43);
+	obj_req->url = xmalloc(strlen(obj_req->repo->base) + STRLEN_PATH_OBJECTS + 43);
 	strcpy(url, obj_req->repo->base);
 	posn = url + strlen(obj_req->repo->base);
-	strcpy(posn, "/objects/");
+	strcpy(posn, "/" PATH_OBJECTS);
 	posn += 9;
 	memcpy(posn, hex, 2);
 	posn += 2;
@@ -398,8 +399,8 @@ static int fetch_index(struct alt_base *repo, unsigned char *sha1)
 	if (get_verbosely)
 		fprintf(stderr, "Getting index for pack %s\n", hex);
 
-	url = xmalloc(strlen(repo->base) + 64);
-	sprintf(url, "%s/objects/pack/pack-%s.idx", repo->base, hex);
+	url = xmalloc(strlen(repo->base) + STRLEN_PATH_OBJECTS + 56);
+	sprintf(url, "%s/" PATH_OBJECTS "pack/pack-%s.idx", repo->base, hex);
 
 	filename = sha1_pack_index_name(sha1);
 	snprintf(tmpfile, sizeof(tmpfile), "%s.temp", filename);
@@ -478,7 +479,7 @@ static void process_alternates_response(void *callback_data)
 
 			/* Try reusing the slot to get non-http alternates */
 			alt_req->http_specific = 0;
-			sprintf(alt_req->url, "%s/objects/info/alternates",
+			sprintf(alt_req->url, "%s/" PATH_OBJECTS "info/alternates",
 				base);
 			curl_easy_setopt(slot->curl, CURLOPT_URL,
 					 alt_req->url);
@@ -625,8 +626,8 @@ static void fetch_alternates(const char *base)
 	if (get_verbosely)
 		fprintf(stderr, "Getting alternates list for %s\n", base);
 
-	url = xmalloc(strlen(base) + 31);
-	sprintf(url, "%s/objects/info/http-alternates", base);
+	url = xmalloc(strlen(base) + STRLEN_PATH_OBJECTS + 23);
+	sprintf(url, "%s/" PATH_OBJECTS "info/http-alternates", base);
 
 	/* Use a callback to process the result, since another request
 	   may fail and need to have alternates loaded before continuing */
@@ -675,8 +676,8 @@ static int fetch_indices(struct alt_base *repo)
 	if (get_verbosely)
 		fprintf(stderr, "Getting pack list for %s\n", repo->base);
 
-	url = xmalloc(strlen(repo->base) + 21);
-	sprintf(url, "%s/objects/info/packs", repo->base);
+	url = xmalloc(strlen(repo->base) + STRLEN_PATH_OBJECTS + 13);
+	sprintf(url, "%s/" PATH_OBJECTS "info/packs", repo->base);
 
 	slot = get_active_slot();
 	slot->results = &results;
@@ -757,8 +758,8 @@ static int fetch_pack(struct alt_base *repo, unsigned char *sha1)
 			sha1_to_hex(sha1));
 	}
 
-	url = xmalloc(strlen(repo->base) + 65);
-	sprintf(url, "%s/objects/pack/pack-%s.pack",
+	url = xmalloc(strlen(repo->base) + STRLEN_PATH_OBJECTS + 57);
+	sprintf(url, "%s/" PATH_OBJECTS "pack/pack-%s.pack",
 		repo->base, sha1_to_hex(target->sha1));
 
 	filename = sha1_pack_name(target->sha1);
@@ -930,14 +931,14 @@ static char *quote_ref_url(const char *base, const char *ref)
 	int len, baselen, ch;
 
 	baselen = strlen(base);
-	len = baselen + 7; /* "/refs/" + NUL */
+	len = baselen + STRLEN_PATH_REFS + 2; /* "/" + "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/", 6);
-	for (cp = ref, dp = qref + baselen + 6; (ch = *cp) != 0; cp++) {
+	memcpy(qref + baselen, "/" PATH_REFS, STRLEN_PATH_REFS + 1);
+	for (cp = ref, dp = qref + baselen + STRLEN_PATH_REFS + 1; (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 7c3720f..1d26b9b 100644
--- a/http-push.c
+++ b/http-push.c
@@ -272,12 +272,12 @@ static void start_fetch_loose(struct transfer_request *request)
 
 	SHA1_Init(&request->c);
 
-	url = xmalloc(strlen(remote->url) + 50);
-	request->url = xmalloc(strlen(remote->url) + 50);
+	url = xmalloc(strlen(remote->url) + 42 + STRLEN_PATH_OBJECTS);
+	request->url = xmalloc(strlen(remote->url) + 42 + STRLEN_PATH_OBJECTS);
 	strcpy(url, remote->url);
 	posn = url + strlen(remote->url);
-	strcpy(posn, "objects/");
-	posn += 8;
+	strcpy(posn, PATH_OBJECTS);
+	posn += STRLEN_PATH_OBJECTS;
 	memcpy(posn, hex, 2);
 	posn += 2;
 	*(posn++) = '/';
@@ -357,11 +357,11 @@ static void start_mkcol(struct transfer_request *request)
 	struct active_request_slot *slot;
 	char *posn;
 
-	request->url = xmalloc(strlen(remote->url) + 13);
+	request->url = xmalloc(strlen(remote->url) + STRLEN_PATH_OBJECTS + 5);
 	strcpy(request->url, remote->url);
 	posn = request->url + strlen(remote->url);
-	strcpy(posn, "objects/");
-	posn += 8;
+	strcpy(posn, PATH_OBJECTS);
+	posn += STRLEN_PATH_OBJECTS;
 	memcpy(posn, hex, 2);
 	posn += 2;
 	strcpy(posn, "/");
@@ -415,8 +415,8 @@ static void start_fetch_packed(struct transfer_request *request)
 	snprintf(request->tmpfile, sizeof(request->tmpfile),
 		 "%s.temp", filename);
 
-	url = xmalloc(strlen(remote->url) + 64);
-	sprintf(url, "%sobjects/pack/pack-%s.pack",
+	url = xmalloc(strlen(remote->url) + STRLEN_PATH_OBJECTS + 56);
+	sprintf(url, "%s" PATH_OBJECTS "pack/pack-%s.pack",
 		remote->url, sha1_to_hex(target->sha1));
 
 	/* Make sure there isn't another open request for this pack */
@@ -519,11 +519,11 @@ static void start_put(struct transfer_request *request)
 	request->buffer.posn = 0;
 
 	request->url = xmalloc(strlen(remote->url) +
-			       strlen(request->lock->token) + 51);
+			       strlen(request->lock->token) + STRLEN_PATH_OBJECTS + 43);
 	strcpy(request->url, remote->url);
 	posn = request->url + strlen(remote->url);
-	strcpy(posn, "objects/");
-	posn += 8;
+	strcpy(posn, PATH_OBJECTS);
+	posn += STRLEN_PATH_OBJECTS;
 	memcpy(posn, hex, 2);
 	posn += 2;
 	*(posn++) = '/';
@@ -922,8 +922,8 @@ static int fetch_index(unsigned char *sha1)
 	struct slot_results results;
 
 	/* Don't use the index if the pack isn't there */
-	url = xmalloc(strlen(remote->url) + 64);
-	sprintf(url, "%sobjects/pack/pack-%s.pack", remote->url, hex);
+	url = xmalloc(strlen(remote->url) + STRLEN_PATH_OBJECTS + 56);
+	sprintf(url, "%s" PATH_OBJECTS "pack/pack-%s.pack", remote->url, hex);
 	slot = get_active_slot();
 	slot->results = &results;
 	curl_easy_setopt(slot->curl, CURLOPT_URL, url);
@@ -945,7 +945,7 @@ static int fetch_index(unsigned char *sha1)
 	if (push_verbosely)
 		fprintf(stderr, "Getting index for pack %s\n", hex);
 
-	sprintf(url, "%sobjects/pack/pack-%s.idx", remote->url, hex);
+	sprintf(url, "%s" PATH_OBJECTS "pack/pack-%s.idx", remote->url, hex);
 
 	filename = sha1_pack_index_name(sha1);
 	snprintf(tmpfile, sizeof(tmpfile), "%s.temp", filename);
@@ -1029,8 +1029,8 @@ static int fetch_indices(void)
 	if (push_verbosely)
 		fprintf(stderr, "Getting pack list\n");
 
-	url = xmalloc(strlen(remote->url) + 20);
-	sprintf(url, "%sobjects/info/packs", remote->url);
+	url = xmalloc(strlen(remote->url) + STRLEN_PATH_OBJECTS + 12);
+	sprintf(url, "%s" PATH_OBJECTS "info/packs", remote->url);
 
 	slot = get_active_slot();
 	slot->results = &results;
@@ -1615,7 +1615,7 @@ static void remote_ls(const char *path, int flags,
 
 static void get_remote_object_list(unsigned char parent)
 {
-	char path[] = "objects/XX/";
+	char path[] = PATH_OBJECTS "XX/";
 	static const char hex[] = "0123456789abcdef";
 	unsigned int val = parent;
 
@@ -1925,7 +1925,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)
@@ -2069,7 +2069,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);
@@ -2375,7 +2375,7 @@ int main(int argc, char **argv)
 	/* Check whether the remote has server info files */
 	remote->can_update_info_refs = 0;
 	remote->has_info_refs = remote_exists("info/refs");
-	remote->has_info_packs = remote_exists("objects/info/packs");
+	remote->has_info_packs = remote_exists(PATH_OBJECTS "info/packs");
 	if (remote->has_info_refs) {
 		info_ref_lock = lock_remote("info/refs", LOCK_TIME);
 		if (info_ref_lock)
diff --git a/local-fetch.c b/local-fetch.c
index bf7ec6c..67dc065 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;
@@ -23,7 +24,7 @@ static void setup_index(unsigned char *sha1)
 	struct packed_git *new_pack;
 	char filename[PATH_MAX];
 	strcpy(filename, path);
-	strcat(filename, "/objects/pack/pack-");
+	strcat(filename, "/" PATH_OBJECTS "pack/pack-");
 	strcat(filename, sha1_to_hex(sha1));
 	strcat(filename, ".idx");
 	new_pack = parse_pack_index_file(sha1, filename);
@@ -37,7 +38,7 @@ static int setup_indices(void)
 	struct dirent *de;
 	char filename[PATH_MAX];
 	unsigned char sha1[20];
-	sprintf(filename, "%s/objects/pack/", path);
+	sprintf(filename, "%s/" PATH_OBJECTS "pack/", path);
 	dir = opendir(filename);
 	if (!dir)
 		return -1;
@@ -122,11 +123,11 @@ static int fetch_pack(const unsigned char *sha1)
 		fprintf(stderr, " which contains %s\n",
 			sha1_to_hex(sha1));
 	}
-	sprintf(filename, "%s/objects/pack/pack-%s.pack",
+	sprintf(filename, "%s/" PATH_OBJECTS "pack/pack-%s.pack",
 		path, sha1_to_hex(target->sha1));
 	copy_file(filename, sha1_pack_name(target->sha1),
 		  sha1_to_hex(target->sha1), 1);
-	sprintf(filename, "%s/objects/pack/pack-%s.idx",
+	sprintf(filename, "%s/" PATH_OBJECTS "pack/pack-%s.idx",
 		path, sha1_to_hex(target->sha1));
 	copy_file(filename, sha1_pack_index_name(target->sha1),
 		  sha1_to_hex(target->sha1), 1);
@@ -143,7 +144,7 @@ static int fetch_file(const unsigned char *sha1)
 
 	if (object_name_start < 0) {
 		strcpy(filename, path); /* e.g. git.git */
-		strcat(filename, "/objects/");
+		strcat(filename, "/" PATH_OBJECTS);
 		object_name_start = strlen(filename);
 	}
 	filename[object_name_start+0] = hex[0];
@@ -169,7 +170,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 4260952..d330bbc 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/";
 
@@ -99,7 +100,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 >= STRLEN_PATH_REFS && !memcmp(PATH_REFS, buffer, STRLEN_PATH_REFS))
 			return 0;
 		return -1;
 	}
@@ -123,7 +124,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 d3c422b..114ea38 100644
--- a/receive-pack.c
+++ b/receive-pack.c
@@ -165,7 +165,7 @@ static const char *update(struct command *cmd)
 	unsigned char *new_sha1 = cmd->new_sha1;
 	struct ref_lock *lock;
 
-	if (!prefixcmp(name, "refs/") && check_ref_format(name + 5)) {
+	if (!prefixcmp(name, PATH_REFS) && check_ref_format(name + STRLEN_PATH_REFS)) {
 		error("refusing to create funny ref '%s' locally", name);
 		return "funny refname";
 	}
@@ -177,7 +177,7 @@ static const char *update(struct command *cmd)
 	}
 	if (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&
 	    !is_null_sha1(old_sha1) &&
-	    !prefixcmp(name, "refs/heads/")) {
+	    !prefixcmp(name, 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 ee1456b..98cf8ef 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 7fb3350..840a433 100644
--- a/refs.c
+++ b/refs.c
@@ -409,7 +409,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;
@@ -561,22 +561,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
@@ -588,7 +588,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);
 }
 
 /*
@@ -824,7 +824,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, 0, NULL);
 }
 
@@ -1078,8 +1078,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 &&
-	    (!prefixcmp(ref_name, "refs/heads/") ||
-	     !prefixcmp(ref_name, "refs/remotes/") ||
+	    (!prefixcmp(ref_name,  PATH_REFS_HEADS) ||
+	     !prefixcmp(ref_name, PATH_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/refs.h b/refs.h
index 6eb98a4..1025d04 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/remote.c b/remote.c
index bb774d0..b8922c7 100644
--- a/remote.c
+++ b/remote.c
@@ -211,8 +211,8 @@ static void read_config(void)
 	current_branch = NULL;
 	head_ref = resolve_ref("HEAD", sha1, 0, &flag);
 	if (head_ref && (flag & REF_ISSYMREF) &&
-	    !prefixcmp(head_ref, "refs/heads/")) {
-		current_branch = head_ref + strlen("refs/heads/");
+	    !prefixcmp(head_ref, PATH_REFS_HEADS)) {
+		current_branch = head_ref + STRLEN_PATH_REFS_HEADS;
 		current_branch_len = strlen(current_branch);
 	}
 	git_config(handle_config);
@@ -398,9 +398,9 @@ static int count_refspec_match(const char *pattern,
 		 * at the remote site.
 		 */
 		if (namelen != patlen &&
-		    patlen != namelen - 5 &&
-		    prefixcmp(name, "refs/heads/") &&
-		    prefixcmp(name, "refs/tags/")) {
+		    patlen != namelen - STRLEN_PATH_REFS &&
+		    prefixcmp(name, PATH_REFS_HEADS) &&
+		    prefixcmp(name, 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
@@ -511,7 +511,7 @@ static int match_explicit(struct ref *src, struct ref *dst,
 	case 1:
 		break;
 	case 0:
-		if (!memcmp(dst_value, "refs/", 5))
+		if (!prefixcmp(dst_value, PATH_REFS))
 			matched_dst = make_linked_ref(dst_value, dst_tail);
 		else
 			error("dst refspec %s does not match any "
@@ -594,7 +594,7 @@ int match_refs(struct ref *src, struct ref *dst, struct ref ***dst_tail,
 			if (!pat)
 				continue;
 		}
-		else if (prefixcmp(src->name, "refs/heads/"))
+		else if (prefixcmp(src->name, PATH_REFS_HEADS))
 			/*
 			 * "matching refs"; traditionally we pushed everything
 			 * including refs outside refs/heads/ hierarchy, but
diff --git a/setup.c b/setup.c
index 06004f1..c8912d2 100644
--- a/setup.c
+++ b/setup.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "dir.h"
+#include "refs.h"
 
 static int inside_git_dir = -1;
 static int inside_work_tree = -1;
@@ -158,12 +159,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 2d727d5..649e438 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -241,11 +241,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 10ce6ee..93dee72 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] = {
@@ -311,8 +312,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 (!prefixcmp(branch_name, "refs/heads/"))
-			branch_name += 11;
+		if (!prefixcmp(branch_name, 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.3.rc5.11.g312e

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

end of thread, other threads:[~2007-10-03 11:40 UTC | newest]

Thread overview: 42+ 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
2007-10-02 15:58 [PATCH 1/2] Change "refs/" references to symbolic constants Jeff King
2007-10-02 18:16 ` [PATCH] " Andy Parkins
2007-10-02 19:11   ` Jeff King
2007-10-02 19:47     ` Junio C Hamano
2007-10-02 20:48       ` Jeff King
2007-10-03  0:22         ` Junio C Hamano
2007-10-03  2:58           ` Jeff King
2007-10-03  4:05             ` Johannes Schindelin
2007-10-03  4:30               ` Jeff King
2007-10-03 11:30               ` Andreas Ericsson
2007-10-03  7:37         ` Andy Parkins
2007-10-03  7:50     ` Andy Parkins
2007-10-03 11:13       ` Andy Parkins

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.