[PATCH v2 00/16] Convert parts of refs.c to struct object_id

[PATCH v2 00/16] Convert parts of refs.c to struct object_id

[brian m. carlson]

This is a conversion of parts of refs.c to use struct object_id.

refs.c, and the for_each_ref series of functions explicitly, is the
source for many instances of object IDs in the codebase.  Therefore, it
makes sense to convert this series of functions to provide a basis for
further conversions.

This series is essentially just for_each_ref and friends, the callbacks,
and callers.  Other parts of refs.c will be converted in a later series,
so as to keep the number of patches to a reasonable size.

There should be no functional change from this patch series.

Changes from v1:
* Rebase onto next to pick up the first set of object_id patches.
* Fix a series of nasty conflicts that occurred due to other topics in
  flight to promote easier testing and integration.

Comments and reviews on this series would be greatly appreciated.

brian m. carlson (16):
  refs: convert struct ref_entry to use struct object_id
  refs: convert for_each_tag_ref to struct object_id
  refs: convert remaining users of for_each_ref_in to object_id
  refs: convert for_each_ref_in_submodule to object_id
  refs: convert head_ref to struct object_id
  refs: convert for_each_ref_submodule to struct object_id
  revision: remove unused _oid helper.
  refs: convert for_each_ref to struct object_id
  refs: convert for_each_replace_ref to struct object_id
  refs: convert namespaced ref iteration functions to object_id
  refs: convert for_each_rawref to struct object_id.
  refs: rename do_for_each_ref_oid to do_for_each_ref
  refs: convert for_each_reflog to struct object_id
  refs: rename each_ref_fn_oid to each_ref_fn
  Remove unneeded *_oid functions.
  refs: convert struct ref_lock to struct object_id

 Documentation/technical/api-ref-iteration.txt |   2 +-
 bisect.c                                      |   8 +-
 builtin/branch.c                              |   4 +-
 builtin/checkout.c                            |   4 +-
 builtin/describe.c                            |  12 +--
 builtin/fetch.c                               |   6 +-
 builtin/for-each-ref.c                        |   4 +-
 builtin/fsck.c                                |  18 ++---
 builtin/name-rev.c                            |   6 +-
 builtin/pack-objects.c                        |  14 ++--
 builtin/receive-pack.c                        |   4 +-
 builtin/reflog.c                              |  10 +--
 builtin/remote.c                              |  14 ++--
 builtin/replace.c                             |  14 ++--
 builtin/rev-parse.c                           |   8 +-
 builtin/show-branch.c                         |  24 +++---
 builtin/show-ref.c                            |  16 ++--
 builtin/tag.c                                 |   8 +-
 fetch-pack.c                                  |  18 ++++-
 help.c                                        |   2 +-
 http-backend.c                                |  14 ++--
 log-tree.c                                    |  10 +--
 notes.c                                       |   2 +-
 reachable.c                                   |   4 +-
 refs.c                                        | 104 +++++++++++++-------------
 refs.h                                        |   4 +-
 remote.c                                      |   8 +-
 replace_object.c                              |   4 +-
 revision.c                                    |  18 +++--
 server-info.c                                 |   6 +-
 sha1_name.c                                   |   4 +-
 shallow.c                                     |   8 +-
 submodule.c                                   |   6 +-
 transport.c                                   |  10 +--
 upload-pack.c                                 |  30 ++++----
 walker.c                                      |   4 +-
 36 files changed, 225 insertions(+), 207 deletions(-)

-- 
2.3.5

[PATCH v2 01/16] refs: convert struct ref_entry to use struct object_id

[brian m. carlson]

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 refs.c | 44 ++++++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/refs.c b/refs.c
index 81a455b..522d15d 100644
--- a/refs.c
+++ b/refs.c
@@ -156,7 +156,7 @@ struct ref_value {
 	 * null.  If REF_ISSYMREF, then this is the name of the object
 	 * referred to by the last reference in the symlink chain.
 	 */
-	unsigned char sha1[20];
+	struct object_id oid;
 
 	/*
 	 * If REF_KNOWS_PEELED, then this field holds the peeled value
@@ -164,7 +164,7 @@ struct ref_value {
 	 * be peelable.  See the documentation for peel_ref() for an
 	 * exact definition of "peelable".
 	 */
-	unsigned char peeled[20];
+	struct object_id peeled;
 };
 
 struct ref_cache;
@@ -346,8 +346,8 @@ static struct ref_entry *create_ref_entry(const char *refname,
 		die("Reference has invalid format: '%s'", refname);
 	len = strlen(refname) + 1;
 	ref = xmalloc(sizeof(struct ref_entry) + len);
-	hashcpy(ref->u.value.sha1, sha1);
-	hashclr(ref->u.value.peeled);
+	hashcpy(ref->u.value.oid.hash, sha1);
+	oidclr(&ref->u.value.peeled);
 	memcpy(ref->name, refname, len);
 	ref->flag = flag;
 	return ref;
@@ -621,7 +621,7 @@ static int is_dup_ref(const struct ref_entry *ref1, const struct ref_entry *ref2
 		/* This is impossible by construction */
 		die("Reference directory conflict: %s", ref1->name);
 
-	if (hashcmp(ref1->u.value.sha1, ref2->u.value.sha1))
+	if (oidcmp(&ref1->u.value.oid, &ref2->u.value.oid))
 		die("Duplicated ref, and SHA1s don't match: %s", ref1->name);
 
 	warning("Duplicated ref: %s", ref1->name);
@@ -669,7 +669,7 @@ static int ref_resolves_to_object(struct ref_entry *entry)
 {
 	if (entry->flag & REF_ISBROKEN)
 		return 0;
-	if (!has_sha1_file(entry->u.value.sha1)) {
+	if (!has_sha1_file(entry->u.value.oid.hash)) {
 		error("%s does not point to a valid object!", entry->name);
 		return 0;
 	}
@@ -717,7 +717,7 @@ static int do_one_ref(struct ref_entry *entry, void *cb_data)
 	/* Store the old value, in case this is a recursive call: */
 	old_current_ref = current_ref;
 	current_ref = entry;
-	retval = data->fn(entry->name + data->trim, entry->u.value.sha1,
+	retval = data->fn(entry->name + data->trim, entry->u.value.oid.hash,
 			  entry->flag, data->cb_data);
 	current_ref = old_current_ref;
 	return retval;
@@ -1193,7 +1193,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
 		    line.len == PEELED_LINE_LENGTH &&
 		    line.buf[PEELED_LINE_LENGTH - 1] == '\n' &&
 		    !get_sha1_hex(line.buf + 1, sha1)) {
-			hashcpy(last->u.value.peeled, sha1);
+			hashcpy(last->u.value.peeled.hash, sha1);
 			/*
 			 * Regardless of what the file header said,
 			 * we definitely know the value of *this*
@@ -1374,7 +1374,7 @@ static int resolve_gitlink_packed_ref(struct ref_cache *refs,
 	if (ref == NULL)
 		return -1;
 
-	hashcpy(sha1, ref->u.value.sha1);
+	hashcpy(sha1, ref->u.value.oid.hash);
 	return 0;
 }
 
@@ -1461,7 +1461,7 @@ static int resolve_missing_loose_ref(const char *refname,
 	 */
 	entry = get_packed_ref(refname);
 	if (entry) {
-		hashcpy(sha1, entry->u.value.sha1);
+		hashcpy(sha1, entry->u.value.oid.hash);
 		if (flags)
 			*flags |= REF_ISPACKED;
 		return 0;
@@ -1771,9 +1771,9 @@ static enum peel_status peel_entry(struct ref_entry *entry, int repeel)
 	if (entry->flag & REF_KNOWS_PEELED) {
 		if (repeel) {
 			entry->flag &= ~REF_KNOWS_PEELED;
-			hashclr(entry->u.value.peeled);
+			oidclr(&entry->u.value.peeled);
 		} else {
-			return is_null_sha1(entry->u.value.peeled) ?
+			return is_null_oid(&entry->u.value.peeled) ?
 				PEEL_NON_TAG : PEEL_PEELED;
 		}
 	}
@@ -1782,7 +1782,7 @@ static enum peel_status peel_entry(struct ref_entry *entry, int repeel)
 	if (entry->flag & REF_ISSYMREF)
 		return PEEL_IS_SYMREF;
 
-	status = peel_object(entry->u.value.sha1, entry->u.value.peeled);
+	status = peel_object(entry->u.value.oid.hash, entry->u.value.peeled.hash);
 	if (status == PEEL_PEELED || status == PEEL_NON_TAG)
 		entry->flag |= REF_KNOWS_PEELED;
 	return status;
@@ -1797,7 +1797,7 @@ int peel_ref(const char *refname, unsigned char *sha1)
 			    || !strcmp(current_ref->name, refname))) {
 		if (peel_entry(current_ref, 0))
 			return -1;
-		hashcpy(sha1, current_ref->u.value.peeled);
+		hashcpy(sha1, current_ref->u.value.peeled.hash);
 		return 0;
 	}
 
@@ -1817,7 +1817,7 @@ int peel_ref(const char *refname, unsigned char *sha1)
 		if (r) {
 			if (peel_entry(r, 0))
 				return -1;
-			hashcpy(sha1, r->u.value.peeled);
+			hashcpy(sha1, r->u.value.peeled.hash);
 			return 0;
 		}
 	}
@@ -2422,9 +2422,9 @@ static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)
 	if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)
 		error("internal error: %s is not a valid packed reference!",
 		      entry->name);
-	write_packed_entry(cb_data, entry->name, entry->u.value.sha1,
+	write_packed_entry(cb_data, entry->name, entry->u.value.oid.hash,
 			   peel_status == PEEL_PEELED ?
-			   entry->u.value.peeled : NULL);
+			   entry->u.value.peeled.hash : NULL);
 	return 0;
 }
 
@@ -2531,24 +2531,24 @@ static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data)
 	peel_status = peel_entry(entry, 1);
 	if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)
 		die("internal error peeling reference %s (%s)",
-		    entry->name, sha1_to_hex(entry->u.value.sha1));
+		    entry->name, oid_to_hex(&entry->u.value.oid));
 	packed_entry = find_ref(cb->packed_refs, entry->name);
 	if (packed_entry) {
 		/* Overwrite existing packed entry with info from loose entry */
 		packed_entry->flag = REF_ISPACKED | REF_KNOWS_PEELED;
-		hashcpy(packed_entry->u.value.sha1, entry->u.value.sha1);
+		oidcpy(&packed_entry->u.value.oid, &entry->u.value.oid);
 	} else {
-		packed_entry = create_ref_entry(entry->name, entry->u.value.sha1,
+		packed_entry = create_ref_entry(entry->name, entry->u.value.oid.hash,
 						REF_ISPACKED | REF_KNOWS_PEELED, 0);
 		add_ref(cb->packed_refs, packed_entry);
 	}
-	hashcpy(packed_entry->u.value.peeled, entry->u.value.peeled);
+	oidcpy(&packed_entry->u.value.peeled, &entry->u.value.peeled);
 
 	/* Schedule the loose reference for pruning if requested. */
 	if ((cb->flags & PACK_REFS_PRUNE)) {
 		int namelen = strlen(entry->name) + 1;
 		struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);
-		hashcpy(n->sha1, entry->u.value.sha1);
+		hashcpy(n->sha1, entry->u.value.oid.hash);
 		strcpy(n->name, entry->name);
 		n->next = cb->ref_to_prune;
 		cb->ref_to_prune = n;
-- 
2.3.5

[PATCH v2 02/16] refs: convert for_each_tag_ref to struct object_id

[brian m. carlson]

To allow piecemeal conversion of the for_each_*_ref functions, introduce
an additional typedef for a callback function that takes struct
object_id * instead of unsigned char *.  Provide an extra field in
struct ref_entry_cb for this callback and ensure at most one is set at a
time.  Temporarily suffix these new entries with _oid to distinguish
them.  Convert for_each_tag_ref and its callers to use the new _oid
functions, introducing temporary wrapper functions to avoid type
mismatches.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/pack-objects.c |  4 ++--
 builtin/rev-parse.c    |  7 ++++++-
 builtin/tag.c          |  8 ++++----
 refs.c                 | 34 ++++++++++++++++++++++++++++++----
 refs.h                 | 10 +++++++++-
 5 files changed, 51 insertions(+), 12 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c067107..0c69b0c 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -540,11 +540,11 @@ static enum write_one_status write_one(struct sha1file *f,
 	return WRITE_ONE_WRITTEN;
 }
 
-static int mark_tagged(const char *path, const unsigned char *sha1, int flag,
+static int mark_tagged(const char *path, const struct object_id *oid, int flag,
 		       void *cb_data)
 {
 	unsigned char peeled[20];
-	struct object_entry *entry = packlist_find(&to_pack, sha1, NULL);
+	struct object_entry *entry = packlist_find(&to_pack, oid->hash, NULL);
 
 	if (entry)
 		entry->tagged = 1;
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 4d10dd9..7b70650 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -198,6 +198,11 @@ static int show_reference(const char *refname, const unsigned char *sha1, int fl
 	return 0;
 }
 
+static int show_reference_oid(const char *refname, const struct object_id *oid, int flag, void *cb_data)
+{
+	return show_reference(refname, oid->hash, flag, cb_data);
+}
+
 static int anti_reference(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
 	show_rev(REVERSED, sha1, refname);
@@ -682,7 +687,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!strcmp(arg, "--tags")) {
-				for_each_tag_ref(show_reference, NULL);
+				for_each_tag_ref(show_reference_oid, NULL);
 				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
diff --git a/builtin/tag.c b/builtin/tag.c
index 6f07ac6..61399b7 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -215,7 +215,7 @@ free_return:
 	free(buf);
 }
 
-static int show_reference(const char *refname, const unsigned char *sha1,
+static int show_reference(const char *refname, const struct object_id *oid,
 			  int flag, void *cb_data)
 {
 	struct tag_filter *filter = cb_data;
@@ -224,14 +224,14 @@ static int show_reference(const char *refname, const unsigned char *sha1,
 		if (filter->with_commit) {
 			struct commit *commit;
 
-			commit = lookup_commit_reference_gently(sha1, 1);
+			commit = lookup_commit_reference_gently(oid->hash, 1);
 			if (!commit)
 				return 0;
 			if (!contains(commit, filter->with_commit))
 				return 0;
 		}
 
-		if (points_at.nr && !match_points_at(refname, sha1))
+		if (points_at.nr && !match_points_at(refname, oid->hash))
 			return 0;
 
 		if (!filter->lines) {
@@ -242,7 +242,7 @@ static int show_reference(const char *refname, const unsigned char *sha1,
 			return 0;
 		}
 		printf("%-15s ", refname);
-		show_tag_lines(sha1, filter->lines);
+		show_tag_lines(oid->hash, filter->lines);
 		putchar('\n');
 	}
 
diff --git a/refs.c b/refs.c
index 522d15d..95863f2 100644
--- a/refs.c
+++ b/refs.c
@@ -694,6 +694,7 @@ struct ref_entry_cb {
 	int trim;
 	int flags;
 	each_ref_fn *fn;
+	each_ref_fn_oid *fn_oid;
 	void *cb_data;
 };
 
@@ -717,8 +718,13 @@ static int do_one_ref(struct ref_entry *entry, void *cb_data)
 	/* Store the old value, in case this is a recursive call: */
 	old_current_ref = current_ref;
 	current_ref = entry;
-	retval = data->fn(entry->name + data->trim, entry->u.value.oid.hash,
-			  entry->flag, data->cb_data);
+	if (data->fn_oid) {
+		retval = data->fn_oid(entry->name + data->trim, &entry->u.value.oid,
+				 entry->flag, data->cb_data);
+	} else {
+		retval = data->fn(entry->name + data->trim, entry->u.value.oid.hash,
+				 entry->flag, data->cb_data);
+	}
 	current_ref = old_current_ref;
 	return retval;
 }
@@ -1950,6 +1956,21 @@ static int do_for_each_ref(struct ref_cache *refs, const char *base,
 	data.trim = trim;
 	data.flags = flags;
 	data.fn = fn;
+	data.fn_oid = NULL;
+	data.cb_data = cb_data;
+
+	return do_for_each_entry(refs, base, do_one_ref, &data);
+}
+
+static int do_for_each_ref_oid(struct ref_cache *refs, const char *base,
+			   each_ref_fn_oid fn, int trim, int flags, void *cb_data)
+{
+	struct ref_entry_cb data;
+	data.base = base;
+	data.trim = trim;
+	data.flags = flags;
+	data.fn = NULL;
+	data.fn_oid = fn;
 	data.cb_data = cb_data;
 
 	if (ref_paranoia < 0)
@@ -1998,6 +2019,11 @@ int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 	return do_for_each_ref(get_ref_cache(submodule), "", fn, 0, 0, cb_data);
 }
 
+static int for_each_ref_in_oid(const char *prefix, each_ref_fn_oid fn, void *cb_data)
+{
+	return do_for_each_ref_oid(&ref_cache, prefix, fn, strlen(prefix), 0, cb_data);
+}
+
 int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
 {
 	return do_for_each_ref(&ref_cache, prefix, fn, strlen(prefix), 0, cb_data);
@@ -2009,9 +2035,9 @@ int for_each_ref_in_submodule(const char *submodule, const char *prefix,
 	return do_for_each_ref(get_ref_cache(submodule), prefix, fn, strlen(prefix), 0, cb_data);
 }
 
-int for_each_tag_ref(each_ref_fn fn, void *cb_data)
+int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
 {
-	return for_each_ref_in("refs/tags/", fn, cb_data);
+	return for_each_ref_in_oid("refs/tags/", fn, cb_data);
 }
 
 int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
diff --git a/refs.h b/refs.h
index 6d7d9b4..b83529b 100644
--- a/refs.h
+++ b/refs.h
@@ -1,6 +1,8 @@
 #ifndef REFS_H
 #define REFS_H
 
+#include "cache.h"
+
 /*
  * A ref_transaction represents a collection of ref updates
  * that should succeed or fail together.
@@ -70,6 +72,12 @@ typedef int each_ref_fn(const char *refname,
 			const unsigned char *sha1, int flags, void *cb_data);
 
 /*
+ * Like each_ref_fn, but passes the object ID using a struct.
+ */
+typedef int each_ref_fn_oid(const char *refname,
+			const struct object_id *oid, int flags, void *cb_data);
+
+/*
  * The following functions invoke the specified callback function for
  * each reference indicated.  If the function ever returns a nonzero
  * value, stop the iteration and return that value.  Please note that
@@ -81,7 +89,7 @@ typedef int each_ref_fn(const char *refname,
 extern int head_ref(each_ref_fn, void *);
 extern int for_each_ref(each_ref_fn, void *);
 extern int for_each_ref_in(const char *, each_ref_fn, void *);
-extern int for_each_tag_ref(each_ref_fn, void *);
+extern int for_each_tag_ref(each_ref_fn_oid, void *);
 extern int for_each_branch_ref(each_ref_fn, void *);
 extern int for_each_remote_ref(each_ref_fn, void *);
 extern int for_each_replace_ref(each_ref_fn, void *);
-- 
2.3.5

[PATCH v2 03/16] refs: convert remaining users of for_each_ref_in to object_id

[brian m. carlson]

Remove the temporary for_each_ref_in_oid function and update the users
of it.  Convert the users of for_each_branch_ref and
for_each_remote_ref (which use for_each_ref_in under the hood) as well.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 bisect.c            |  8 ++++----
 builtin/rev-parse.c | 10 +++++-----
 refs.c              | 13 ++++---------
 refs.h              |  6 +++---
 4 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/bisect.c b/bisect.c
index 10f5e57..03d5cd9 100644
--- a/bisect.c
+++ b/bisect.c
@@ -400,16 +400,16 @@ struct commit_list *find_bisection(struct commit_list *list,
 	return best;
 }
 
-static int register_ref(const char *refname, const unsigned char *sha1,
+static int register_ref(const char *refname, const struct object_id *oid,
 			int flags, void *cb_data)
 {
 	if (!strcmp(refname, "bad")) {
 		current_bad_oid = xmalloc(sizeof(*current_bad_oid));
-		hashcpy(current_bad_oid->hash, sha1);
+		oidcpy(current_bad_oid, oid);
 	} else if (starts_with(refname, "good-")) {
-		sha1_array_append(&good_revs, sha1);
+		sha1_array_append(&good_revs, oid->hash);
 	} else if (starts_with(refname, "skip-")) {
-		sha1_array_append(&skipped_revs, sha1);
+		sha1_array_append(&skipped_revs, oid->hash);
 	}
 
 	return 0;
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 7b70650..c9f2c93 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -203,9 +203,9 @@ static int show_reference_oid(const char *refname, const struct object_id *oid,
 	return show_reference(refname, oid->hash, flag, cb_data);
 }
 
-static int anti_reference(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+static int anti_reference(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
-	show_rev(REVERSED, sha1, refname);
+	show_rev(REVERSED, oid->hash, refname);
 	return 0;
 }
 
@@ -665,7 +665,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!strcmp(arg, "--bisect")) {
-				for_each_ref_in("refs/bisect/bad", show_reference, NULL);
+				for_each_ref_in("refs/bisect/bad", show_reference_oid, NULL);
 				for_each_ref_in("refs/bisect/good", anti_reference, NULL);
 				continue;
 			}
@@ -676,7 +676,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!strcmp(arg, "--branches")) {
-				for_each_branch_ref(show_reference, NULL);
+				for_each_branch_ref(show_reference_oid, NULL);
 				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
@@ -703,7 +703,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!strcmp(arg, "--remotes")) {
-				for_each_remote_ref(show_reference, NULL);
+				for_each_remote_ref(show_reference_oid, NULL);
 				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
diff --git a/refs.c b/refs.c
index 95863f2..a81c301 100644
--- a/refs.c
+++ b/refs.c
@@ -2019,16 +2019,11 @@ int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 	return do_for_each_ref(get_ref_cache(submodule), "", fn, 0, 0, cb_data);
 }
 
-static int for_each_ref_in_oid(const char *prefix, each_ref_fn_oid fn, void *cb_data)
+int for_each_ref_in(const char *prefix, each_ref_fn_oid fn, void *cb_data)
 {
 	return do_for_each_ref_oid(&ref_cache, prefix, fn, strlen(prefix), 0, cb_data);
 }
 
-int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
-{
-	return do_for_each_ref(&ref_cache, prefix, fn, strlen(prefix), 0, cb_data);
-}
-
 int for_each_ref_in_submodule(const char *submodule, const char *prefix,
 		each_ref_fn fn, void *cb_data)
 {
@@ -2037,7 +2032,7 @@ int for_each_ref_in_submodule(const char *submodule, const char *prefix,
 
 int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
 {
-	return for_each_ref_in_oid("refs/tags/", fn, cb_data);
+	return for_each_ref_in("refs/tags/", fn, cb_data);
 }
 
 int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
@@ -2045,7 +2040,7 @@ int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_d
 	return for_each_ref_in_submodule(submodule, "refs/tags/", fn, cb_data);
 }
 
-int for_each_branch_ref(each_ref_fn fn, void *cb_data)
+int for_each_branch_ref(each_ref_fn_oid fn, void *cb_data)
 {
 	return for_each_ref_in("refs/heads/", fn, cb_data);
 }
@@ -2055,7 +2050,7 @@ int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void *c
 	return for_each_ref_in_submodule(submodule, "refs/heads/", fn, cb_data);
 }
 
-int for_each_remote_ref(each_ref_fn fn, void *cb_data)
+int for_each_remote_ref(each_ref_fn_oid fn, void *cb_data)
 {
 	return for_each_ref_in("refs/remotes/", fn, cb_data);
 }
diff --git a/refs.h b/refs.h
index b83529b..d6ef917 100644
--- a/refs.h
+++ b/refs.h
@@ -88,10 +88,10 @@ typedef int each_ref_fn_oid(const char *refname,
  */
 extern int head_ref(each_ref_fn, void *);
 extern int for_each_ref(each_ref_fn, void *);
-extern int for_each_ref_in(const char *, each_ref_fn, void *);
+extern int for_each_ref_in(const char *, each_ref_fn_oid, void *);
 extern int for_each_tag_ref(each_ref_fn_oid, void *);
-extern int for_each_branch_ref(each_ref_fn, void *);
-extern int for_each_remote_ref(each_ref_fn, void *);
+extern int for_each_branch_ref(each_ref_fn_oid, void *);
+extern int for_each_remote_ref(each_ref_fn_oid, void *);
 extern int for_each_replace_ref(each_ref_fn, void *);
 extern int for_each_glob_ref(each_ref_fn, const char *pattern, void *);
 extern int for_each_glob_ref_in(each_ref_fn, const char *pattern, const char* prefix, void *);
-- 
2.3.5

[PATCH v2 04/16] refs: convert for_each_ref_in_submodule to object_id

[brian m. carlson]

Convert for_each_ref_in_submodule and all of its caller.  Introduce two
temporary wrappers in revision.c to handle the incompatibilities between
each_ref_fn and each_ref_fn_oid.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 refs.c      | 10 +++++-----
 refs.h      |  8 ++++----
 revision.c  | 28 +++++++++++++++++++++-------
 submodule.c |  2 +-
 4 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/refs.c b/refs.c
index a81c301..aa13cc2 100644
--- a/refs.c
+++ b/refs.c
@@ -2025,9 +2025,9 @@ int for_each_ref_in(const char *prefix, each_ref_fn_oid fn, void *cb_data)
 }
 
 int for_each_ref_in_submodule(const char *submodule, const char *prefix,
-		each_ref_fn fn, void *cb_data)
+		each_ref_fn_oid fn, void *cb_data)
 {
-	return do_for_each_ref(get_ref_cache(submodule), prefix, fn, strlen(prefix), 0, cb_data);
+	return do_for_each_ref_oid(get_ref_cache(submodule), prefix, fn, strlen(prefix), 0, cb_data);
 }
 
 int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
@@ -2035,7 +2035,7 @@ int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
 	return for_each_ref_in("refs/tags/", fn, cb_data);
 }
 
-int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+int for_each_tag_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data)
 {
 	return for_each_ref_in_submodule(submodule, "refs/tags/", fn, cb_data);
 }
@@ -2045,7 +2045,7 @@ int for_each_branch_ref(each_ref_fn_oid fn, void *cb_data)
 	return for_each_ref_in("refs/heads/", fn, cb_data);
 }
 
-int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+int for_each_branch_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data)
 {
 	return for_each_ref_in_submodule(submodule, "refs/heads/", fn, cb_data);
 }
@@ -2055,7 +2055,7 @@ int for_each_remote_ref(each_ref_fn_oid fn, void *cb_data)
 	return for_each_ref_in("refs/remotes/", fn, cb_data);
 }
 
-int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+int for_each_remote_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data)
 {
 	return for_each_ref_in_submodule(submodule, "refs/remotes/", fn, cb_data);
 }
diff --git a/refs.h b/refs.h
index d6ef917..62eb553 100644
--- a/refs.h
+++ b/refs.h
@@ -99,10 +99,10 @@ extern int for_each_glob_ref_in(each_ref_fn, const char *pattern, const char* pr
 extern int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
 extern int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
 extern int for_each_ref_in_submodule(const char *submodule, const char *prefix,
-		each_ref_fn fn, void *cb_data);
-extern int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
-extern int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
-extern int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
+		each_ref_fn_oid fn, void *cb_data);
+extern int for_each_tag_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
+extern int for_each_branch_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
+extern int for_each_remote_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
 
 extern int head_ref_namespaced(each_ref_fn fn, void *cb_data);
 extern int for_each_namespaced_ref(each_ref_fn fn, void *cb_data);
diff --git a/revision.c b/revision.c
index 7ddbaa0..93d9311 100644
--- a/revision.c
+++ b/revision.c
@@ -1232,6 +1232,12 @@ static int handle_one_ref(const char *path, const unsigned char *sha1, int flag,
 	return 0;
 }
 
+static int handle_one_ref_oid(const char *path, const struct object_id *oid,
+	int flag, void *cb_data)
+{
+	return handle_one_ref(path, oid->hash, flag, cb_data);
+}
+
 static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs,
 	unsigned flags)
 {
@@ -1265,6 +1271,14 @@ static void handle_refs(const char *submodule, struct rev_info *revs, unsigned f
 	for_each(submodule, handle_one_ref, &cb);
 }
 
+static void handle_refs_oid(const char *submodule, struct rev_info *revs, unsigned flags,
+		int (*for_each)(const char *, each_ref_fn_oid, void *))
+{
+	struct all_refs_cb cb;
+	init_all_refs_cb(&cb, revs, flags);
+	for_each(submodule, handle_one_ref_oid, &cb);
+}
+
 static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
 {
 	struct all_refs_cb *cb = cb_data;
@@ -2073,12 +2087,12 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
 	ctx->argc -= n;
 }
 
-static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)
+static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn_oid fn, void *cb_data)
 {
 	return for_each_ref_in_submodule(submodule, "refs/bisect/bad", fn, cb_data);
 }
 
-static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)
+static int for_each_good_bisect_ref(const char *submodule, each_ref_fn_oid fn, void *cb_data)
 {
 	return for_each_ref_in_submodule(submodule, "refs/bisect/good", fn, cb_data);
 }
@@ -2106,17 +2120,17 @@ static int handle_revision_pseudo_opt(const char *submodule,
 		handle_refs(submodule, revs, *flags, head_ref_submodule);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--branches")) {
-		handle_refs(submodule, revs, *flags, for_each_branch_ref_submodule);
+		handle_refs_oid(submodule, revs, *flags, for_each_branch_ref_submodule);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--bisect")) {
-		handle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);
-		handle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM), for_each_good_bisect_ref);
+		handle_refs_oid(submodule, revs, *flags, for_each_bad_bisect_ref);
+		handle_refs_oid(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM), for_each_good_bisect_ref);
 		revs->bisect = 1;
 	} else if (!strcmp(arg, "--tags")) {
-		handle_refs(submodule, revs, *flags, for_each_tag_ref_submodule);
+		handle_refs_oid(submodule, revs, *flags, for_each_tag_ref_submodule);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--remotes")) {
-		handle_refs(submodule, revs, *flags, for_each_remote_ref_submodule);
+		handle_refs_oid(submodule, revs, *flags, for_each_remote_ref_submodule);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if ((argcount = parse_long_opt("glob", argv, &optarg))) {
 		struct all_refs_cb cb;
diff --git a/submodule.c b/submodule.c
index d491e6a..97e74c5 100644
--- a/submodule.c
+++ b/submodule.c
@@ -422,7 +422,7 @@ void set_config_fetch_recurse_submodules(int value)
 	config_fetch_recurse_submodules = value;
 }
 
-static int has_remote(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
+static int has_remote(const char *refname, const struct object_id *oid, int flags, void *cb_data)
 {
 	return 1;
 }
-- 
2.3.5

[PATCH v2 05/16] refs: convert head_ref to struct object_id

[brian m. carlson]

Convert head_ref and head_ref_submodule to use struct object_id.
Introduce some wrappers in some of the callers to handle
incompatibilities between each_ref_fn and each_ref_fn_oid.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/show-ref.c |  7 ++++++-
 log-tree.c         |  7 ++++++-
 reachable.c        |  7 ++++++-
 refs.c             | 16 ++++++++--------
 refs.h             |  4 ++--
 revision.c         |  2 +-
 shallow.c          | 19 ++++++++++++++++---
 7 files changed, 45 insertions(+), 17 deletions(-)

diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index afb1030..c6c5939 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -88,6 +88,11 @@ match:
 	return 0;
 }
 
+static int show_ref_oid(const char *refname, const struct object_id *oid, int flag, void *cbdata)
+{
+	return show_ref(refname, oid->hash, flag, cbdata);
+}
+
 static int add_existing(const char *refname, const unsigned char *sha1, int flag, void *cbdata)
 {
 	struct string_list *list = (struct string_list *)cbdata;
@@ -225,7 +230,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
 	}
 
 	if (show_head)
-		head_ref(show_ref, NULL);
+		head_ref(show_ref_oid, NULL);
 	for_each_ref(show_ref, NULL);
 	if (!found_match) {
 		if (verify && !quiet)
diff --git a/log-tree.c b/log-tree.c
index cf4646b..a29c17e 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -135,6 +135,11 @@ static int add_ref_decoration(const char *refname, const unsigned char *sha1, in
 	return 0;
 }
 
+static int add_ref_decoration_oid(const char *refname, const struct object_id *oid, int flags, void *cb_data)
+{
+	return add_ref_decoration(refname, oid->hash, flags, cb_data);
+}
+
 static int add_graft_decoration(const struct commit_graft *graft, void *cb_data)
 {
 	struct commit *commit = lookup_commit(graft->oid.hash);
@@ -150,7 +155,7 @@ void load_ref_decorations(int flags)
 	if (!loaded) {
 		loaded = 1;
 		for_each_ref(add_ref_decoration, &flags);
-		head_ref(add_ref_decoration, &flags);
+		head_ref(add_ref_decoration_oid, &flags);
 		for_each_commit_graft(add_graft_decoration, NULL);
 	}
 }
diff --git a/reachable.c b/reachable.c
index 69fa685..110ce92 100644
--- a/reachable.c
+++ b/reachable.c
@@ -32,6 +32,11 @@ static int add_one_ref(const char *path, const unsigned char *sha1, int flag, vo
 	return 0;
 }
 
+static int add_one_ref_oid(const char *path, const struct object_id *oid, int flag, void *cb_data)
+{
+	return add_one_ref(path, oid->hash, flag, cb_data);
+}
+
 /*
  * The traversal will have already marked us as SEEN, so we
  * only need to handle any progress reporting here.
@@ -171,7 +176,7 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
 	for_each_ref(add_one_ref, revs);
 
 	/* detached HEAD is not included in the list above */
-	head_ref(add_one_ref, revs);
+	head_ref(add_one_ref_oid, revs);
 
 	/* Add all reflog info */
 	if (mark_reflog)
diff --git a/refs.c b/refs.c
index aa13cc2..7a8b579 100644
--- a/refs.c
+++ b/refs.c
@@ -1981,30 +1981,30 @@ static int do_for_each_ref_oid(struct ref_cache *refs, const char *base,
 	return do_for_each_entry(refs, base, do_one_ref, &data);
 }
 
-static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
+static int do_head_ref(const char *submodule, each_ref_fn_oid fn, void *cb_data)
 {
-	unsigned char sha1[20];
+	struct object_id oid;
 	int flag;
 
 	if (submodule) {
-		if (resolve_gitlink_ref(submodule, "HEAD", sha1) == 0)
-			return fn("HEAD", sha1, 0, cb_data);
+		if (resolve_gitlink_ref(submodule, "HEAD", oid.hash) == 0)
+			return fn("HEAD", &oid, 0, cb_data);
 
 		return 0;
 	}
 
-	if (!read_ref_full("HEAD", RESOLVE_REF_READING, sha1, &flag))
-		return fn("HEAD", sha1, flag, cb_data);
+	if (!read_ref_full("HEAD", RESOLVE_REF_READING, oid.hash, &flag))
+		return fn("HEAD", &oid, flag, cb_data);
 
 	return 0;
 }
 
-int head_ref(each_ref_fn fn, void *cb_data)
+int head_ref(each_ref_fn_oid fn, void *cb_data)
 {
 	return do_head_ref(NULL, fn, cb_data);
 }
 
-int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+int head_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data)
 {
 	return do_head_ref(submodule, fn, cb_data);
 }
diff --git a/refs.h b/refs.h
index 62eb553..a14e7ef 100644
--- a/refs.h
+++ b/refs.h
@@ -86,7 +86,7 @@ typedef int each_ref_fn_oid(const char *refname,
  * modifies the reference also returns a nonzero value to immediately
  * stop the iteration.
  */
-extern int head_ref(each_ref_fn, void *);
+extern int head_ref(each_ref_fn_oid, void *);
 extern int for_each_ref(each_ref_fn, void *);
 extern int for_each_ref_in(const char *, each_ref_fn_oid, void *);
 extern int for_each_tag_ref(each_ref_fn_oid, void *);
@@ -96,7 +96,7 @@ extern int for_each_replace_ref(each_ref_fn, void *);
 extern int for_each_glob_ref(each_ref_fn, const char *pattern, void *);
 extern int for_each_glob_ref_in(each_ref_fn, const char *pattern, const char* prefix, void *);
 
-extern int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
+extern int head_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
 extern int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
 extern int for_each_ref_in_submodule(const char *submodule, const char *prefix,
 		each_ref_fn_oid fn, void *cb_data);
diff --git a/revision.c b/revision.c
index 93d9311..19005e8 100644
--- a/revision.c
+++ b/revision.c
@@ -2117,7 +2117,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	 */
 	if (!strcmp(arg, "--all")) {
 		handle_refs(submodule, revs, *flags, for_each_ref_submodule);
-		handle_refs(submodule, revs, *flags, head_ref_submodule);
+		handle_refs_oid(submodule, revs, *flags, head_ref_submodule);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--branches")) {
 		handle_refs_oid(submodule, revs, *flags, for_each_branch_ref_submodule);
diff --git a/shallow.c b/shallow.c
index d08d264..0343b16 100644
--- a/shallow.c
+++ b/shallow.c
@@ -487,6 +487,13 @@ static int mark_uninteresting(const char *refname,
 	return 0;
 }
 
+static int mark_uninteresting_oid(const char *refname,
+				const struct object_id *oid,
+				int flags, void *cb_data)
+{
+	return mark_uninteresting(refname, oid->hash, flags, cb_data);
+}
+
 static void post_assign_shallow(struct shallow_info *info,
 				struct ref_bitmap *ref_bitmap,
 				int *ref_status);
@@ -542,7 +549,7 @@ void assign_shallow_commits_to_refs(struct shallow_info *info,
 	 * connect to old refs. If not (e.g. force ref updates) it'll
 	 * have to go down to the current shallow commits.
 	 */
-	head_ref(mark_uninteresting, NULL);
+	head_ref(mark_uninteresting_oid, NULL);
 	for_each_ref(mark_uninteresting, NULL);
 
 	/* Mark potential bottoms so we won't go out of bound */
@@ -595,6 +602,12 @@ static int add_ref(const char *refname,
 	return 0;
 }
 
+static int add_ref_oid(const char *refname,
+		   const struct object_id *oid, int flags, void *cb_data)
+{
+	return add_ref(refname, oid->hash, flags, cb_data);
+}
+
 static void update_refstatus(int *ref_status, int nr, uint32_t *bitmap)
 {
 	int i;
@@ -641,7 +654,7 @@ static void post_assign_shallow(struct shallow_info *info,
 	info->nr_theirs = dst;
 
 	memset(&ca, 0, sizeof(ca));
-	head_ref(add_ref, &ca);
+	head_ref(add_ref_oid, &ca);
 	for_each_ref(add_ref, &ca);
 
 	/* Remove unreachable shallow commits from "ours" */
@@ -675,7 +688,7 @@ int delayed_reachability_test(struct shallow_info *si, int c)
 		if (!si->commits) {
 			struct commit_array ca;
 			memset(&ca, 0, sizeof(ca));
-			head_ref(add_ref, &ca);
+			head_ref(add_ref_oid, &ca);
 			for_each_ref(add_ref, &ca);
 			si->commits = ca.commits;
 			si->nr_commits = ca.nr;
-- 
2.3.5

[PATCH v2 06/16] refs: convert for_each_ref_submodule to struct object_id

[brian m. carlson]

Convert the callers as well.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 refs.c     | 4 ++--
 refs.h     | 2 +-
 revision.c | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/refs.c b/refs.c
index 7a8b579..68d0af8 100644
--- a/refs.c
+++ b/refs.c
@@ -2014,9 +2014,9 @@ int for_each_ref(each_ref_fn fn, void *cb_data)
 	return do_for_each_ref(&ref_cache, "", fn, 0, 0, cb_data);
 }
 
-int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
+int for_each_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data)
 {
-	return do_for_each_ref(get_ref_cache(submodule), "", fn, 0, 0, cb_data);
+	return do_for_each_ref_oid(get_ref_cache(submodule), "", fn, 0, 0, cb_data);
 }
 
 int for_each_ref_in(const char *prefix, each_ref_fn_oid fn, void *cb_data)
diff --git a/refs.h b/refs.h
index a14e7ef..2476951 100644
--- a/refs.h
+++ b/refs.h
@@ -97,7 +97,7 @@ extern int for_each_glob_ref(each_ref_fn, const char *pattern, void *);
 extern int for_each_glob_ref_in(each_ref_fn, const char *pattern, const char* prefix, void *);
 
 extern int head_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
-extern int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
+extern int for_each_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
 extern int for_each_ref_in_submodule(const char *submodule, const char *prefix,
 		each_ref_fn_oid fn, void *cb_data);
 extern int for_each_tag_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
diff --git a/revision.c b/revision.c
index 19005e8..825fbba 100644
--- a/revision.c
+++ b/revision.c
@@ -2116,7 +2116,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	 * register it in the list at the top of handle_revision_opt.
 	 */
 	if (!strcmp(arg, "--all")) {
-		handle_refs(submodule, revs, *flags, for_each_ref_submodule);
+		handle_refs_oid(submodule, revs, *flags, for_each_ref_submodule);
 		handle_refs_oid(submodule, revs, *flags, head_ref_submodule);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--branches")) {
-- 
2.3.5

[PATCH v2 07/16] revision: remove unused _oid helper.

[brian m. carlson]

Now that all the callers of handle_refs are gone, rename handle_refs_oid
to handle_refs and update the callers accordingly.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 revision.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/revision.c b/revision.c
index 825fbba..6fb499f 100644
--- a/revision.c
+++ b/revision.c
@@ -1264,14 +1264,6 @@ void add_ref_exclusion(struct string_list **ref_excludes_p, const char *exclude)
 }
 
 static void handle_refs(const char *submodule, struct rev_info *revs, unsigned flags,
-		int (*for_each)(const char *, each_ref_fn, void *))
-{
-	struct all_refs_cb cb;
-	init_all_refs_cb(&cb, revs, flags);
-	for_each(submodule, handle_one_ref, &cb);
-}
-
-static void handle_refs_oid(const char *submodule, struct rev_info *revs, unsigned flags,
 		int (*for_each)(const char *, each_ref_fn_oid, void *))
 {
 	struct all_refs_cb cb;
@@ -2116,21 +2108,21 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	 * register it in the list at the top of handle_revision_opt.
 	 */
 	if (!strcmp(arg, "--all")) {
-		handle_refs_oid(submodule, revs, *flags, for_each_ref_submodule);
-		handle_refs_oid(submodule, revs, *flags, head_ref_submodule);
+		handle_refs(submodule, revs, *flags, for_each_ref_submodule);
+		handle_refs(submodule, revs, *flags, head_ref_submodule);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--branches")) {
-		handle_refs_oid(submodule, revs, *flags, for_each_branch_ref_submodule);
+		handle_refs(submodule, revs, *flags, for_each_branch_ref_submodule);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--bisect")) {
-		handle_refs_oid(submodule, revs, *flags, for_each_bad_bisect_ref);
-		handle_refs_oid(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM), for_each_good_bisect_ref);
+		handle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);
+		handle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM), for_each_good_bisect_ref);
 		revs->bisect = 1;
 	} else if (!strcmp(arg, "--tags")) {
-		handle_refs_oid(submodule, revs, *flags, for_each_tag_ref_submodule);
+		handle_refs(submodule, revs, *flags, for_each_tag_ref_submodule);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--remotes")) {
-		handle_refs_oid(submodule, revs, *flags, for_each_remote_ref_submodule);
+		handle_refs(submodule, revs, *flags, for_each_remote_ref_submodule);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if ((argcount = parse_long_opt("glob", argv, &optarg))) {
 		struct all_refs_cb cb;
-- 
2.3.5

[PATCH v2 08/16] refs: convert for_each_ref to struct object_id

[brian m. carlson]

Convert for_each_ref, for_each_glob_ref, and for_each_glob_ref_in to use
struct object_id, as the latter two call the former with the function
pointer they are provided.

Convert callers to refer to properly-typed functions.  Convert uses of
the constant 20 to GIT_SHA1_RAWSZ.  Where possible, convert modified
functions to use struct object_id instead of unsigned char [20].

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/checkout.c     |  4 ++--
 builtin/fetch.c        |  6 +++---
 builtin/name-rev.c     |  6 +++---
 builtin/pack-objects.c | 10 +++++-----
 builtin/receive-pack.c |  4 ++--
 builtin/reflog.c       |  6 +++---
 builtin/remote.c       | 14 +++++++-------
 builtin/rev-parse.c    | 10 +++++-----
 builtin/show-branch.c  | 24 ++++++++++++------------
 builtin/show-ref.c     |  4 ++--
 fetch-pack.c           | 18 ++++++++++++++----
 help.c                 |  2 +-
 log-tree.c             |  2 +-
 notes.c                |  2 +-
 reachable.c            |  2 +-
 refs.c                 | 16 ++++++++--------
 refs.h                 |  6 +++---
 remote.c               |  8 ++++----
 revision.c             |  8 ++++----
 server-info.c          |  6 +++---
 sha1_name.c            |  4 ++--
 shallow.c              |  6 +++---
 submodule.c            |  4 ++--
 transport.c            | 10 +++++-----
 walker.c               |  4 ++--
 25 files changed, 98 insertions(+), 88 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 2f92328..9b49f0e 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -702,10 +702,10 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
 }
 
 static int add_pending_uninteresting_ref(const char *refname,
-					 const unsigned char *sha1,
+					 const struct object_id *oid,
 					 int flags, void *cb_data)
 {
-	add_pending_sha1(cb_data, refname, sha1, UNINTERESTING);
+	add_pending_sha1(cb_data, refname, oid->hash, UNINTERESTING);
 	return 0;
 }
 
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 7910419..2e792f3 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -179,13 +179,13 @@ static void add_merge_config(struct ref **head,
 	}
 }
 
-static int add_existing(const char *refname, const unsigned char *sha1,
+static int add_existing(const char *refname, const struct object_id *oid,
 			int flag, void *cbdata)
 {
 	struct string_list *list = (struct string_list *)cbdata;
 	struct string_list_item *item = string_list_insert(list, refname);
-	item->util = xmalloc(20);
-	hashcpy(item->util, sha1);
+	item->util = xmalloc(GIT_SHA1_RAWSZ);
+	hashcpy(item->util, oid->hash);
 	return 0;
 }
 
diff --git a/builtin/name-rev.c b/builtin/name-rev.c
index 9736d44..248a3eb 100644
--- a/builtin/name-rev.c
+++ b/builtin/name-rev.c
@@ -138,9 +138,9 @@ static int tipcmp(const void *a_, const void *b_)
 	return hashcmp(a->sha1, b->sha1);
 }
 
-static int name_ref(const char *path, const unsigned char *sha1, int flags, void *cb_data)
+static int name_ref(const char *path, const struct object_id *oid, int flags, void *cb_data)
 {
-	struct object *o = parse_object(sha1);
+	struct object *o = parse_object(oid->hash);
 	struct name_ref_data *data = cb_data;
 	int can_abbreviate_output = data->tags_only && data->name_only;
 	int deref = 0;
@@ -160,7 +160,7 @@ static int name_ref(const char *path, const unsigned char *sha1, int flags, void
 		}
 	}
 
-	add_to_tip_table(sha1, path, can_abbreviate_output);
+	add_to_tip_table(oid->hash, path, can_abbreviate_output);
 
 	while (o && o->type == OBJ_TAG) {
 		struct tag *t = (struct tag *) o;
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 0c69b0c..80fe8c7 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2097,14 +2097,14 @@ static void ll_find_deltas(struct object_entry **list, unsigned list_size,
 #define ll_find_deltas(l, s, w, d, p)	find_deltas(l, &s, w, d, p)
 #endif
 
-static int add_ref_tag(const char *path, const unsigned char *sha1, int flag, void *cb_data)
+static int add_ref_tag(const char *path, const struct object_id *oid, int flag, void *cb_data)
 {
-	unsigned char peeled[20];
+	struct object_id peeled;
 
 	if (starts_with(path, "refs/tags/") && /* is a tag? */
-	    !peel_ref(path, peeled)        && /* peelable? */
-	    packlist_find(&to_pack, peeled, NULL))      /* object packed? */
-		add_object_entry(sha1, OBJ_TAG, NULL, 0);
+	    !peel_ref(path, peeled.hash)    && /* peelable? */
+	    packlist_find(&to_pack, peeled.hash, NULL))      /* object packed? */
+		add_object_entry(oid->hash, OBJ_TAG, NULL, 0);
 	return 0;
 }
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index d2ec52b..85e60e2 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -197,7 +197,7 @@ static void show_ref(const char *path, const unsigned char *sha1)
 	}
 }
 
-static int show_ref_cb(const char *path, const unsigned char *sha1, int flag, void *unused)
+static int show_ref_cb(const char *path, const struct object_id *oid, int flag, void *unused)
 {
 	path = strip_namespace(path);
 	/*
@@ -210,7 +210,7 @@ static int show_ref_cb(const char *path, const unsigned char *sha1, int flag, vo
 	 */
 	if (!path)
 		path = ".have";
-	show_ref(path, sha1);
+	show_ref(path, oid->hash);
 	return 0;
 }
 
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 8182b64..f3f9201 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -313,14 +313,14 @@ static int should_expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 	return 0;
 }
 
-static int push_tip_to_list(const char *refname, const unsigned char *sha1,
-			    int flags, void *cb_data)
+static int push_tip_to_list(const char *refname, const struct object_id *oid,
+				int flags, void *cb_data)
 {
 	struct commit_list **list = cb_data;
 	struct commit *tip_commit;
 	if (flags & REF_ISSYMREF)
 		return 0;
-	tip_commit = lookup_commit_reference_gently(sha1, 1);
+	tip_commit = lookup_commit_reference_gently(oid->hash, 1);
 	if (!tip_commit)
 		return 0;
 	commit_list_insert(tip_commit, list);
diff --git a/builtin/remote.c b/builtin/remote.c
index ad57fc9..67307ed 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -509,7 +509,7 @@ struct branches_for_remote {
 };
 
 static int add_branch_for_removal(const char *refname,
-	const unsigned char *sha1, int flags, void *cb_data)
+	const struct object_id *oid, int flags, void *cb_data)
 {
 	struct branches_for_remote *branches = cb_data;
 	struct refspec refspec;
@@ -544,8 +544,8 @@ static int add_branch_for_removal(const char *refname,
 		return unlink(git_path("%s", refname));
 
 	item = string_list_append(branches->branches, refname);
-	item->util = xmalloc(20);
-	hashcpy(item->util, sha1);
+	item->util = xmalloc(GIT_SHA1_RAWSZ);
+	hashcpy(item->util, oid->hash);
 
 	return 0;
 }
@@ -557,20 +557,20 @@ struct rename_info {
 };
 
 static int read_remote_branches(const char *refname,
-	const unsigned char *sha1, int flags, void *cb_data)
+	const struct object_id *oid, int flags, void *cb_data)
 {
 	struct rename_info *rename = cb_data;
 	struct strbuf buf = STRBUF_INIT;
 	struct string_list_item *item;
 	int flag;
-	unsigned char orig_sha1[20];
+	struct object_id orig_oid;
 	const char *symref;
 
 	strbuf_addf(&buf, "refs/remotes/%s/", rename->old);
 	if (starts_with(refname, buf.buf)) {
 		item = string_list_append(rename->remote_branches, xstrdup(refname));
 		symref = resolve_ref_unsafe(refname, RESOLVE_REF_READING,
-					    orig_sha1, &flag);
+					    orig_oid.hash, &flag);
 		if (flag & REF_ISSYMREF)
 			item->util = xstrdup(symref);
 		else
@@ -867,7 +867,7 @@ static void free_remote_ref_states(struct ref_states *states)
 }
 
 static int append_ref_to_tracked_list(const char *refname,
-	const unsigned char *sha1, int flags, void *cb_data)
+	const struct object_id *oid, int flags, void *cb_data)
 {
 	struct ref_states *states = cb_data;
 	struct refspec refspec;
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index c9f2c93..4aa7a25 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -657,7 +657,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!strcmp(arg, "--all")) {
-				for_each_ref(show_reference, NULL);
+				for_each_ref(show_reference_oid, NULL);
 				continue;
 			}
 			if (starts_with(arg, "--disambiguate=")) {
@@ -670,7 +670,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (starts_with(arg, "--branches=")) {
-				for_each_glob_ref_in(show_reference, arg + 11,
+				for_each_glob_ref_in(show_reference_oid, arg + 11,
 					"refs/heads/", NULL);
 				clear_ref_exclusion(&ref_excludes);
 				continue;
@@ -681,7 +681,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (starts_with(arg, "--tags=")) {
-				for_each_glob_ref_in(show_reference, arg + 7,
+				for_each_glob_ref_in(show_reference_oid, arg + 7,
 					"refs/tags/", NULL);
 				clear_ref_exclusion(&ref_excludes);
 				continue;
@@ -692,12 +692,12 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (starts_with(arg, "--glob=")) {
-				for_each_glob_ref(show_reference, arg + 7, NULL);
+				for_each_glob_ref(show_reference_oid, arg + 7, NULL);
 				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
 			if (starts_with(arg, "--remotes=")) {
-				for_each_glob_ref_in(show_reference, arg + 10,
+				for_each_glob_ref_in(show_reference_oid, arg + 10,
 					"refs/remotes/", NULL);
 				clear_ref_exclusion(&ref_excludes);
 				continue;
diff --git a/builtin/show-branch.c b/builtin/show-branch.c
index e69fb7c..e04fa8e 100644
--- a/builtin/show-branch.c
+++ b/builtin/show-branch.c
@@ -394,32 +394,32 @@ static int append_ref(const char *refname, const unsigned char *sha1,
 	return 0;
 }
 
-static int append_head_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+static int append_head_ref(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
-	unsigned char tmp[20];
+	struct object_id tmp;
 	int ofs = 11;
 	if (!starts_with(refname, "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))
+	if (get_sha1(refname + ofs, tmp.hash) || oidcmp(&tmp, oid))
 		ofs = 5;
-	return append_ref(refname + ofs, sha1, 0);
+	return append_ref(refname + ofs, oid->hash, 0);
 }
 
-static int append_remote_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+static int append_remote_ref(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
-	unsigned char tmp[20];
+	struct object_id tmp;
 	int ofs = 13;
 	if (!starts_with(refname, "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))
+	if (get_sha1(refname + ofs, tmp.hash) || oidcmp(&tmp, oid))
 		ofs = 5;
-	return append_ref(refname + ofs, sha1, 0);
+	return append_ref(refname + ofs, oid->hash, 0);
 }
 
 static int append_tag_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
@@ -440,7 +440,7 @@ static int count_slash(const char *s)
 	return cnt;
 }
 
-static int append_matching_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+static int append_matching_ref(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
 	/* we want to allow pattern hold/<asterisk> to show all
 	 * branches under refs/heads/hold/, and v0.99.9? to show
@@ -456,10 +456,10 @@ static int append_matching_ref(const char *refname, const unsigned char *sha1, i
 	if (wildmatch(match_ref_pattern, tail, 0, NULL))
 		return 0;
 	if (starts_with(refname, "refs/heads/"))
-		return append_head_ref(refname, sha1, flag, cb_data);
+		return append_head_ref(refname, oid, flag, cb_data);
 	if (starts_with(refname, "refs/tags/"))
-		return append_tag_ref(refname, sha1, flag, cb_data);
-	return append_ref(refname, sha1, 0);
+		return append_tag_ref(refname, oid->hash, flag, cb_data);
+	return append_ref(refname, oid->hash, 0);
 }
 
 static void snarf_refs(int head, int remotes)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index c6c5939..6731721 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -93,7 +93,7 @@ static int show_ref_oid(const char *refname, const struct object_id *oid, int fl
 	return show_ref(refname, oid->hash, flag, cbdata);
 }
 
-static int add_existing(const char *refname, const unsigned char *sha1, int flag, void *cbdata)
+static int add_existing(const char *refname, const struct object_id *oid, int flag, void *cbdata)
 {
 	struct string_list *list = (struct string_list *)cbdata;
 	string_list_insert(list, refname);
@@ -231,7 +231,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
 
 	if (show_head)
 		head_ref(show_ref_oid, NULL);
-	for_each_ref(show_ref, NULL);
+	for_each_ref(show_ref_oid, NULL);
 	if (!found_match) {
 		if (verify && !quiet)
 			die("No match");
diff --git a/fetch-pack.c b/fetch-pack.c
index 48526aa..65a57e1 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -70,9 +70,14 @@ static int rev_list_insert_ref(const char *refname, const unsigned char *sha1, i
 	return 0;
 }
 
-static int clear_marks(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+static int rev_list_insert_ref_oid(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
-	struct object *o = deref_tag(parse_object(sha1), refname, 0);
+	return rev_list_insert_ref(refname, oid->hash, flag, cb_data);
+}
+
+static int clear_marks(const char *refname, const struct object_id *oid, int flag, void *cb_data)
+{
+	struct object *o = deref_tag(parse_object(oid->hash), refname, 0);
 
 	if (o && o->type == OBJ_COMMIT)
 		clear_commit_marks((struct commit *)o,
@@ -263,7 +268,7 @@ static int find_common(struct fetch_pack_args *args,
 		for_each_ref(clear_marks, NULL);
 	marked = 1;
 
-	for_each_ref(rev_list_insert_ref, NULL);
+	for_each_ref(rev_list_insert_ref_oid, NULL);
 	for_each_alternate_ref(insert_one_alternate_ref, NULL);
 
 	fetching = 0;
@@ -487,6 +492,11 @@ static int mark_complete(const char *refname, const unsigned char *sha1, int fla
 	return 0;
 }
 
+static int mark_complete_oid(const char *refname, const struct object_id *oid, int flag, void *cb_data)
+{
+	return mark_complete(refname, oid->hash, flag, cb_data);
+}
+
 static void mark_recent_complete_commits(struct fetch_pack_args *args,
 					 unsigned long cutoff)
 {
@@ -599,7 +609,7 @@ static int everything_local(struct fetch_pack_args *args,
 	}
 
 	if (!args->depth) {
-		for_each_ref(mark_complete, NULL);
+		for_each_ref(mark_complete_oid, NULL);
 		for_each_alternate_ref(mark_alternate_complete, NULL);
 		commit_list_sort_by_date(&complete);
 		if (cutoff)
diff --git a/help.c b/help.c
index 2072a87..6f3415b 100644
--- a/help.c
+++ b/help.c
@@ -407,7 +407,7 @@ struct similar_ref_cb {
 	struct string_list *similar_refs;
 };
 
-static int append_similar_ref(const char *refname, const unsigned char *sha1,
+static int append_similar_ref(const char *refname, const struct object_id *oid,
 			      int flags, void *cb_data)
 {
 	struct similar_ref_cb *cb = (struct similar_ref_cb *)(cb_data);
diff --git a/log-tree.c b/log-tree.c
index a29c17e..acbc859 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -154,7 +154,7 @@ void load_ref_decorations(int flags)
 	static int loaded;
 	if (!loaded) {
 		loaded = 1;
-		for_each_ref(add_ref_decoration, &flags);
+		for_each_ref(add_ref_decoration_oid, &flags);
 		head_ref(add_ref_decoration_oid, &flags);
 		for_each_commit_graft(add_graft_decoration, NULL);
 	}
diff --git a/notes.c b/notes.c
index 2be4d7f..df08209 100644
--- a/notes.c
+++ b/notes.c
@@ -918,7 +918,7 @@ out:
 	return ret;
 }
 
-static int string_list_add_one_ref(const char *refname, const unsigned char *sha1,
+static int string_list_add_one_ref(const char *refname, const struct object_id *oid,
 				   int flag, void *cb)
 {
 	struct string_list *refs = cb;
diff --git a/reachable.c b/reachable.c
index 110ce92..b2ca0c3 100644
--- a/reachable.c
+++ b/reachable.c
@@ -173,7 +173,7 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
 	add_index_objects_to_pending(revs, 0);
 
 	/* Add all external refs */
-	for_each_ref(add_one_ref, revs);
+	for_each_ref(add_one_ref_oid, revs);
 
 	/* detached HEAD is not included in the list above */
 	head_ref(add_one_ref_oid, revs);
diff --git a/refs.c b/refs.c
index 68d0af8..6bf7abc 100644
--- a/refs.c
+++ b/refs.c
@@ -1675,7 +1675,7 @@ char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, in
 /* The argument to filter_refs */
 struct ref_filter {
 	const char *pattern;
-	each_ref_fn *fn;
+	each_ref_fn_oid *fn;
 	void *cb_data;
 };
 
@@ -1697,13 +1697,13 @@ int ref_exists(const char *refname)
 	return !!resolve_ref_unsafe(refname, RESOLVE_REF_READING, sha1, NULL);
 }
 
-static int filter_refs(const char *refname, const unsigned char *sha1, int flags,
-		       void *data)
+static int filter_refs(const char *refname, const struct object_id *oid,
+			   int flags, void *data)
 {
 	struct ref_filter *filter = (struct ref_filter *)data;
 	if (wildmatch(filter->pattern, refname, 0, NULL))
 		return 0;
-	return filter->fn(refname, sha1, flags, filter->cb_data);
+	return filter->fn(refname, oid, flags, filter->cb_data);
 }
 
 enum peel_status {
@@ -2009,9 +2009,9 @@ int head_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data)
 	return do_head_ref(submodule, fn, cb_data);
 }
 
-int for_each_ref(each_ref_fn fn, void *cb_data)
+int for_each_ref(each_ref_fn_oid fn, void *cb_data)
 {
-	return do_for_each_ref(&ref_cache, "", fn, 0, 0, cb_data);
+	return do_for_each_ref_oid(&ref_cache, "", fn, 0, 0, cb_data);
 }
 
 int for_each_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data)
@@ -2090,7 +2090,7 @@ int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
 	return ret;
 }
 
-int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
+int for_each_glob_ref_in(each_ref_fn_oid fn, const char *pattern,
 	const char *prefix, void *cb_data)
 {
 	struct strbuf real_pattern = STRBUF_INIT;
@@ -2120,7 +2120,7 @@ int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
 	return ret;
 }
 
-int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data)
+int for_each_glob_ref(each_ref_fn_oid fn, const char *pattern, void *cb_data)
 {
 	return for_each_glob_ref_in(fn, pattern, NULL, cb_data);
 }
diff --git a/refs.h b/refs.h
index 2476951..c041cd1 100644
--- a/refs.h
+++ b/refs.h
@@ -87,14 +87,14 @@ typedef int each_ref_fn_oid(const char *refname,
  * stop the iteration.
  */
 extern int head_ref(each_ref_fn_oid, void *);
-extern int for_each_ref(each_ref_fn, void *);
+extern int for_each_ref(each_ref_fn_oid, void *);
 extern int for_each_ref_in(const char *, each_ref_fn_oid, void *);
 extern int for_each_tag_ref(each_ref_fn_oid, void *);
 extern int for_each_branch_ref(each_ref_fn_oid, void *);
 extern int for_each_remote_ref(each_ref_fn_oid, void *);
 extern int for_each_replace_ref(each_ref_fn, void *);
-extern int for_each_glob_ref(each_ref_fn, const char *pattern, void *);
-extern int for_each_glob_ref_in(each_ref_fn, const char *pattern, const char* prefix, void *);
+extern int for_each_glob_ref(each_ref_fn_oid, const char *pattern, void *);
+extern int for_each_glob_ref_in(each_ref_fn_oid, const char *pattern, const char* prefix, void *);
 
 extern int head_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
 extern int for_each_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
diff --git a/remote.c b/remote.c
index 68901b0..b544997 100644
--- a/remote.c
+++ b/remote.c
@@ -2024,7 +2024,7 @@ int format_tracking_info(struct branch *branch, struct strbuf *sb)
 	return 1;
 }
 
-static int one_local_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+static int one_local_ref(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
 	struct ref ***local_tail = cb_data;
 	struct ref *ref;
@@ -2036,7 +2036,7 @@ static int one_local_ref(const char *refname, const unsigned char *sha1, int fla
 
 	len = strlen(refname) + 1;
 	ref = xcalloc(1, sizeof(*ref) + len);
-	hashcpy(ref->new_sha1, sha1);
+	hashcpy(ref->new_sha1, oid->hash);
 	memcpy(ref->name, refname, len);
 	**local_tail = ref;
 	*local_tail = &ref->next;
@@ -2099,7 +2099,7 @@ struct stale_heads_info {
 };
 
 static int get_stale_heads_cb(const char *refname,
-	const unsigned char *sha1, int flags, void *cb_data)
+	const struct object_id *oid, int flags, void *cb_data)
 {
 	struct stale_heads_info *info = cb_data;
 	struct string_list matches = STRING_LIST_INIT_DUP;
@@ -2128,7 +2128,7 @@ static int get_stale_heads_cb(const char *refname,
 
 	if (stale) {
 		struct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);
-		hashcpy(ref->new_sha1, sha1);
+		hashcpy(ref->new_sha1, oid->hash);
 	}
 
 clean_exit:
diff --git a/revision.c b/revision.c
index 6fb499f..29af759 100644
--- a/revision.c
+++ b/revision.c
@@ -2127,7 +2127,7 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	} else if ((argcount = parse_long_opt("glob", argv, &optarg))) {
 		struct all_refs_cb cb;
 		init_all_refs_cb(&cb, revs, *flags);
-		for_each_glob_ref(handle_one_ref, optarg, &cb);
+		for_each_glob_ref(handle_one_ref_oid, optarg, &cb);
 		clear_ref_exclusion(&revs->ref_excludes);
 		return argcount;
 	} else if ((argcount = parse_long_opt("exclude", argv, &optarg))) {
@@ -2136,17 +2136,17 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	} else if (starts_with(arg, "--branches=")) {
 		struct all_refs_cb cb;
 		init_all_refs_cb(&cb, revs, *flags);
-		for_each_glob_ref_in(handle_one_ref, arg + 11, "refs/heads/", &cb);
+		for_each_glob_ref_in(handle_one_ref_oid, arg + 11, "refs/heads/", &cb);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (starts_with(arg, "--tags=")) {
 		struct all_refs_cb cb;
 		init_all_refs_cb(&cb, revs, *flags);
-		for_each_glob_ref_in(handle_one_ref, arg + 7, "refs/tags/", &cb);
+		for_each_glob_ref_in(handle_one_ref_oid, arg + 7, "refs/tags/", &cb);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (starts_with(arg, "--remotes=")) {
 		struct all_refs_cb cb;
 		init_all_refs_cb(&cb, revs, *flags);
-		for_each_glob_ref_in(handle_one_ref, arg + 10, "refs/remotes/", &cb);
+		for_each_glob_ref_in(handle_one_ref_oid, arg + 10, "refs/remotes/", &cb);
 		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--reflog")) {
 		add_reflogs_to_pending(revs, *flags);
diff --git a/server-info.c b/server-info.c
index 34b0253..11f21bd 100644
--- a/server-info.c
+++ b/server-info.c
@@ -47,14 +47,14 @@ out:
 	return ret;
 }
 
-static int add_info_ref(const char *path, const unsigned char *sha1, int flag, void *cb_data)
+static int add_info_ref(const char *path, const struct object_id *oid, int flag, void *cb_data)
 {
 	FILE *fp = cb_data;
-	struct object *o = parse_object(sha1);
+	struct object *o = parse_object(oid->hash);
 	if (!o)
 		return -1;
 
-	if (fprintf(fp, "%s	%s\n", sha1_to_hex(sha1), path) < 0)
+	if (fprintf(fp, "%s	%s\n", oid_to_hex(oid), path) < 0)
 		return -1;
 
 	if (o->type == OBJ_TAG) {
diff --git a/sha1_name.c b/sha1_name.c
index 6d10f05..959e6c1 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -832,10 +832,10 @@ static int get_sha1_1(const char *name, int len, unsigned char *sha1, unsigned l
 #define ONELINE_SEEN (1u<<20)
 
 static int handle_one_ref(const char *path,
-		const unsigned char *sha1, int flag, void *cb_data)
+		const struct object_id *oid, int flag, void *cb_data)
 {
 	struct commit_list **list = cb_data;
-	struct object *object = parse_object(sha1);
+	struct object *object = parse_object(oid->hash);
 	if (!object)
 		return 0;
 	if (object->type == OBJ_TAG) {
diff --git a/shallow.c b/shallow.c
index 0343b16..6511bbb 100644
--- a/shallow.c
+++ b/shallow.c
@@ -550,7 +550,7 @@ void assign_shallow_commits_to_refs(struct shallow_info *info,
 	 * have to go down to the current shallow commits.
 	 */
 	head_ref(mark_uninteresting_oid, NULL);
-	for_each_ref(mark_uninteresting, NULL);
+	for_each_ref(mark_uninteresting_oid, NULL);
 
 	/* Mark potential bottoms so we won't go out of bound */
 	for (i = 0; i < nr_shallow; i++) {
@@ -655,7 +655,7 @@ static void post_assign_shallow(struct shallow_info *info,
 
 	memset(&ca, 0, sizeof(ca));
 	head_ref(add_ref_oid, &ca);
-	for_each_ref(add_ref, &ca);
+	for_each_ref(add_ref_oid, &ca);
 
 	/* Remove unreachable shallow commits from "ours" */
 	for (i = dst = 0; i < info->nr_ours; i++) {
@@ -689,7 +689,7 @@ int delayed_reachability_test(struct shallow_info *si, int c)
 			struct commit_array ca;
 			memset(&ca, 0, sizeof(ca));
 			head_ref(add_ref_oid, &ca);
-			for_each_ref(add_ref, &ca);
+			for_each_ref(add_ref_oid, &ca);
 			si->commits = ca.commits;
 			si->nr_commits = ca.nr;
 		}
diff --git a/submodule.c b/submodule.c
index 97e74c5..b284444 100644
--- a/submodule.c
+++ b/submodule.c
@@ -616,10 +616,10 @@ static void submodule_collect_changed_cb(struct diff_queue_struct *q,
 	}
 }
 
-static int add_sha1_to_array(const char *ref, const unsigned char *sha1,
+static int add_sha1_to_array(const char *ref, const struct object_id *oid,
 			     int flags, void *data)
 {
-	sha1_array_append(data, sha1);
+	sha1_array_append(data, oid->hash);
 	return 0;
 }
 
diff --git a/transport.c b/transport.c
index f080e93..c13108b 100644
--- a/transport.c
+++ b/transport.c
@@ -278,7 +278,7 @@ static int fetch_objs_via_rsync(struct transport *transport,
 	return run_command(&rsync);
 }
 
-static int write_one_ref(const char *name, const unsigned char *sha1,
+static int write_one_ref(const char *name, const struct object_id *oid,
 		int flags, void *data)
 {
 	struct strbuf *buf = data;
@@ -291,7 +291,7 @@ static int write_one_ref(const char *name, const unsigned char *sha1,
 
 	strbuf_addstr(buf, name);
 	if (safe_create_leading_directories(buf->buf) ||
-	    write_file(buf->buf, 0, "%s\n", sha1_to_hex(sha1)))
+	    write_file(buf->buf, 0, "%s\n", oid_to_hex(oid)))
 		return error("problems writing temporary file %s: %s",
 			     buf->buf, strerror(errno));
 	strbuf_setlen(buf, len);
@@ -304,13 +304,13 @@ static int write_refs_to_temp_dir(struct strbuf *temp_dir,
 	int i;
 
 	for (i = 0; i < refspec_nr; i++) {
-		unsigned char sha1[20];
+		struct object_id oid;
 		char *ref;
 
-		if (dwim_ref(refspec[i], strlen(refspec[i]), sha1, &ref) != 1)
+		if (dwim_ref(refspec[i], strlen(refspec[i]), oid.hash, &ref) != 1)
 			return error("Could not get ref %s", refspec[i]);
 
-		if (write_one_ref(ref, sha1, 0, temp_dir)) {
+		if (write_one_ref(ref, &oid, 0, temp_dir)) {
 			free(ref);
 			return -1;
 		}
diff --git a/walker.c b/walker.c
index 58ffeca..033c8f2 100644
--- a/walker.c
+++ b/walker.c
@@ -200,9 +200,9 @@ static int interpret_target(struct walker *walker, char *target, unsigned char *
 	return -1;
 }
 
-static int mark_complete(const char *path, const unsigned char *sha1, int flag, void *cb_data)
+static int mark_complete(const char *path, const struct object_id *oid, int flag, void *cb_data)
 {
-	struct commit *commit = lookup_commit_reference_gently(sha1, 1);
+	struct commit *commit = lookup_commit_reference_gently(oid->hash, 1);
 	if (commit) {
 		commit->object.flags |= COMPLETE;
 		commit_list_insert(commit, &complete);
-- 
2.3.5

[PATCH v2 09/16] refs: convert for_each_replace_ref to struct object_id

[brian m. carlson]

Update callbacks to take the proper parameters and use struct object_id
elsewhere in the callbacks.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/replace.c | 14 +++++++-------
 refs.c            |  4 ++--
 refs.h            |  2 +-
 replace_object.c  |  4 ++--
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/builtin/replace.c b/builtin/replace.c
index 54bf01a..a569420 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -35,7 +35,7 @@ struct show_data {
 	enum replace_format format;
 };
 
-static int show_reference(const char *refname, const unsigned char *sha1,
+static int show_reference(const char *refname, const struct object_id *oid,
 			  int flag, void *cb_data)
 {
 	struct show_data *data = cb_data;
@@ -44,19 +44,19 @@ static int show_reference(const char *refname, const unsigned char *sha1,
 		if (data->format == REPLACE_FORMAT_SHORT)
 			printf("%s\n", refname);
 		else if (data->format == REPLACE_FORMAT_MEDIUM)
-			printf("%s -> %s\n", refname, sha1_to_hex(sha1));
+			printf("%s -> %s\n", refname, oid_to_hex(oid));
 		else { /* data->format == REPLACE_FORMAT_LONG */
-			unsigned char object[20];
+			struct object_id object;
 			enum object_type obj_type, repl_type;
 
-			if (get_sha1(refname, object))
+			if (get_sha1(refname, object.hash))
 				return error("Failed to resolve '%s' as a valid ref.", refname);
 
-			obj_type = sha1_object_info(object, NULL);
-			repl_type = sha1_object_info(sha1, NULL);
+			obj_type = sha1_object_info(object.hash, NULL);
+			repl_type = sha1_object_info(oid->hash, NULL);
 
 			printf("%s (%s) -> %s (%s)\n", refname, typename(obj_type),
-			       sha1_to_hex(sha1), typename(repl_type));
+			       oid_to_hex(oid), typename(repl_type));
 		}
 	}
 
diff --git a/refs.c b/refs.c
index 6bf7abc..89606a7 100644
--- a/refs.c
+++ b/refs.c
@@ -2060,9 +2060,9 @@ int for_each_remote_ref_submodule(const char *submodule, each_ref_fn_oid fn, voi
 	return for_each_ref_in_submodule(submodule, "refs/remotes/", fn, cb_data);
 }
 
-int for_each_replace_ref(each_ref_fn fn, void *cb_data)
+int for_each_replace_ref(each_ref_fn_oid fn, void *cb_data)
 {
-	return do_for_each_ref(&ref_cache, "refs/replace/", fn, 13, 0, cb_data);
+	return do_for_each_ref_oid(&ref_cache, "refs/replace/", fn, 13, 0, cb_data);
 }
 
 int head_ref_namespaced(each_ref_fn fn, void *cb_data)
diff --git a/refs.h b/refs.h
index c041cd1..9c6bc06 100644
--- a/refs.h
+++ b/refs.h
@@ -92,7 +92,7 @@ extern int for_each_ref_in(const char *, each_ref_fn_oid, void *);
 extern int for_each_tag_ref(each_ref_fn_oid, void *);
 extern int for_each_branch_ref(each_ref_fn_oid, void *);
 extern int for_each_remote_ref(each_ref_fn_oid, void *);
-extern int for_each_replace_ref(each_ref_fn, void *);
+extern int for_each_replace_ref(each_ref_fn_oid, void *);
 extern int for_each_glob_ref(each_ref_fn_oid, const char *pattern, void *);
 extern int for_each_glob_ref_in(each_ref_fn_oid, const char *pattern, const char* prefix, void *);
 
diff --git a/replace_object.c b/replace_object.c
index 0ab2dc1..f0b39f0 100644
--- a/replace_object.c
+++ b/replace_object.c
@@ -53,7 +53,7 @@ static int register_replace_object(struct replace_object *replace,
 }
 
 static int register_replace_ref(const char *refname,
-				const unsigned char *sha1,
+				const struct object_id *oid,
 				int flag, void *cb_data)
 {
 	/* Get sha1 from refname */
@@ -68,7 +68,7 @@ static int register_replace_ref(const char *refname,
 	}
 
 	/* Copy sha1 from the read ref */
-	hashcpy(repl_obj->replacement, sha1);
+	hashcpy(repl_obj->replacement, oid->hash);
 
 	/* Register new object */
 	if (register_replace_object(repl_obj, 1))
-- 
2.3.5

[PATCH v2 10/16] refs: convert namespaced ref iteration functions to object_id

[brian m. carlson]

Convert head_ref_namespaced and for_each_namespaced_ref to use struct
object_id.  Update the various callbacks to use struct object_id
internally as well.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 http-backend.c | 14 +++++++-------
 refs.c         | 12 ++++++------
 refs.h         |  4 ++--
 upload-pack.c  | 30 +++++++++++++++---------------
 4 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/http-backend.c b/http-backend.c
index b6c0484..e0d6627 100644
--- a/http-backend.c
+++ b/http-backend.c
@@ -350,16 +350,16 @@ static void run_service(const char **argv)
 		exit(1);
 }
 
-static int show_text_ref(const char *name, const unsigned char *sha1,
+static int show_text_ref(const char *name, const struct object_id *oid,
 	int flag, void *cb_data)
 {
 	const char *name_nons = strip_namespace(name);
 	struct strbuf *buf = cb_data;
-	struct object *o = parse_object(sha1);
+	struct object *o = parse_object(oid->hash);
 	if (!o)
 		return 0;
 
-	strbuf_addf(buf, "%s\t%s\n", sha1_to_hex(sha1), name_nons);
+	strbuf_addf(buf, "%s\t%s\n", oid_to_hex(oid), name_nons);
 	if (o->type == OBJ_TAG) {
 		o = deref_tag(o, name, 0);
 		if (!o)
@@ -402,21 +402,21 @@ static void get_info_refs(char *arg)
 	strbuf_release(&buf);
 }
 
-static int show_head_ref(const char *refname, const unsigned char *sha1,
+static int show_head_ref(const char *refname, const struct object_id *oid,
 	int flag, void *cb_data)
 {
 	struct strbuf *buf = cb_data;
 
 	if (flag & REF_ISSYMREF) {
-		unsigned char unused[20];
+		struct object_id unused;
 		const char *target = resolve_ref_unsafe(refname,
 							RESOLVE_REF_READING,
-							unused, NULL);
+							unused.hash, NULL);
 		const char *target_nons = strip_namespace(target);
 
 		strbuf_addf(buf, "ref: %s\n", target_nons);
 	} else {
-		strbuf_addf(buf, "%s\n", sha1_to_hex(sha1));
+		strbuf_addf(buf, "%s\n", oid_to_hex(oid));
 	}
 
 	return 0;
diff --git a/refs.c b/refs.c
index 89606a7..19ce65a 100644
--- a/refs.c
+++ b/refs.c
@@ -2065,27 +2065,27 @@ int for_each_replace_ref(each_ref_fn_oid fn, void *cb_data)
 	return do_for_each_ref_oid(&ref_cache, "refs/replace/", fn, 13, 0, cb_data);
 }
 
-int head_ref_namespaced(each_ref_fn fn, void *cb_data)
+int head_ref_namespaced(each_ref_fn_oid fn, void *cb_data)
 {
 	struct strbuf buf = STRBUF_INIT;
 	int ret = 0;
-	unsigned char sha1[20];
+	struct object_id oid;
 	int flag;
 
 	strbuf_addf(&buf, "%sHEAD", get_git_namespace());
-	if (!read_ref_full(buf.buf, RESOLVE_REF_READING, sha1, &flag))
-		ret = fn(buf.buf, sha1, flag, cb_data);
+	if (!read_ref_full(buf.buf, RESOLVE_REF_READING, oid.hash, &flag))
+		ret = fn(buf.buf, &oid, flag, cb_data);
 	strbuf_release(&buf);
 
 	return ret;
 }
 
-int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
+int for_each_namespaced_ref(each_ref_fn_oid fn, void *cb_data)
 {
 	struct strbuf buf = STRBUF_INIT;
 	int ret;
 	strbuf_addf(&buf, "%srefs/", get_git_namespace());
-	ret = do_for_each_ref(&ref_cache, buf.buf, fn, 0, 0, cb_data);
+	ret = do_for_each_ref_oid(&ref_cache, buf.buf, fn, 0, 0, cb_data);
 	strbuf_release(&buf);
 	return ret;
 }
diff --git a/refs.h b/refs.h
index 9c6bc06..2b2a688 100644
--- a/refs.h
+++ b/refs.h
@@ -104,8 +104,8 @@ extern int for_each_tag_ref_submodule(const char *submodule, each_ref_fn_oid fn,
 extern int for_each_branch_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
 extern int for_each_remote_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
 
-extern int head_ref_namespaced(each_ref_fn fn, void *cb_data);
-extern int for_each_namespaced_ref(each_ref_fn fn, void *cb_data);
+extern int head_ref_namespaced(each_ref_fn_oid fn, void *cb_data);
+extern int for_each_namespaced_ref(each_ref_fn_oid fn, void *cb_data);
 
 static inline const char *has_glob_specials(const char *pattern)
 {
diff --git a/upload-pack.c b/upload-pack.c
index 745fda8..b60086f 100644
--- a/upload-pack.c
+++ b/upload-pack.c
@@ -681,9 +681,9 @@ static void receive_needs(void)
 }
 
 /* return non-zero if the ref is hidden, otherwise 0 */
-static int mark_our_ref(const char *refname, const unsigned char *sha1)
+static int mark_our_ref(const char *refname, const struct object_id *oid)
 {
-	struct object *o = lookup_unknown_object(sha1);
+	struct object *o = lookup_unknown_object(oid->hash);
 
 	if (ref_is_hidden(refname)) {
 		o->flags |= HIDDEN_REF;
@@ -693,9 +693,9 @@ static int mark_our_ref(const char *refname, const unsigned char *sha1)
 	return 0;
 }
 
-static int check_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+static int check_ref(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
-	mark_our_ref(refname, sha1);
+	mark_our_ref(refname, oid);
 	return 0;
 }
 
@@ -709,15 +709,15 @@ static void format_symref_info(struct strbuf *buf, struct string_list *symref)
 		strbuf_addf(buf, " symref=%s:%s", item->string, (char *)item->util);
 }
 
-static int send_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+static int send_ref(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
 	static const char *capabilities = "multi_ack thin-pack side-band"
 		" side-band-64k ofs-delta shallow no-progress"
 		" include-tag multi_ack_detailed";
 	const char *refname_nons = strip_namespace(refname);
-	unsigned char peeled[20];
+	struct object_id peeled;
 
-	if (mark_our_ref(refname, sha1))
+	if (mark_our_ref(refname, oid))
 		return 0;
 
 	if (capabilities) {
@@ -725,7 +725,7 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
 
 		format_symref_info(&symref_info, cb_data);
 		packet_write(1, "%s %s%c%s%s%s%s agent=%s\n",
-			     sha1_to_hex(sha1), refname_nons,
+			     oid_to_hex(oid), refname_nons,
 			     0, capabilities,
 			     allow_tip_sha1_in_want ? " allow-tip-sha1-in-want" : "",
 			     stateless_rpc ? " no-done" : "",
@@ -733,24 +733,24 @@ static int send_ref(const char *refname, const unsigned char *sha1, int flag, vo
 			     git_user_agent_sanitized());
 		strbuf_release(&symref_info);
 	} else {
-		packet_write(1, "%s %s\n", sha1_to_hex(sha1), refname_nons);
+		packet_write(1, "%s %s\n", oid_to_hex(oid), refname_nons);
 	}
 	capabilities = NULL;
-	if (!peel_ref(refname, peeled))
-		packet_write(1, "%s %s^{}\n", sha1_to_hex(peeled), refname_nons);
+	if (!peel_ref(refname, peeled.hash))
+		packet_write(1, "%s %s^{}\n", oid_to_hex(&peeled), refname_nons);
 	return 0;
 }
 
-static int find_symref(const char *refname, const unsigned char *sha1, int flag,
-		       void *cb_data)
+static int find_symref(const char *refname, const struct object_id *oid,
+			   int flag, void *cb_data)
 {
 	const char *symref_target;
 	struct string_list_item *item;
-	unsigned char unused[20];
+	struct object_id unused;
 
 	if ((flag & REF_ISSYMREF) == 0)
 		return 0;
-	symref_target = resolve_ref_unsafe(refname, 0, unused, &flag);
+	symref_target = resolve_ref_unsafe(refname, 0, unused.hash, &flag);
 	if (!symref_target || (flag & REF_ISSYMREF) == 0)
 		die("'%s' is a symref but it is not?", refname);
 	item = string_list_append(cb_data, refname);
-- 
2.3.5

[PATCH v2 11/16] refs: convert for_each_rawref to struct object_id.

[brian m. carlson]

Convert callbacks to use struct object_id internally as well.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/branch.c       |  4 ++--
 builtin/describe.c     | 12 ++++++------
 builtin/for-each-ref.c |  4 ++--
 builtin/fsck.c         | 16 ++++++++--------
 refs.c                 | 10 +++++-----
 refs.h                 |  2 +-
 6 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/builtin/branch.c b/builtin/branch.c
index 258fe2f..ee3e909 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -328,7 +328,7 @@ static int match_patterns(const char **pattern, const char *refname)
 	return 0;
 }
 
-static int append_ref(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
+static int append_ref(const char *refname, const struct object_id *oid, int flags, void *cb_data)
 {
 	struct append_ref_cb *cb = (struct append_ref_cb *)(cb_data);
 	struct ref_list *ref_list = cb->ref_list;
@@ -365,7 +365,7 @@ static int append_ref(const char *refname, const unsigned char *sha1, int flags,
 
 	commit = NULL;
 	if (ref_list->verbose || ref_list->with_commit || merge_filter != NO_FILTER) {
-		commit = lookup_commit_reference_gently(sha1, 1);
+		commit = lookup_commit_reference_gently(oid->hash, 1);
 		if (!commit) {
 			cb->ret = error(_("branch '%s' does not point at a commit"), refname);
 			return 0;
diff --git a/builtin/describe.c b/builtin/describe.c
index e00a75b..a36c829 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -119,10 +119,10 @@ static void add_to_known_names(const char *path,
 	}
 }
 
-static int get_name(const char *path, const unsigned char *sha1, int flag, void *cb_data)
+static int get_name(const char *path, const struct object_id *oid, int flag, void *cb_data)
 {
 	int is_tag = starts_with(path, "refs/tags/");
-	unsigned char peeled[20];
+	struct object_id peeled;
 	int is_annotated, prio;
 
 	/* Reject anything outside refs/tags/ unless --all */
@@ -134,10 +134,10 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
 		return 0;
 
 	/* Is it annotated? */
-	if (!peel_ref(path, peeled)) {
-		is_annotated = !!hashcmp(sha1, peeled);
+	if (!peel_ref(path, peeled.hash)) {
+		is_annotated = !!oidcmp(oid, &peeled);
 	} else {
-		hashcpy(peeled, sha1);
+		oidcpy(&peeled, oid);
 		is_annotated = 0;
 	}
 
@@ -154,7 +154,7 @@ static int get_name(const char *path, const unsigned char *sha1, int flag, void
 	else
 		prio = 0;
 
-	add_to_known_names(all ? path + 5 : path + 10, peeled, prio, sha1);
+	add_to_known_names(all ? path + 5 : path + 10, peeled.hash, prio, oid->hash);
 	return 0;
 }
 
diff --git a/builtin/for-each-ref.c b/builtin/for-each-ref.c
index 83f9cf9..50c3fbe 100644
--- a/builtin/for-each-ref.c
+++ b/builtin/for-each-ref.c
@@ -840,7 +840,7 @@ struct grab_ref_cbdata {
  * A call-back given to for_each_ref().  Filter refs and keep them for
  * later object processing.
  */
-static int grab_single_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+static int grab_single_ref(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
 	struct grab_ref_cbdata *cb = cb_data;
 	struct refinfo *ref;
@@ -878,7 +878,7 @@ static int grab_single_ref(const char *refname, const unsigned char *sha1, int f
 	 */
 	ref = xcalloc(1, sizeof(*ref));
 	ref->refname = xstrdup(refname);
-	hashcpy(ref->objectname, sha1);
+	hashcpy(ref->objectname, oid->hash);
 	ref->flag = flag;
 
 	cnt = cb->grab_cnt;
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 4783896..85238a7 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -25,7 +25,7 @@ static int include_reflogs = 1;
 static int check_full = 1;
 static int check_strict;
 static int keep_cache_objects;
-static unsigned char head_sha1[20];
+static struct object_id head_oid;
 static const char *head_points_at;
 static int errors_found;
 static int write_lost_and_found;
@@ -482,13 +482,13 @@ static int fsck_handle_reflog(const char *logname, const unsigned char *sha1, in
 	return 0;
 }
 
-static int fsck_handle_ref(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+static int fsck_handle_ref(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
 	struct object *obj;
 
-	obj = parse_object(sha1);
+	obj = parse_object(oid->hash);
 	if (!obj) {
-		error("%s: invalid sha1 pointer %s", refname, sha1_to_hex(sha1));
+		error("%s: invalid sha1 pointer %s", refname, oid_to_hex(oid));
 		errors_found |= ERROR_REACHABLE;
 		/* We'll continue with the rest despite the error.. */
 		return 0;
@@ -504,8 +504,8 @@ static int fsck_handle_ref(const char *refname, const unsigned char *sha1, int f
 
 static void get_default_heads(void)
 {
-	if (head_points_at && !is_null_sha1(head_sha1))
-		fsck_handle_ref("HEAD", head_sha1, 0, NULL);
+	if (head_points_at && !is_null_oid(&head_oid))
+		fsck_handle_ref("HEAD", &head_oid, 0, NULL);
 	for_each_rawref(fsck_handle_ref, NULL);
 	if (include_reflogs)
 		for_each_reflog(fsck_handle_reflog, NULL);
@@ -556,7 +556,7 @@ static int fsck_head_link(void)
 	if (verbose)
 		fprintf(stderr, "Checking HEAD link\n");
 
-	head_points_at = resolve_ref_unsafe("HEAD", 0, head_sha1, &flag);
+	head_points_at = resolve_ref_unsafe("HEAD", 0, head_oid.hash, &flag);
 	if (!head_points_at)
 		return error("Invalid HEAD");
 	if (!strcmp(head_points_at, "HEAD"))
@@ -565,7 +565,7 @@ static int fsck_head_link(void)
 	else if (!starts_with(head_points_at, "refs/heads/"))
 		return error("HEAD points to something strange (%s)",
 			     head_points_at);
-	if (is_null_sha1(head_sha1)) {
+	if (is_null_oid(&head_oid)) {
 		if (null_is_error)
 			return error("HEAD: detached HEAD points at nothing");
 		fprintf(stderr, "notice: HEAD points to an unborn branch (%s)\n",
diff --git a/refs.c b/refs.c
index 19ce65a..185b40f 100644
--- a/refs.c
+++ b/refs.c
@@ -1838,17 +1838,17 @@ struct warn_if_dangling_data {
 	const char *msg_fmt;
 };
 
-static int warn_if_dangling_symref(const char *refname, const unsigned char *sha1,
+static int warn_if_dangling_symref(const char *refname, const struct object_id *oid,
 				   int flags, void *cb_data)
 {
 	struct warn_if_dangling_data *d = cb_data;
 	const char *resolves_to;
-	unsigned char junk[20];
+	struct object_id junk;
 
 	if (!(flags & REF_ISSYMREF))
 		return 0;
 
-	resolves_to = resolve_ref_unsafe(refname, 0, junk, NULL);
+	resolves_to = resolve_ref_unsafe(refname, 0, junk.hash, NULL);
 	if (!resolves_to
 	    || (d->refname
 		? strcmp(resolves_to, d->refname)
@@ -2125,9 +2125,9 @@ int for_each_glob_ref(each_ref_fn_oid fn, const char *pattern, void *cb_data)
 	return for_each_glob_ref_in(fn, pattern, NULL, cb_data);
 }
 
-int for_each_rawref(each_ref_fn fn, void *cb_data)
+int for_each_rawref(each_ref_fn_oid fn, void *cb_data)
 {
-	return do_for_each_ref(&ref_cache, "", fn, 0,
+	return do_for_each_ref_oid(&ref_cache, "", fn, 0,
 			       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
 }
 
diff --git a/refs.h b/refs.h
index 2b2a688..6df7d8a 100644
--- a/refs.h
+++ b/refs.h
@@ -113,7 +113,7 @@ static inline const char *has_glob_specials(const char *pattern)
 }
 
 /* can be used to learn about broken ref and symref */
-extern int for_each_rawref(each_ref_fn, void *);
+extern int for_each_rawref(each_ref_fn_oid, void *);
 
 extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname);
 extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct string_list *refnames);
-- 
2.3.5

[PATCH v2 12/16] refs: rename do_for_each_ref_oid to do_for_each_ref

[brian m. carlson]

do_for_each_ref was unused due to previous patches, so rename
do_for_each_ref_oid to do_for_each_ref.  Similarly, remove the unused fn
member from struct ref_entry in favor of renaming the fn_oid member.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 refs.c | 43 +++++++++++--------------------------------
 1 file changed, 11 insertions(+), 32 deletions(-)

diff --git a/refs.c b/refs.c
index 185b40f..9f687e8 100644
--- a/refs.c
+++ b/refs.c
@@ -693,8 +693,7 @@ struct ref_entry_cb {
 	const char *base;
 	int trim;
 	int flags;
-	each_ref_fn *fn;
-	each_ref_fn_oid *fn_oid;
+	each_ref_fn_oid *fn;
 	void *cb_data;
 };
 
@@ -718,13 +717,8 @@ static int do_one_ref(struct ref_entry *entry, void *cb_data)
 	/* Store the old value, in case this is a recursive call: */
 	old_current_ref = current_ref;
 	current_ref = entry;
-	if (data->fn_oid) {
-		retval = data->fn_oid(entry->name + data->trim, &entry->u.value.oid,
-				 entry->flag, data->cb_data);
-	} else {
-		retval = data->fn(entry->name + data->trim, entry->u.value.oid.hash,
-				 entry->flag, data->cb_data);
-	}
+	retval = data->fn(entry->name + data->trim, &entry->u.value.oid,
+			 entry->flag, data->cb_data);
 	current_ref = old_current_ref;
 	return retval;
 }
@@ -1949,28 +1943,13 @@ static int do_for_each_entry(struct ref_cache *refs, const char *base,
  * 0.
  */
 static int do_for_each_ref(struct ref_cache *refs, const char *base,
-			   each_ref_fn fn, int trim, int flags, void *cb_data)
-{
-	struct ref_entry_cb data;
-	data.base = base;
-	data.trim = trim;
-	data.flags = flags;
-	data.fn = fn;
-	data.fn_oid = NULL;
-	data.cb_data = cb_data;
-
-	return do_for_each_entry(refs, base, do_one_ref, &data);
-}
-
-static int do_for_each_ref_oid(struct ref_cache *refs, const char *base,
 			   each_ref_fn_oid fn, int trim, int flags, void *cb_data)
 {
 	struct ref_entry_cb data;
 	data.base = base;
 	data.trim = trim;
 	data.flags = flags;
-	data.fn = NULL;
-	data.fn_oid = fn;
+	data.fn = fn;
 	data.cb_data = cb_data;
 
 	if (ref_paranoia < 0)
@@ -2011,23 +1990,23 @@ int head_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data)
 
 int for_each_ref(each_ref_fn_oid fn, void *cb_data)
 {
-	return do_for_each_ref_oid(&ref_cache, "", fn, 0, 0, cb_data);
+	return do_for_each_ref(&ref_cache, "", fn, 0, 0, cb_data);
 }
 
 int for_each_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data)
 {
-	return do_for_each_ref_oid(get_ref_cache(submodule), "", fn, 0, 0, cb_data);
+	return do_for_each_ref(get_ref_cache(submodule), "", fn, 0, 0, cb_data);
 }
 
 int for_each_ref_in(const char *prefix, each_ref_fn_oid fn, void *cb_data)
 {
-	return do_for_each_ref_oid(&ref_cache, prefix, fn, strlen(prefix), 0, cb_data);
+	return do_for_each_ref(&ref_cache, prefix, fn, strlen(prefix), 0, cb_data);
 }
 
 int for_each_ref_in_submodule(const char *submodule, const char *prefix,
 		each_ref_fn_oid fn, void *cb_data)
 {
-	return do_for_each_ref_oid(get_ref_cache(submodule), prefix, fn, strlen(prefix), 0, cb_data);
+	return do_for_each_ref(get_ref_cache(submodule), prefix, fn, strlen(prefix), 0, cb_data);
 }
 
 int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
@@ -2062,7 +2041,7 @@ int for_each_remote_ref_submodule(const char *submodule, each_ref_fn_oid fn, voi
 
 int for_each_replace_ref(each_ref_fn_oid fn, void *cb_data)
 {
-	return do_for_each_ref_oid(&ref_cache, "refs/replace/", fn, 13, 0, cb_data);
+	return do_for_each_ref(&ref_cache, "refs/replace/", fn, 13, 0, cb_data);
 }
 
 int head_ref_namespaced(each_ref_fn_oid fn, void *cb_data)
@@ -2085,7 +2064,7 @@ int for_each_namespaced_ref(each_ref_fn_oid fn, void *cb_data)
 	struct strbuf buf = STRBUF_INIT;
 	int ret;
 	strbuf_addf(&buf, "%srefs/", get_git_namespace());
-	ret = do_for_each_ref_oid(&ref_cache, buf.buf, fn, 0, 0, cb_data);
+	ret = do_for_each_ref(&ref_cache, buf.buf, fn, 0, 0, cb_data);
 	strbuf_release(&buf);
 	return ret;
 }
@@ -2127,7 +2106,7 @@ int for_each_glob_ref(each_ref_fn_oid fn, const char *pattern, void *cb_data)
 
 int for_each_rawref(each_ref_fn_oid fn, void *cb_data)
 {
-	return do_for_each_ref_oid(&ref_cache, "", fn, 0,
+	return do_for_each_ref(&ref_cache, "", fn, 0,
 			       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
 }
 
-- 
2.3.5

[PATCH v2 13/16] refs: convert for_each_reflog to struct object_id

[brian m. carlson]

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/fsck.c   |  2 +-
 builtin/reflog.c |  4 ++--
 refs.c           | 10 +++++-----
 refs.h           |  2 +-
 revision.c       |  2 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 85238a7..6659f81 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -476,7 +476,7 @@ static int fsck_handle_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 	return 0;
 }
 
-static int fsck_handle_reflog(const char *logname, const unsigned char *sha1, int flag, void *cb_data)
+static int fsck_handle_reflog(const char *logname, const struct object_id *oid, int flag, void *cb_data)
 {
 	for_each_reflog_ent(logname, fsck_handle_reflog_ent, NULL);
 	return 0;
diff --git a/builtin/reflog.c b/builtin/reflog.c
index f3f9201..da890f0 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -379,14 +379,14 @@ static void reflog_expiry_cleanup(void *cb_data)
 	}
 }
 
-static int collect_reflog(const char *ref, const unsigned char *sha1, int unused, void *cb_data)
+static int collect_reflog(const char *ref, const struct object_id *oid, int unused, void *cb_data)
 {
 	struct collected_reflog *e;
 	struct collect_reflog_cb *cb = cb_data;
 	size_t namelen = strlen(ref);
 
 	e = xmalloc(sizeof(*e) + namelen + 1);
-	hashcpy(e->sha1, sha1);
+	hashcpy(e->sha1, oid->hash);
 	memcpy(e->reflog, ref, namelen + 1);
 	ALLOC_GROW(cb->e, cb->nr + 1, cb->alloc);
 	cb->e[cb->nr++] = e;
diff --git a/refs.c b/refs.c
index 9f687e8..38ecc2a 100644
--- a/refs.c
+++ b/refs.c
@@ -3484,7 +3484,7 @@ int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void *cb_dat
  * must be empty or end with '/'.  Name will be used as a scratch
  * space, but its contents will be restored before return.
  */
-static int do_for_each_reflog(struct strbuf *name, each_ref_fn fn, void *cb_data)
+static int do_for_each_reflog(struct strbuf *name, each_ref_fn_oid fn, void *cb_data)
 {
 	DIR *d = opendir(git_path("logs/%s", name->buf));
 	int retval = 0;
@@ -3509,11 +3509,11 @@ static int do_for_each_reflog(struct strbuf *name, each_ref_fn fn, void *cb_data
 				strbuf_addch(name, '/');
 				retval = do_for_each_reflog(name, fn, cb_data);
 			} else {
-				unsigned char sha1[20];
-				if (read_ref_full(name->buf, 0, sha1, NULL))
+				struct object_id oid;
+				if (read_ref_full(name->buf, 0, oid.hash, NULL))
 					retval = error("bad ref for %s", name->buf);
 				else
-					retval = fn(name->buf, sha1, 0, cb_data);
+					retval = fn(name->buf, &oid, 0, cb_data);
 			}
 			if (retval)
 				break;
@@ -3524,7 +3524,7 @@ static int do_for_each_reflog(struct strbuf *name, each_ref_fn fn, void *cb_data
 	return retval;
 }
 
-int for_each_reflog(each_ref_fn fn, void *cb_data)
+int for_each_reflog(each_ref_fn_oid fn, void *cb_data)
 {
 	int retval;
 	struct strbuf name;
diff --git a/refs.h b/refs.h
index 6df7d8a..abdfb00 100644
--- a/refs.h
+++ b/refs.h
@@ -222,7 +222,7 @@ int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void
  * Calls the specified function for each reflog file until it returns nonzero,
  * and returns the value
  */
-extern int for_each_reflog(each_ref_fn, void *);
+extern int for_each_reflog(each_ref_fn_oid, void *);
 
 #define REFNAME_ALLOW_ONELEVEL 1
 #define REFNAME_REFSPEC_PATTERN 2
diff --git a/revision.c b/revision.c
index 29af759..94eb94b 100644
--- a/revision.c
+++ b/revision.c
@@ -1298,7 +1298,7 @@ static int handle_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 	return 0;
 }
 
-static int handle_one_reflog(const char *path, const unsigned char *sha1, int flag, void *cb_data)
+static int handle_one_reflog(const char *path, const struct object_id *oid, int flag, void *cb_data)
 {
 	struct all_refs_cb *cb = cb_data;
 	cb->warned_bad_reflog = 0;
-- 
2.3.5

[PATCH v2 14/16] refs: rename each_ref_fn_oid to each_ref_fn

[brian m. carlson]

each_ref_fn is no longer used, so rename each_ref_fn_oid to each_ref_fn.
Update the documentation to note the change in function signature.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 Documentation/technical/api-ref-iteration.txt |  2 +-
 refs.c                                        | 48 +++++++++++++--------------
 refs.h                                        | 44 +++++++++++-------------
 revision.c                                    |  6 ++--
 4 files changed, 47 insertions(+), 53 deletions(-)

diff --git a/Documentation/technical/api-ref-iteration.txt b/Documentation/technical/api-ref-iteration.txt
index 02adfd4..37379d8 100644
--- a/Documentation/technical/api-ref-iteration.txt
+++ b/Documentation/technical/api-ref-iteration.txt
@@ -6,7 +6,7 @@ Iteration of refs is done by using an iterate function which will call a
 callback function for every ref. The callback function has this
 signature:
 
-	int handle_one_ref(const char *refname, const unsigned char *sha1,
+	int handle_one_ref(const char *refname, const struct object_id *oid,
 			   int flags, void *cb_data);
 
 There are different kinds of iterate functions which all take a
diff --git a/refs.c b/refs.c
index 38ecc2a..9e61b32 100644
--- a/refs.c
+++ b/refs.c
@@ -693,7 +693,7 @@ struct ref_entry_cb {
 	const char *base;
 	int trim;
 	int flags;
-	each_ref_fn_oid *fn;
+	each_ref_fn *fn;
 	void *cb_data;
 };
 
@@ -1669,7 +1669,7 @@ char *resolve_refdup(const char *ref, int resolve_flags, unsigned char *sha1, in
 /* The argument to filter_refs */
 struct ref_filter {
 	const char *pattern;
-	each_ref_fn_oid *fn;
+	each_ref_fn *fn;
 	void *cb_data;
 };
 
@@ -1943,7 +1943,7 @@ static int do_for_each_entry(struct ref_cache *refs, const char *base,
  * 0.
  */
 static int do_for_each_ref(struct ref_cache *refs, const char *base,
-			   each_ref_fn_oid fn, int trim, int flags, void *cb_data)
+			   each_ref_fn fn, int trim, int flags, void *cb_data)
 {
 	struct ref_entry_cb data;
 	data.base = base;
@@ -1960,7 +1960,7 @@ static int do_for_each_ref(struct ref_cache *refs, const char *base,
 	return do_for_each_entry(refs, base, do_one_ref, &data);
 }
 
-static int do_head_ref(const char *submodule, each_ref_fn_oid fn, void *cb_data)
+static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
 {
 	struct object_id oid;
 	int flag;
@@ -1978,73 +1978,73 @@ static int do_head_ref(const char *submodule, each_ref_fn_oid fn, void *cb_data)
 	return 0;
 }
 
-int head_ref(each_ref_fn_oid fn, void *cb_data)
+int head_ref(each_ref_fn fn, void *cb_data)
 {
 	return do_head_ref(NULL, fn, cb_data);
 }
 
-int head_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data)
+int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 {
 	return do_head_ref(submodule, fn, cb_data);
 }
 
-int for_each_ref(each_ref_fn_oid fn, void *cb_data)
+int for_each_ref(each_ref_fn fn, void *cb_data)
 {
 	return do_for_each_ref(&ref_cache, "", fn, 0, 0, cb_data);
 }
 
-int for_each_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data)
+int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 {
 	return do_for_each_ref(get_ref_cache(submodule), "", fn, 0, 0, cb_data);
 }
 
-int for_each_ref_in(const char *prefix, each_ref_fn_oid fn, void *cb_data)
+int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
 {
 	return do_for_each_ref(&ref_cache, prefix, fn, strlen(prefix), 0, cb_data);
 }
 
 int for_each_ref_in_submodule(const char *submodule, const char *prefix,
-		each_ref_fn_oid fn, void *cb_data)
+		each_ref_fn fn, void *cb_data)
 {
 	return do_for_each_ref(get_ref_cache(submodule), prefix, fn, strlen(prefix), 0, cb_data);
 }
 
-int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
+int for_each_tag_ref(each_ref_fn fn, void *cb_data)
 {
 	return for_each_ref_in("refs/tags/", fn, cb_data);
 }
 
-int for_each_tag_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data)
+int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 {
 	return for_each_ref_in_submodule(submodule, "refs/tags/", fn, cb_data);
 }
 
-int for_each_branch_ref(each_ref_fn_oid fn, void *cb_data)
+int for_each_branch_ref(each_ref_fn fn, void *cb_data)
 {
 	return for_each_ref_in("refs/heads/", fn, cb_data);
 }
 
-int for_each_branch_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data)
+int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 {
 	return for_each_ref_in_submodule(submodule, "refs/heads/", fn, cb_data);
 }
 
-int for_each_remote_ref(each_ref_fn_oid fn, void *cb_data)
+int for_each_remote_ref(each_ref_fn fn, void *cb_data)
 {
 	return for_each_ref_in("refs/remotes/", fn, cb_data);
 }
 
-int for_each_remote_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data)
+int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
 {
 	return for_each_ref_in_submodule(submodule, "refs/remotes/", fn, cb_data);
 }
 
-int for_each_replace_ref(each_ref_fn_oid fn, void *cb_data)
+int for_each_replace_ref(each_ref_fn fn, void *cb_data)
 {
 	return do_for_each_ref(&ref_cache, "refs/replace/", fn, 13, 0, cb_data);
 }
 
-int head_ref_namespaced(each_ref_fn_oid fn, void *cb_data)
+int head_ref_namespaced(each_ref_fn fn, void *cb_data)
 {
 	struct strbuf buf = STRBUF_INIT;
 	int ret = 0;
@@ -2059,7 +2059,7 @@ int head_ref_namespaced(each_ref_fn_oid fn, void *cb_data)
 	return ret;
 }
 
-int for_each_namespaced_ref(each_ref_fn_oid fn, void *cb_data)
+int for_each_namespaced_ref(each_ref_fn fn, void *cb_data)
 {
 	struct strbuf buf = STRBUF_INIT;
 	int ret;
@@ -2069,7 +2069,7 @@ int for_each_namespaced_ref(each_ref_fn_oid fn, void *cb_data)
 	return ret;
 }
 
-int for_each_glob_ref_in(each_ref_fn_oid fn, const char *pattern,
+int for_each_glob_ref_in(each_ref_fn fn, const char *pattern,
 	const char *prefix, void *cb_data)
 {
 	struct strbuf real_pattern = STRBUF_INIT;
@@ -2099,12 +2099,12 @@ int for_each_glob_ref_in(each_ref_fn_oid fn, const char *pattern,
 	return ret;
 }
 
-int for_each_glob_ref(each_ref_fn_oid fn, const char *pattern, void *cb_data)
+int for_each_glob_ref(each_ref_fn fn, const char *pattern, void *cb_data)
 {
 	return for_each_glob_ref_in(fn, pattern, NULL, cb_data);
 }
 
-int for_each_rawref(each_ref_fn_oid fn, void *cb_data)
+int for_each_rawref(each_ref_fn fn, void *cb_data)
 {
 	return do_for_each_ref(&ref_cache, "", fn, 0,
 			       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
@@ -3484,7 +3484,7 @@ int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void *cb_dat
  * must be empty or end with '/'.  Name will be used as a scratch
  * space, but its contents will be restored before return.
  */
-static int do_for_each_reflog(struct strbuf *name, each_ref_fn_oid fn, void *cb_data)
+static int do_for_each_reflog(struct strbuf *name, each_ref_fn fn, void *cb_data)
 {
 	DIR *d = opendir(git_path("logs/%s", name->buf));
 	int retval = 0;
@@ -3524,7 +3524,7 @@ static int do_for_each_reflog(struct strbuf *name, each_ref_fn_oid fn, void *cb_
 	return retval;
 }
 
-int for_each_reflog(each_ref_fn_oid fn, void *cb_data)
+int for_each_reflog(each_ref_fn fn, void *cb_data)
 {
 	int retval;
 	struct strbuf name;
diff --git a/refs.h b/refs.h
index abdfb00..0bd2fce 100644
--- a/refs.h
+++ b/refs.h
@@ -69,12 +69,6 @@ struct ref_transaction;
  * single callback invocation.
  */
 typedef int each_ref_fn(const char *refname,
-			const unsigned char *sha1, int flags, void *cb_data);
-
-/*
- * Like each_ref_fn, but passes the object ID using a struct.
- */
-typedef int each_ref_fn_oid(const char *refname,
 			const struct object_id *oid, int flags, void *cb_data);
 
 /*
@@ -86,26 +80,26 @@ typedef int each_ref_fn_oid(const char *refname,
  * modifies the reference also returns a nonzero value to immediately
  * stop the iteration.
  */
-extern int head_ref(each_ref_fn_oid, void *);
-extern int for_each_ref(each_ref_fn_oid, void *);
-extern int for_each_ref_in(const char *, each_ref_fn_oid, void *);
-extern int for_each_tag_ref(each_ref_fn_oid, void *);
-extern int for_each_branch_ref(each_ref_fn_oid, void *);
-extern int for_each_remote_ref(each_ref_fn_oid, void *);
-extern int for_each_replace_ref(each_ref_fn_oid, void *);
-extern int for_each_glob_ref(each_ref_fn_oid, const char *pattern, void *);
-extern int for_each_glob_ref_in(each_ref_fn_oid, const char *pattern, const char* prefix, void *);
+extern int head_ref(each_ref_fn, void *);
+extern int for_each_ref(each_ref_fn, void *);
+extern int for_each_ref_in(const char *, each_ref_fn, void *);
+extern int for_each_tag_ref(each_ref_fn, void *);
+extern int for_each_branch_ref(each_ref_fn, void *);
+extern int for_each_remote_ref(each_ref_fn, void *);
+extern int for_each_replace_ref(each_ref_fn, void *);
+extern int for_each_glob_ref(each_ref_fn, const char *pattern, void *);
+extern int for_each_glob_ref_in(each_ref_fn, const char *pattern, const char* prefix, void *);
 
-extern int head_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
-extern int for_each_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
+extern int head_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
+extern int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
 extern int for_each_ref_in_submodule(const char *submodule, const char *prefix,
-		each_ref_fn_oid fn, void *cb_data);
-extern int for_each_tag_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
-extern int for_each_branch_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
-extern int for_each_remote_ref_submodule(const char *submodule, each_ref_fn_oid fn, void *cb_data);
+		each_ref_fn fn, void *cb_data);
+extern int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
+extern int for_each_branch_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
+extern int for_each_remote_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data);
 
-extern int head_ref_namespaced(each_ref_fn_oid fn, void *cb_data);
-extern int for_each_namespaced_ref(each_ref_fn_oid fn, void *cb_data);
+extern int head_ref_namespaced(each_ref_fn fn, void *cb_data);
+extern int for_each_namespaced_ref(each_ref_fn fn, void *cb_data);
 
 static inline const char *has_glob_specials(const char *pattern)
 {
@@ -113,7 +107,7 @@ static inline const char *has_glob_specials(const char *pattern)
 }
 
 /* can be used to learn about broken ref and symref */
-extern int for_each_rawref(each_ref_fn_oid, void *);
+extern int for_each_rawref(each_ref_fn, void *);
 
 extern void warn_dangling_symref(FILE *fp, const char *msg_fmt, const char *refname);
 extern void warn_dangling_symrefs(FILE *fp, const char *msg_fmt, const struct string_list *refnames);
@@ -222,7 +216,7 @@ int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void
  * Calls the specified function for each reflog file until it returns nonzero,
  * and returns the value
  */
-extern int for_each_reflog(each_ref_fn_oid, void *);
+extern int for_each_reflog(each_ref_fn, void *);
 
 #define REFNAME_ALLOW_ONELEVEL 1
 #define REFNAME_REFSPEC_PATTERN 2
diff --git a/revision.c b/revision.c
index 94eb94b..6d92c0e 100644
--- a/revision.c
+++ b/revision.c
@@ -1264,7 +1264,7 @@ void add_ref_exclusion(struct string_list **ref_excludes_p, const char *exclude)
 }
 
 static void handle_refs(const char *submodule, struct rev_info *revs, unsigned flags,
-		int (*for_each)(const char *, each_ref_fn_oid, void *))
+		int (*for_each)(const char *, each_ref_fn, void *))
 {
 	struct all_refs_cb cb;
 	init_all_refs_cb(&cb, revs, flags);
@@ -2079,12 +2079,12 @@ void parse_revision_opt(struct rev_info *revs, struct parse_opt_ctx_t *ctx,
 	ctx->argc -= n;
 }
 
-static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn_oid fn, void *cb_data)
+static int for_each_bad_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)
 {
 	return for_each_ref_in_submodule(submodule, "refs/bisect/bad", fn, cb_data);
 }
 
-static int for_each_good_bisect_ref(const char *submodule, each_ref_fn_oid fn, void *cb_data)
+static int for_each_good_bisect_ref(const char *submodule, each_ref_fn fn, void *cb_data)
 {
 	return for_each_ref_in_submodule(submodule, "refs/bisect/good", fn, cb_data);
 }
-- 
2.3.5

[PATCH v2 15/16] Remove unneeded *_oid functions.

[brian m. carlson]

While these functions were needed during the intermediate steps of
converting for_each_ref and friends to struct object_id, there is no
longer any need to have these wrapper functions.  Update each of the
functions that the wrapper functions call and remove the _oid wrapper
functions themselves.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/rev-parse.c | 27 +++++++++++----------------
 builtin/show-ref.c  | 23 +++++++++--------------
 log-tree.c          | 19 +++++++------------
 reachable.c         | 13 ++++---------
 shallow.c           | 33 ++++++++++-----------------------
 5 files changed, 41 insertions(+), 74 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 4aa7a25..b623239 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -190,19 +190,14 @@ static int show_default(void)
 	return 0;
 }
 
-static int show_reference(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
+static int show_reference(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
 	if (ref_excluded(ref_excludes, refname))
 		return 0;
-	show_rev(NORMAL, sha1, refname);
+	show_rev(NORMAL, oid->hash, refname);
 	return 0;
 }
 
-static int show_reference_oid(const char *refname, const struct object_id *oid, int flag, void *cb_data)
-{
-	return show_reference(refname, oid->hash, flag, cb_data);
-}
-
 static int anti_reference(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
 	show_rev(REVERSED, oid->hash, refname);
@@ -657,7 +652,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!strcmp(arg, "--all")) {
-				for_each_ref(show_reference_oid, NULL);
+				for_each_ref(show_reference, NULL);
 				continue;
 			}
 			if (starts_with(arg, "--disambiguate=")) {
@@ -665,45 +660,45 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!strcmp(arg, "--bisect")) {
-				for_each_ref_in("refs/bisect/bad", show_reference_oid, NULL);
+				for_each_ref_in("refs/bisect/bad", show_reference, NULL);
 				for_each_ref_in("refs/bisect/good", anti_reference, NULL);
 				continue;
 			}
 			if (starts_with(arg, "--branches=")) {
-				for_each_glob_ref_in(show_reference_oid, arg + 11,
+				for_each_glob_ref_in(show_reference, arg + 11,
 					"refs/heads/", NULL);
 				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
 			if (!strcmp(arg, "--branches")) {
-				for_each_branch_ref(show_reference_oid, NULL);
+				for_each_branch_ref(show_reference, NULL);
 				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
 			if (starts_with(arg, "--tags=")) {
-				for_each_glob_ref_in(show_reference_oid, arg + 7,
+				for_each_glob_ref_in(show_reference, arg + 7,
 					"refs/tags/", NULL);
 				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
 			if (!strcmp(arg, "--tags")) {
-				for_each_tag_ref(show_reference_oid, NULL);
+				for_each_tag_ref(show_reference, NULL);
 				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
 			if (starts_with(arg, "--glob=")) {
-				for_each_glob_ref(show_reference_oid, arg + 7, NULL);
+				for_each_glob_ref(show_reference, arg + 7, NULL);
 				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
 			if (starts_with(arg, "--remotes=")) {
-				for_each_glob_ref_in(show_reference_oid, arg + 10,
+				for_each_glob_ref_in(show_reference, arg + 10,
 					"refs/remotes/", NULL);
 				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
 			if (!strcmp(arg, "--remotes")) {
-				for_each_remote_ref(show_reference_oid, NULL);
+				for_each_remote_ref(show_reference, NULL);
 				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 6731721..2d2267e 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -26,10 +26,10 @@ static void show_one(const char *refname, const unsigned char *sha1)
 		printf("%s %s\n", hex, refname);
 }
 
-static int show_ref(const char *refname, const unsigned char *sha1, int flag, void *cbdata)
+static int show_ref(const char *refname, const struct object_id *oid, int flag, void *cbdata)
 {
 	const char *hex;
-	unsigned char peeled[20];
+	struct object_id peeled;
 
 	if (show_head && !strcmp(refname, "HEAD"))
 		goto match;
@@ -69,30 +69,25 @@ match:
 	 * detect and return error if the repository is corrupt and
 	 * ref points at a nonexistent object.
 	 */
-	if (!has_sha1_file(sha1))
+	if (!has_sha1_file(oid->hash))
 		die("git show-ref: bad ref %s (%s)", refname,
-		    sha1_to_hex(sha1));
+		    oid_to_hex(oid));
 
 	if (quiet)
 		return 0;
 
-	show_one(refname, sha1);
+	show_one(refname, oid->hash);
 
 	if (!deref_tags)
 		return 0;
 
-	if (!peel_ref(refname, peeled)) {
-		hex = find_unique_abbrev(peeled, abbrev);
+	if (!peel_ref(refname, peeled.hash)) {
+		hex = find_unique_abbrev(peeled.hash, abbrev);
 		printf("%s %s^{}\n", hex, refname);
 	}
 	return 0;
 }
 
-static int show_ref_oid(const char *refname, const struct object_id *oid, int flag, void *cbdata)
-{
-	return show_ref(refname, oid->hash, flag, cbdata);
-}
-
 static int add_existing(const char *refname, const struct object_id *oid, int flag, void *cbdata)
 {
 	struct string_list *list = (struct string_list *)cbdata;
@@ -230,8 +225,8 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
 	}
 
 	if (show_head)
-		head_ref(show_ref_oid, NULL);
-	for_each_ref(show_ref_oid, NULL);
+		head_ref(show_ref, NULL);
+	for_each_ref(show_ref, NULL);
 	if (!found_match) {
 		if (verify && !quiet)
 			die("No match");
diff --git a/log-tree.c b/log-tree.c
index acbc859..64768f7 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -87,26 +87,26 @@ const struct name_decoration *get_name_decoration(const struct object *obj)
 	return lookup_decoration(&name_decoration, obj);
 }
 
-static int add_ref_decoration(const char *refname, const unsigned char *sha1, int flags, void *cb_data)
+static int add_ref_decoration(const char *refname, const struct object_id *oid, int flags, void *cb_data)
 {
 	struct object *obj;
 	enum decoration_type type = DECORATION_NONE;
 
 	if (starts_with(refname, "refs/replace/")) {
-		unsigned char original_sha1[20];
+		struct object_id original_oid;
 		if (!check_replace_refs)
 			return 0;
-		if (get_sha1_hex(refname + 13, original_sha1)) {
+		if (get_oid_hex(refname + 13, &original_oid)) {
 			warning("invalid replace ref %s", refname);
 			return 0;
 		}
-		obj = parse_object(original_sha1);
+		obj = parse_object(original_oid.hash);
 		if (obj)
 			add_name_decoration(DECORATION_GRAFTED, "replaced", obj);
 		return 0;
 	}
 
-	obj = parse_object(sha1);
+	obj = parse_object(oid->hash);
 	if (!obj)
 		return 0;
 
@@ -135,11 +135,6 @@ static int add_ref_decoration(const char *refname, const unsigned char *sha1, in
 	return 0;
 }
 
-static int add_ref_decoration_oid(const char *refname, const struct object_id *oid, int flags, void *cb_data)
-{
-	return add_ref_decoration(refname, oid->hash, flags, cb_data);
-}
-
 static int add_graft_decoration(const struct commit_graft *graft, void *cb_data)
 {
 	struct commit *commit = lookup_commit(graft->oid.hash);
@@ -154,8 +149,8 @@ void load_ref_decorations(int flags)
 	static int loaded;
 	if (!loaded) {
 		loaded = 1;
-		for_each_ref(add_ref_decoration_oid, &flags);
-		head_ref(add_ref_decoration_oid, &flags);
+		for_each_ref(add_ref_decoration, &flags);
+		head_ref(add_ref_decoration, &flags);
 		for_each_commit_graft(add_graft_decoration, NULL);
 	}
 }
diff --git a/reachable.c b/reachable.c
index b2ca0c3..8715eb9 100644
--- a/reachable.c
+++ b/reachable.c
@@ -22,9 +22,9 @@ static void update_progress(struct connectivity_progress *cp)
 		display_progress(cp->progress, cp->count);
 }
 
-static int add_one_ref(const char *path, const unsigned char *sha1, int flag, void *cb_data)
+static int add_one_ref(const char *path, const struct object_id *oid, int flag, void *cb_data)
 {
-	struct object *object = parse_object_or_die(sha1, path);
+	struct object *object = parse_object_or_die(oid->hash, path);
 	struct rev_info *revs = (struct rev_info *)cb_data;
 
 	add_pending_object(revs, object, "");
@@ -32,11 +32,6 @@ static int add_one_ref(const char *path, const unsigned char *sha1, int flag, vo
 	return 0;
 }
 
-static int add_one_ref_oid(const char *path, const struct object_id *oid, int flag, void *cb_data)
-{
-	return add_one_ref(path, oid->hash, flag, cb_data);
-}
-
 /*
  * The traversal will have already marked us as SEEN, so we
  * only need to handle any progress reporting here.
@@ -173,10 +168,10 @@ void mark_reachable_objects(struct rev_info *revs, int mark_reflog,
 	add_index_objects_to_pending(revs, 0);
 
 	/* Add all external refs */
-	for_each_ref(add_one_ref_oid, revs);
+	for_each_ref(add_one_ref, revs);
 
 	/* detached HEAD is not included in the list above */
-	head_ref(add_one_ref_oid, revs);
+	head_ref(add_one_ref, revs);
 
 	/* Add all reflog info */
 	if (mark_reflog)
diff --git a/shallow.c b/shallow.c
index 6511bbb..1c6de02 100644
--- a/shallow.c
+++ b/shallow.c
@@ -476,10 +476,10 @@ static void paint_down(struct paint_info *info, const unsigned char *sha1,
 }
 
 static int mark_uninteresting(const char *refname,
-			      const unsigned char *sha1,
+			      const struct object_id *oid,
 			      int flags, void *cb_data)
 {
-	struct commit *commit = lookup_commit_reference_gently(sha1, 1);
+	struct commit *commit = lookup_commit_reference_gently(oid->hash, 1);
 	if (!commit)
 		return 0;
 	commit->object.flags |= UNINTERESTING;
@@ -487,13 +487,6 @@ static int mark_uninteresting(const char *refname,
 	return 0;
 }
 
-static int mark_uninteresting_oid(const char *refname,
-				const struct object_id *oid,
-				int flags, void *cb_data)
-{
-	return mark_uninteresting(refname, oid->hash, flags, cb_data);
-}
-
 static void post_assign_shallow(struct shallow_info *info,
 				struct ref_bitmap *ref_bitmap,
 				int *ref_status);
@@ -549,8 +542,8 @@ void assign_shallow_commits_to_refs(struct shallow_info *info,
 	 * connect to old refs. If not (e.g. force ref updates) it'll
 	 * have to go down to the current shallow commits.
 	 */
-	head_ref(mark_uninteresting_oid, NULL);
-	for_each_ref(mark_uninteresting_oid, NULL);
+	head_ref(mark_uninteresting, NULL);
+	for_each_ref(mark_uninteresting, NULL);
 
 	/* Mark potential bottoms so we won't go out of bound */
 	for (i = 0; i < nr_shallow; i++) {
@@ -592,22 +585,16 @@ struct commit_array {
 };
 
 static int add_ref(const char *refname,
-		   const unsigned char *sha1, int flags, void *cb_data)
+		   const struct object_id *oid, int flags, void *cb_data)
 {
 	struct commit_array *ca = cb_data;
 	ALLOC_GROW(ca->commits, ca->nr + 1, ca->alloc);
-	ca->commits[ca->nr] = lookup_commit_reference_gently(sha1, 1);
+	ca->commits[ca->nr] = lookup_commit_reference_gently(oid->hash, 1);
 	if (ca->commits[ca->nr])
 		ca->nr++;
 	return 0;
 }
 
-static int add_ref_oid(const char *refname,
-		   const struct object_id *oid, int flags, void *cb_data)
-{
-	return add_ref(refname, oid->hash, flags, cb_data);
-}
-
 static void update_refstatus(int *ref_status, int nr, uint32_t *bitmap)
 {
 	int i;
@@ -654,8 +641,8 @@ static void post_assign_shallow(struct shallow_info *info,
 	info->nr_theirs = dst;
 
 	memset(&ca, 0, sizeof(ca));
-	head_ref(add_ref_oid, &ca);
-	for_each_ref(add_ref_oid, &ca);
+	head_ref(add_ref, &ca);
+	for_each_ref(add_ref, &ca);
 
 	/* Remove unreachable shallow commits from "ours" */
 	for (i = dst = 0; i < info->nr_ours; i++) {
@@ -688,8 +675,8 @@ int delayed_reachability_test(struct shallow_info *si, int c)
 		if (!si->commits) {
 			struct commit_array ca;
 			memset(&ca, 0, sizeof(ca));
-			head_ref(add_ref_oid, &ca);
-			for_each_ref(add_ref_oid, &ca);
+			head_ref(add_ref, &ca);
+			for_each_ref(add_ref, &ca);
 			si->commits = ca.commits;
 			si->nr_commits = ca.nr;
 		}
-- 
2.3.5

[PATCH v2 16/16] refs: convert struct ref_lock to struct object_id

[brian m. carlson]

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 refs.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/refs.c b/refs.c
index 9e61b32..6c04189 100644
--- a/refs.c
+++ b/refs.c
@@ -10,7 +10,7 @@ struct ref_lock {
 	char *ref_name;
 	char *orig_ref_name;
 	struct lock_file *lk;
-	unsigned char old_sha1[20];
+	struct object_id old_oid;
 	int lock_fd;
 };
 
@@ -2159,16 +2159,16 @@ static struct ref_lock *verify_lock(struct ref_lock *lock,
 {
 	if (read_ref_full(lock->ref_name,
 			  mustexist ? RESOLVE_REF_READING : 0,
-			  lock->old_sha1, NULL)) {
+			  lock->old_oid.hash, NULL)) {
 		int save_errno = errno;
 		error("Can't verify ref %s", lock->ref_name);
 		unlock_ref(lock);
 		errno = save_errno;
 		return NULL;
 	}
-	if (hashcmp(lock->old_sha1, old_sha1)) {
+	if (hashcmp(lock->old_oid.hash, old_sha1)) {
 		error("Ref %s is at %s but expected %s", lock->ref_name,
-			sha1_to_hex(lock->old_sha1), sha1_to_hex(old_sha1));
+			oid_to_hex(&lock->old_oid), sha1_to_hex(old_sha1));
 		unlock_ref(lock);
 		errno = EBUSY;
 		return NULL;
@@ -2313,7 +2313,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
 	}
 
 	refname = resolve_ref_unsafe(refname, resolve_flags,
-				     lock->old_sha1, &type);
+				     lock->old_oid.hash, &type);
 	if (!refname && errno == EISDIR) {
 		/* we are trying to lock foo but we used to
 		 * have foo/bar which now does not exist;
@@ -2327,7 +2327,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
 			goto error_return;
 		}
 		refname = resolve_ref_unsafe(orig_refname, resolve_flags,
-					     lock->old_sha1, &type);
+					     lock->old_oid.hash, &type);
 	}
 	if (type_p)
 	    *type_p = type;
@@ -2343,7 +2343,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname,
 	 * refname, nor a packed ref whose name is a proper prefix of
 	 * our refname.
 	 */
-	if (is_null_sha1(lock->old_sha1) &&
+	if (is_null_oid(&lock->old_oid) &&
 	     !is_refname_available(refname, skip, get_packed_refs(&ref_cache))) {
 		last_errno = ENOTDIR;
 		goto error_return;
@@ -2849,7 +2849,7 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms
 		error("unable to lock %s for update", newrefname);
 		goto rollback;
 	}
-	hashcpy(lock->old_sha1, orig_sha1);
+	hashcpy(lock->old_oid.hash, orig_sha1);
 	if (write_ref_sha1(lock, orig_sha1, logmsg)) {
 		error("unable to write current sha1 into %s", newrefname);
 		goto rollback;
@@ -3091,9 +3091,9 @@ static int write_ref_sha1(struct ref_lock *lock,
 		return -1;
 	}
 	clear_loose_ref_cache(&ref_cache);
-	if (log_ref_write(lock->ref_name, lock->old_sha1, sha1, logmsg) < 0 ||
+	if (log_ref_write(lock->ref_name, lock->old_oid.hash, sha1, logmsg) < 0 ||
 	    (strcmp(lock->ref_name, lock->orig_ref_name) &&
-	     log_ref_write(lock->orig_ref_name, lock->old_sha1, sha1, logmsg) < 0)) {
+	     log_ref_write(lock->orig_ref_name, lock->old_oid.hash, sha1, logmsg) < 0)) {
 		unlock_ref(lock);
 		return -1;
 	}
@@ -3117,7 +3117,7 @@ static int write_ref_sha1(struct ref_lock *lock,
 					      head_sha1, &head_flag);
 		if (head_ref && (head_flag & REF_ISSYMREF) &&
 		    !strcmp(head_ref, lock->ref_name))
-			log_ref_write("HEAD", lock->old_sha1, sha1, logmsg);
+			log_ref_write("HEAD", lock->old_oid.hash, sha1, logmsg);
 	}
 	if (commit_ref(lock)) {
 		error("Couldn't set %s", lock->ref_name);
@@ -3814,7 +3814,7 @@ int ref_transaction_commit(struct ref_transaction *transaction,
 						  (update->flags & REF_NODEREF));
 
 			if (!overwriting_symref
-			    && !hashcmp(update->lock->old_sha1, update->new_sha1)) {
+			    && !hashcmp(update->lock->old_oid.hash, update->new_sha1)) {
 				/*
 				 * The reference already has the desired
 				 * value, so we don't need to write it.
-- 
2.3.5

Re: [PATCH v2 01/16] refs: convert struct ref_entry to use struct object_id

[Stefan Beller]

On Wed, Apr 22, 2015 at 4:24 PM, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  refs.c | 44 ++++++++++++++++++++++----------------------
>  1 file changed, 22 insertions(+), 22 deletions(-)
>
> diff --git a/refs.c b/refs.c
> index 81a455b..522d15d 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -156,7 +156,7 @@ struct ref_value {
>          * null.  If REF_ISSYMREF, then this is the name of the object
>          * referred to by the last reference in the symlink chain.
>          */
> -       unsigned char sha1[20];
> +       struct object_id oid;
>
>         /*
>          * If REF_KNOWS_PEELED, then this field holds the peeled value
> @@ -164,7 +164,7 @@ struct ref_value {
>          * be peelable.  See the documentation for peel_ref() for an
>          * exact definition of "peelable".
>          */
> -       unsigned char peeled[20];
> +       struct object_id peeled;
>  };
>
>  struct ref_cache;
> @@ -346,8 +346,8 @@ static struct ref_entry *create_ref_entry(const char *refname,
>                 die("Reference has invalid format: '%s'", refname);
>         len = strlen(refname) + 1;
>         ref = xmalloc(sizeof(struct ref_entry) + len);
> -       hashcpy(ref->u.value.sha1, sha1);
> -       hashclr(ref->u.value.peeled);
> +       hashcpy(ref->u.value.oid.hash, sha1);
> +       oidclr(&ref->u.value.peeled);
>         memcpy(ref->name, refname, len);
>         ref->flag = flag;
>         return ref;
> @@ -621,7 +621,7 @@ static int is_dup_ref(const struct ref_entry *ref1, const struct ref_entry *ref2
>                 /* This is impossible by construction */
>                 die("Reference directory conflict: %s", ref1->name);
>
> -       if (hashcmp(ref1->u.value.sha1, ref2->u.value.sha1))
> +       if (oidcmp(&ref1->u.value.oid, &ref2->u.value.oid))
>                 die("Duplicated ref, and SHA1s don't match: %s", ref1->name);

So you're switching the code for a possible future
In an earlier series/cover letter you wrote

> The goal of this series to improve type-checking in the codebase and to
> make it easier to move to a different hash function if the project
> decides to do that.  This series does not convert all of the codebase,
> but only parts.  I've dropped some of the patches from earlier (which no
> longer apply) and added others.

Which yields the question if you also want to take care of the error message
(It may not be a SHA1 any more but some $HASHFUNCTION)?

That said I'll focus on the type checking part in this review
and not annotate the SHA1s I find any more. ;)
>
>         warning("Duplicated ref: %s", ref1->name);
> @@ -669,7 +669,7 @@ static int ref_resolves_to_object(struct ref_entry *entry)
>  {
>         if (entry->flag & REF_ISBROKEN)
>                 return 0;
> -       if (!has_sha1_file(entry->u.value.sha1)) {
> +       if (!has_sha1_file(entry->u.value.oid.hash)) {
>                 error("%s does not point to a valid object!", entry->name);
>                 return 0;
>         }
> @@ -717,7 +717,7 @@ static int do_one_ref(struct ref_entry *entry, void *cb_data)
>         /* Store the old value, in case this is a recursive call: */
>         old_current_ref = current_ref;
>         current_ref = entry;
> -       retval = data->fn(entry->name + data->trim, entry->u.value.sha1,
> +       retval = data->fn(entry->name + data->trim, entry->u.value.oid.hash,
>                           entry->flag, data->cb_data);
>         current_ref = old_current_ref;
>         return retval;
> @@ -1193,7 +1193,7 @@ static void read_packed_refs(FILE *f, struct ref_dir *dir)
>                     line.len == PEELED_LINE_LENGTH &&
>                     line.buf[PEELED_LINE_LENGTH - 1] == '\n' &&
>                     !get_sha1_hex(line.buf + 1, sha1)) {
> -                       hashcpy(last->u.value.peeled, sha1);
> +                       hashcpy(last->u.value.peeled.hash, sha1);
>                         /*
>                          * Regardless of what the file header said,
>                          * we definitely know the value of *this*
> @@ -1374,7 +1374,7 @@ static int resolve_gitlink_packed_ref(struct ref_cache *refs,
>         if (ref == NULL)
>                 return -1;
>
> -       hashcpy(sha1, ref->u.value.sha1);
> +       hashcpy(sha1, ref->u.value.oid.hash);
>         return 0;
>  }
>
> @@ -1461,7 +1461,7 @@ static int resolve_missing_loose_ref(const char *refname,
>          */
>         entry = get_packed_ref(refname);
>         if (entry) {
> -               hashcpy(sha1, entry->u.value.sha1);
> +               hashcpy(sha1, entry->u.value.oid.hash);
>                 if (flags)
>                         *flags |= REF_ISPACKED;
>                 return 0;
> @@ -1771,9 +1771,9 @@ static enum peel_status peel_entry(struct ref_entry *entry, int repeel)
>         if (entry->flag & REF_KNOWS_PEELED) {
>                 if (repeel) {
>                         entry->flag &= ~REF_KNOWS_PEELED;
> -                       hashclr(entry->u.value.peeled);
> +                       oidclr(&entry->u.value.peeled);
>                 } else {
> -                       return is_null_sha1(entry->u.value.peeled) ?
> +                       return is_null_oid(&entry->u.value.peeled) ?
>                                 PEEL_NON_TAG : PEEL_PEELED;
>                 }
>         }
> @@ -1782,7 +1782,7 @@ static enum peel_status peel_entry(struct ref_entry *entry, int repeel)
>         if (entry->flag & REF_ISSYMREF)
>                 return PEEL_IS_SYMREF;
>
> -       status = peel_object(entry->u.value.sha1, entry->u.value.peeled);
> +       status = peel_object(entry->u.value.oid.hash, entry->u.value.peeled.hash);
>         if (status == PEEL_PEELED || status == PEEL_NON_TAG)
>                 entry->flag |= REF_KNOWS_PEELED;
>         return status;
> @@ -1797,7 +1797,7 @@ int peel_ref(const char *refname, unsigned char *sha1)
>                             || !strcmp(current_ref->name, refname))) {
>                 if (peel_entry(current_ref, 0))
>                         return -1;
> -               hashcpy(sha1, current_ref->u.value.peeled);
> +               hashcpy(sha1, current_ref->u.value.peeled.hash);
>                 return 0;
>         }
>
> @@ -1817,7 +1817,7 @@ int peel_ref(const char *refname, unsigned char *sha1)
>                 if (r) {
>                         if (peel_entry(r, 0))
>                                 return -1;
> -                       hashcpy(sha1, r->u.value.peeled);
> +                       hashcpy(sha1, r->u.value.peeled.hash);
>                         return 0;
>                 }
>         }
> @@ -2422,9 +2422,9 @@ static int write_packed_entry_fn(struct ref_entry *entry, void *cb_data)
>         if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)
>                 error("internal error: %s is not a valid packed reference!",
>                       entry->name);
> -       write_packed_entry(cb_data, entry->name, entry->u.value.sha1,
> +       write_packed_entry(cb_data, entry->name, entry->u.value.oid.hash,
>                            peel_status == PEEL_PEELED ?
> -                          entry->u.value.peeled : NULL);
> +                          entry->u.value.peeled.hash : NULL);
>         return 0;
>  }
>
> @@ -2531,24 +2531,24 @@ static int pack_if_possible_fn(struct ref_entry *entry, void *cb_data)
>         peel_status = peel_entry(entry, 1);
>         if (peel_status != PEEL_PEELED && peel_status != PEEL_NON_TAG)
>                 die("internal error peeling reference %s (%s)",
> -                   entry->name, sha1_to_hex(entry->u.value.sha1));
> +                   entry->name, oid_to_hex(&entry->u.value.oid));
>         packed_entry = find_ref(cb->packed_refs, entry->name);
>         if (packed_entry) {
>                 /* Overwrite existing packed entry with info from loose entry */
>                 packed_entry->flag = REF_ISPACKED | REF_KNOWS_PEELED;
> -               hashcpy(packed_entry->u.value.sha1, entry->u.value.sha1);
> +               oidcpy(&packed_entry->u.value.oid, &entry->u.value.oid);
>         } else {
> -               packed_entry = create_ref_entry(entry->name, entry->u.value.sha1,
> +               packed_entry = create_ref_entry(entry->name, entry->u.value.oid.hash,
>                                                 REF_ISPACKED | REF_KNOWS_PEELED, 0);
>                 add_ref(cb->packed_refs, packed_entry);
>         }
> -       hashcpy(packed_entry->u.value.peeled, entry->u.value.peeled);
> +       oidcpy(&packed_entry->u.value.peeled, &entry->u.value.peeled);
>
>         /* Schedule the loose reference for pruning if requested. */
>         if ((cb->flags & PACK_REFS_PRUNE)) {
>                 int namelen = strlen(entry->name) + 1;
>                 struct ref_to_prune *n = xcalloc(1, sizeof(*n) + namelen);
> -               hashcpy(n->sha1, entry->u.value.sha1);
> +               hashcpy(n->sha1, entry->u.value.oid.hash);
>                 strcpy(n->name, entry->name);
>                 n->next = cb->ref_to_prune;
>                 cb->ref_to_prune = n;
> --
> 2.3.5

The patch is
Reviewed-by: Stefan Beller <sbeller@google.com>

>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 02/16] refs: convert for_each_tag_ref to struct object_id

[Stefan Beller]

On Wed, Apr 22, 2015 at 4:24 PM, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
> To allow piecemeal conversion of the for_each_*_ref functions, introduce
> an additional typedef for a callback function that takes struct
> object_id * instead of unsigned char *.  Provide an extra field in
> struct ref_entry_cb for this callback and ensure at most one is set at a
> time.  Temporarily suffix these new entries with _oid to distinguish
> them.  Convert for_each_tag_ref and its callers to use the new _oid
> functions, introducing temporary wrapper functions to avoid type
> mismatches.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>

I am currently running this patch series via
git rebase -i origin/next --exec=make --exec="make test"
through the compilation and test suite one by one.
(My current view of origin/next is (c8da2d582, Sync with 2.4.0-rc3)
and this commit fails in t5312-prune-corruption.sh test 3 5 and 8

> ---
>  builtin/pack-objects.c |  4 ++--
>  builtin/rev-parse.c    |  7 ++++++-
>  builtin/tag.c          |  8 ++++----
>  refs.c                 | 34 ++++++++++++++++++++++++++++++----
>  refs.h                 | 10 +++++++++-
>  5 files changed, 51 insertions(+), 12 deletions(-)
>
> diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
> index c067107..0c69b0c 100644
> --- a/builtin/pack-objects.c
> +++ b/builtin/pack-objects.c
> @@ -540,11 +540,11 @@ static enum write_one_status write_one(struct sha1file *f,
>         return WRITE_ONE_WRITTEN;
>  }
>
> -static int mark_tagged(const char *path, const unsigned char *sha1, int flag,
> +static int mark_tagged(const char *path, const struct object_id *oid, int flag,
>                        void *cb_data)
>  {
>         unsigned char peeled[20];
> -       struct object_entry *entry = packlist_find(&to_pack, sha1, NULL);
> +       struct object_entry *entry = packlist_find(&to_pack, oid->hash, NULL);
>
>         if (entry)
>                 entry->tagged = 1;
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index 4d10dd9..7b70650 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -198,6 +198,11 @@ static int show_reference(const char *refname, const unsigned char *sha1, int fl
>         return 0;
>  }
>
> +static int show_reference_oid(const char *refname, const struct object_id *oid, int flag, void *cb_data)
> +{
> +       return show_reference(refname, oid->hash, flag, cb_data);
> +}
> +
>  static int anti_reference(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
>  {
>         show_rev(REVERSED, sha1, refname);
> @@ -682,7 +687,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>                                 continue;
>                         }
>                         if (!strcmp(arg, "--tags")) {
> -                               for_each_tag_ref(show_reference, NULL);
> +                               for_each_tag_ref(show_reference_oid, NULL);
>                                 clear_ref_exclusion(&ref_excludes);
>                                 continue;
>                         }
> diff --git a/builtin/tag.c b/builtin/tag.c
> index 6f07ac6..61399b7 100644
> --- a/builtin/tag.c
> +++ b/builtin/tag.c
> @@ -215,7 +215,7 @@ free_return:
>         free(buf);
>  }
>
> -static int show_reference(const char *refname, const unsigned char *sha1,
> +static int show_reference(const char *refname, const struct object_id *oid,
>                           int flag, void *cb_data)
>  {
>         struct tag_filter *filter = cb_data;
> @@ -224,14 +224,14 @@ static int show_reference(const char *refname, const unsigned char *sha1,
>                 if (filter->with_commit) {
>                         struct commit *commit;
>
> -                       commit = lookup_commit_reference_gently(sha1, 1);
> +                       commit = lookup_commit_reference_gently(oid->hash, 1);
>                         if (!commit)
>                                 return 0;
>                         if (!contains(commit, filter->with_commit))
>                                 return 0;
>                 }
>
> -               if (points_at.nr && !match_points_at(refname, sha1))
> +               if (points_at.nr && !match_points_at(refname, oid->hash))
>                         return 0;
>
>                 if (!filter->lines) {
> @@ -242,7 +242,7 @@ static int show_reference(const char *refname, const unsigned char *sha1,
>                         return 0;
>                 }
>                 printf("%-15s ", refname);
> -               show_tag_lines(sha1, filter->lines);
> +               show_tag_lines(oid->hash, filter->lines);
>                 putchar('\n');
>         }
>
> diff --git a/refs.c b/refs.c
> index 522d15d..95863f2 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -694,6 +694,7 @@ struct ref_entry_cb {
>         int trim;
>         int flags;
>         each_ref_fn *fn;
> +       each_ref_fn_oid *fn_oid;
>         void *cb_data;
>  };
>
> @@ -717,8 +718,13 @@ static int do_one_ref(struct ref_entry *entry, void *cb_data)
>         /* Store the old value, in case this is a recursive call: */
>         old_current_ref = current_ref;
>         current_ref = entry;
> -       retval = data->fn(entry->name + data->trim, entry->u.value.oid.hash,
> -                         entry->flag, data->cb_data);
> +       if (data->fn_oid) {
> +               retval = data->fn_oid(entry->name + data->trim, &entry->u.value.oid,
> +                                entry->flag, data->cb_data);
> +       } else {
> +               retval = data->fn(entry->name + data->trim, entry->u.value.oid.hash,
> +                                entry->flag, data->cb_data);
> +       }
>         current_ref = old_current_ref;
>         return retval;
>  }
> @@ -1950,6 +1956,21 @@ static int do_for_each_ref(struct ref_cache *refs, const char *base,
>         data.trim = trim;
>         data.flags = flags;
>         data.fn = fn;
> +       data.fn_oid = NULL;
> +       data.cb_data = cb_data;
> +
> +       return do_for_each_entry(refs, base, do_one_ref, &data);
> +}
> +
> +static int do_for_each_ref_oid(struct ref_cache *refs, const char *base,
> +                          each_ref_fn_oid fn, int trim, int flags, void *cb_data)
> +{
> +       struct ref_entry_cb data;
> +       data.base = base;
> +       data.trim = trim;
> +       data.flags = flags;
> +       data.fn = NULL;
> +       data.fn_oid = fn;
>         data.cb_data = cb_data;
>
>         if (ref_paranoia < 0)
> @@ -1998,6 +2019,11 @@ int for_each_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
>         return do_for_each_ref(get_ref_cache(submodule), "", fn, 0, 0, cb_data);
>  }
>
> +static int for_each_ref_in_oid(const char *prefix, each_ref_fn_oid fn, void *cb_data)
> +{
> +       return do_for_each_ref_oid(&ref_cache, prefix, fn, strlen(prefix), 0, cb_data);
> +}
> +
>  int for_each_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
>  {
>         return do_for_each_ref(&ref_cache, prefix, fn, strlen(prefix), 0, cb_data);
> @@ -2009,9 +2035,9 @@ int for_each_ref_in_submodule(const char *submodule, const char *prefix,
>         return do_for_each_ref(get_ref_cache(submodule), prefix, fn, strlen(prefix), 0, cb_data);
>  }
>
> -int for_each_tag_ref(each_ref_fn fn, void *cb_data)
> +int for_each_tag_ref(each_ref_fn_oid fn, void *cb_data)
>  {
> -       return for_each_ref_in("refs/tags/", fn, cb_data);
> +       return for_each_ref_in_oid("refs/tags/", fn, cb_data);
>  }
>
>  int for_each_tag_ref_submodule(const char *submodule, each_ref_fn fn, void *cb_data)
> diff --git a/refs.h b/refs.h
> index 6d7d9b4..b83529b 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -1,6 +1,8 @@
>  #ifndef REFS_H
>  #define REFS_H
>
> +#include "cache.h"
> +
>  /*
>   * A ref_transaction represents a collection of ref updates
>   * that should succeed or fail together.
> @@ -70,6 +72,12 @@ typedef int each_ref_fn(const char *refname,
>                         const unsigned char *sha1, int flags, void *cb_data);
>
>  /*
> + * Like each_ref_fn, but passes the object ID using a struct.
> + */
> +typedef int each_ref_fn_oid(const char *refname,
> +                       const struct object_id *oid, int flags, void *cb_data);
> +
> +/*
>   * The following functions invoke the specified callback function for
>   * each reference indicated.  If the function ever returns a nonzero
>   * value, stop the iteration and return that value.  Please note that
> @@ -81,7 +89,7 @@ typedef int each_ref_fn(const char *refname,
>  extern int head_ref(each_ref_fn, void *);
>  extern int for_each_ref(each_ref_fn, void *);
>  extern int for_each_ref_in(const char *, each_ref_fn, void *);
> -extern int for_each_tag_ref(each_ref_fn, void *);
> +extern int for_each_tag_ref(each_ref_fn_oid, void *);
>  extern int for_each_branch_ref(each_ref_fn, void *);
>  extern int for_each_remote_ref(each_ref_fn, void *);
>  extern int for_each_replace_ref(each_ref_fn, void *);
> --
> 2.3.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 02/16] refs: convert for_each_tag_ref to struct object_id

[Jeff King]

On Thu, Apr 23, 2015 at 11:13:32AM -0700, Stefan Beller wrote:

> On Wed, Apr 22, 2015 at 4:24 PM, brian m. carlson
> <sandals@crustytoothpaste.net> wrote:
> > To allow piecemeal conversion of the for_each_*_ref functions, introduce
> > an additional typedef for a callback function that takes struct
> > object_id * instead of unsigned char *.  Provide an extra field in
> > struct ref_entry_cb for this callback and ensure at most one is set at a
> > time.  Temporarily suffix these new entries with _oid to distinguish
> > them.  Convert for_each_tag_ref and its callers to use the new _oid
> > functions, introducing temporary wrapper functions to avoid type
> > mismatches.
> >
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> 
> I am currently running this patch series via
> git rebase -i origin/next --exec=make --exec="make test"
> through the compilation and test suite one by one.
> (My current view of origin/next is (c8da2d582, Sync with 2.4.0-rc3)
> and this commit fails in t5312-prune-corruption.sh test 3 5 and 8

It's because of this hunk:

> > @@ -1950,6 +1956,21 @@ static int do_for_each_ref(struct ref_cache *refs, const char *base,
> >         data.trim = trim;
> >         data.flags = flags;
> >         data.fn = fn;
> > +       data.fn_oid = NULL;
> > +       data.cb_data = cb_data;
> > +
> > +       return do_for_each_entry(refs, base, do_one_ref, &data);
> > +}
> > +
> > +static int do_for_each_ref_oid(struct ref_cache *refs, const char *base,
> > +                          each_ref_fn_oid fn, int trim, int flags, void *cb_data)
> > +{
> > +       struct ref_entry_cb data;
> > +       data.base = base;
> > +       data.trim = trim;
> > +       data.flags = flags;
> > +       data.fn = NULL;
> > +       data.fn_oid = fn;
> >         data.cb_data = cb_data;
> >
> >         if (ref_paranoia < 0)

The ref_paranoia code gets pushed down into do_for_each_ref_oid, but it
needs called in both do_for_each_ref variants. This is probably an
artifact of rebasing the patches (the ref_paranoia stuff was added
recently).

I think it would make sense to pull the setup of the data struct into a
shared function rather than duplicate it. But we want to avoid having to
update do_for_each_ref callsites, so we'll have to provide a wrapper.

Like this:

diff --git a/refs.c b/refs.c
index 95863f2..ad39d74 100644
--- a/refs.c
+++ b/refs.c
@@ -1948,29 +1948,16 @@ static int do_for_each_entry(struct ref_cache *refs, const char *base,
  * value, stop the iteration and return that value; otherwise, return
  * 0.
  */
-static int do_for_each_ref(struct ref_cache *refs, const char *base,
-			   each_ref_fn fn, int trim, int flags, void *cb_data)
+static int do_for_each_ref_generic(struct ref_cache *refs, const char *base,
+				   each_ref_fn fn, each_ref_fn_oid fn_oid,
+				   int trim, int flags, void *cb_data)
 {
 	struct ref_entry_cb data;
 	data.base = base;
 	data.trim = trim;
 	data.flags = flags;
 	data.fn = fn;
-	data.fn_oid = NULL;
-	data.cb_data = cb_data;
-
-	return do_for_each_entry(refs, base, do_one_ref, &data);
-}
-
-static int do_for_each_ref_oid(struct ref_cache *refs, const char *base,
-			   each_ref_fn_oid fn, int trim, int flags, void *cb_data)
-{
-	struct ref_entry_cb data;
-	data.base = base;
-	data.trim = trim;
-	data.flags = flags;
-	data.fn = NULL;
-	data.fn_oid = fn;
+	data.fn_oid = fn_oid;
 	data.cb_data = cb_data;
 
 	if (ref_paranoia < 0)
@@ -1981,6 +1968,18 @@ static int do_for_each_ref_oid(struct ref_cache *refs, const char *base,
 	return do_for_each_entry(refs, base, do_one_ref, &data);
 }
 
+static int do_for_each_ref(struct ref_cache *refs, const char *base,
+			   each_ref_fn fn, int trim, int flags, void *cb_data)
+{
+	return do_for_each_ref_generic(refs, base, fn, NULL, trim, flags, cb_data);
+}
+
+static int do_for_each_ref_oid(struct ref_cache *refs, const char *base,
+			       each_ref_fn_oid fn, int trim, int flags, void *cb_data)
+{
+	return do_for_each_ref_generic(refs, base, NULL, fn, trim, flags, cb_data);
+}
+
 static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
 {
 	unsigned char sha1[20];

You can even dispense with the _oid variant wrapper, and just call into
the generic version directly from the new callsites.

-Peff

Re: [PATCH v2 01/16] refs: convert struct ref_entry to use struct object_id

[brian m. carlson]

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

On Wed, Apr 22, 2015 at 05:52:09PM -0700, Stefan Beller wrote:
> So you're switching the code for a possible future
> In an earlier series/cover letter you wrote
> 
> > The goal of this series to improve type-checking in the codebase and to
> > make it easier to move to a different hash function if the project
> > decides to do that.  This series does not convert all of the codebase,
> > but only parts.  I've dropped some of the patches from earlier (which no
> > longer apply) and added others.
> 
> Which yields the question if you also want to take care of the error message
> (It may not be a SHA1 any more but some $HASHFUNCTION)?

This is true.  However, I'll clean those up with a future patch series
when we get to that point.  I'll need to pass through the documentation
as well to make it accurate and consistent, and I'll want to discuss the
words we want to use before I make those changes.

> That said I'll focus on the type checking part in this review
> and not annotate the SHA1s I find any more. ;)

Please do comment on any hardcoded 20s or 40s (or quantities based off
those), as I do want to fix those up.  I want to fix up any hardcoded
assumptions we may have about the hash that don't involve text or
documentation at this point, if only for maintainability reasons.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 02/16] refs: convert for_each_tag_ref to struct object_id

[brian m. carlson]

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

On Thu, Apr 23, 2015 at 03:27:23PM -0400, Jeff King wrote:
> +static int do_for_each_ref(struct ref_cache *refs, const char *base,
> +			   each_ref_fn fn, int trim, int flags, void *cb_data)
> +{
> +	return do_for_each_ref_generic(refs, base, fn, NULL, trim, flags, cb_data);
> +}
> +
> +static int do_for_each_ref_oid(struct ref_cache *refs, const char *base,
> +			       each_ref_fn_oid fn, int trim, int flags, void *cb_data)
> +{
> +	return do_for_each_ref_generic(refs, base, NULL, fn, trim, flags, cb_data);
> +}
> +
>  static int do_head_ref(const char *submodule, each_ref_fn fn, void *cb_data)
>  {
>  	unsigned char sha1[20];
> 
> You can even dispense with the _oid variant wrapper, and just call into
> the generic version directly from the new callsites.

Sounds like a good idea.  I'll reroll with that change probably sometime
this weekend.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2 00/16] Convert parts of refs.c to struct object_id

[Michael Haggerty]

On 04/23/2015 01:24 AM, brian m. carlson wrote:
> This is a conversion of parts of refs.c to use struct object_id.
> 
> refs.c, and the for_each_ref series of functions explicitly, is the
> source for many instances of object IDs in the codebase.  Therefore, it
> makes sense to convert this series of functions to provide a basis for
> further conversions.
> 
> This series is essentially just for_each_ref and friends, the callbacks,
> and callers.  Other parts of refs.c will be converted in a later series,
> so as to keep the number of patches to a reasonable size.
> 
> There should be no functional change from this patch series.

I wanted to review your patches, but wasn't really sure how to go about
it in a way that would make me confident in the result. In a way these
refactoring patch series are easier to implement than to review.

...so that's what I did. I reimplemented your changes "from scratch" [1]
and then checked how my result differed from yours. My conclusion is
that the final result of your patches looks good, though there are some
other changes in the neighborhood that could sensibly be added.


However, I reached the destination via a different route and I thought
I'd describe it in case you are interested, and as the technique might
be useful for future "object_id" refactorings. My patch series is
available on my GitHub account at

    https://github.com/mhagger/git branch oid-refs-adapter

My starting point was to change each_ref_fn to take a "const object_id
*" parameter instead of "const unsigned char *". This change requires
all call sites of all of the for_each_ref functions to be modified,
because they currently pass callback functions that match the old signature.

So I kept the old typedef (the one that takes "const unsigned char *")
but renamed it to each_ref_sha1_fn. And I added an adapter that allows
such functions to be wrapped then passed to the new for_each_ref
functions. It looks like this:

    typedef int each_ref_sha1_fn(const char *refname,
    			         const unsigned char *sha1, int flags, void *cb_data);

    struct each_ref_fn_sha1_adapter {
	    each_ref_sha1_fn *original_fn;
	    void *original_cb_data;
    };

    extern int each_ref_fn_adapter(const char *refname,
			           const struct object_id *oid, int flags, void *cb_data);

Each callsite has to be changed, but the changes are quite
straightforward. At a callsite that would have called

    for_each_ref(my_function, &my_data)

you wrap my_function and my_data in an each_ref_fn_sha1_adapter and call
for_each_ref using each_ref_fn_adapter as the callback:

    struct each_ref_fn_sha1_adapter wrapped_my_function =
            {my_function, &my_data};

    for_each_ref(each_ref_fn_adapter, &wrapped_my_function);

The function each_ref_fn_adapter extracts the SHA-1 out of the oid and
calls my_function, passing it &my_data as extra data.

This patch is thus giant but very straightforward.

After that, there is one patch for each callsite, rewriting it to use
for_each_ref natively (which usually entails modifying my_function to
take an object_id parameter then undoing the wrapper). These patches
involve a little bit of thought, but not too much. And the results are
very bisectable because each patch makes a single small change. I also
suspect it might be easier to rebase and/or merge my patch series, for
the same reason.

The end result was very similar to yours, so I am confident that the net
result of your patch series is correct. But the remaining differences in
the end results are also interesting. I made a few more changes in the
neighborhood of the patches, not to mention a few formatting
improvements in code that I touched. If you compare the tip of my
branch, above, to the tip of yours (I uploaded that to my repo too, as
branch "bc-oid-refs"), it may give you some ideas for other code that
can be changed to object_id.

Yours,
Michael

[1] Obviously I glanced at your patches while I was working to make sure
that I was headed in the same direction as you, and to minimize
gratuitous differences between our versions.

-- 
Michael Haggerty
mhagger@alum.mit.edu

Re: [PATCH v2 00/16] Convert parts of refs.c to struct object_id

[brian m. carlson]

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

On Sun, Apr 26, 2015 at 10:26:22PM +0200, Michael Haggerty wrote:
> After that, there is one patch for each callsite, rewriting it to use
> for_each_ref natively (which usually entails modifying my_function to
> take an object_id parameter then undoing the wrapper). These patches
> involve a little bit of thought, but not too much. And the results are
> very bisectable because each patch makes a single small change. I also
> suspect it might be easier to rebase and/or merge my patch series, for
> the same reason.
> 
> The end result was very similar to yours, so I am confident that the net
> result of your patch series is correct. But the remaining differences in
> the end results are also interesting. I made a few more changes in the
> neighborhood of the patches, not to mention a few formatting
> improvements in code that I touched. If you compare the tip of my
> branch, above, to the tip of yours (I uploaded that to my repo too, as
> branch "bc-oid-refs"), it may give you some ideas for other code that
> can be changed to object_id.

This is a very interesting approach.  I've only just had time to look at
it, but I like it.

I agree that it's much more bisectable, although your series has a much
larger quantity of patches.  I feel like sending an 83-patch series to
the list may frighten reviewers away.  That's really the only thing
preventing me from replacing my series with yours.

Junio, what would you think about such a series?  Would you prefer
slightly larger patches, one per file, if it meant that we had many
fewer patches, or would you prefer a large number of patches touching a
single function each?
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]