[PATCH v2 00/21] object_id part 7

[PATCH v2 00/21] object_id part 7

[brian m. carlson]

This is part 7 in the continuing transition to use struct object_id.

This series focuses on two main areas: adding two constants for the
maximum hash size we'll be using (which will be suitable for allocating
memory) and converting struct sha1_array to struct oid_array.

The rationale for adding separate constants for allocating memory is
that with a new 256-bit hash function, we're going to need two different
items: a constant for allocating memory that's as large as the largest
hash, and a global variable telling us size the current hash is.  I've
opted to provide GIT_MAX_RAWSZ and GIT_MAX_HEXSZ for allocating memory,
and leave GIT_SHA1_RAWSZ and GIT_SHA1_HEXSZ as values that can be later
replaced by the aforementioned global.

Replacing struct sha1_array with struct oid_array necessarily involves
converting the shallow code, so I did that.  The structure now handles
objects of struct object_id.  While I renamed the documentation (since
people will search for that), I chose not to rename the sha1-array.[ch]
files or the test helper because I didn't think it was worth the hassle,
especially for people who don't have rename support turned on by
default.

There is also a patch for fixing some broken pointer arithmetic that was
discovered during review of v1.  I don't think it's exploitable, but it
seems good to fix anyway.  Additional eyes on this patch are welcomed.

I chose to use Coccinelle quite a bit in this series, as it automates a
lot of the manual work and aides in review.  There is also some use of
Perl one-liners.

This series is available at https://github.com/bk2204/git under
object-id-part7; it may be rebased.

Changes from v1:
* Rebase on current master (no changes).
* Remove check for empty line in queue_command.
* Add patch 6 to fix invalid pointer arithmetic.

brian m. carlson (21):
  Define new hash-size constants for allocating memory
  Convert GIT_SHA1_HEXSZ used for allocation to GIT_MAX_HEXSZ
  Convert GIT_SHA1_RAWSZ used for allocation to GIT_MAX_RAWSZ
  builtin/diff: convert to struct object_id
  builtin/pull: convert portions to struct object_id
  builtin/receive-pack: fix incorrect pointer arithmetic
  builtin/receive-pack: convert portions to struct object_id
  fsck: convert init_skiplist to struct object_id
  parse-options-cb: convert sha1_array_append caller to struct object_id
  test-sha1-array: convert most code to struct object_id
  sha1_name: convert struct disambiguate_state to object_id
  sha1_name: convert disambiguate_hint_fn to take object_id
  submodule: convert check_for_new_submodule_commits to object_id
  builtin/pull: convert to struct object_id
  sha1-array: convert internal storage for struct sha1_array to
    object_id
  Make sha1_array_append take a struct object_id *
  Convert remaining callers of sha1_array_lookup to object_id
  Convert sha1_array_lookup to take struct object_id
  Convert sha1_array_for_each_unique and for_each_abbrev to object_id
  Rename sha1_array to oid_array
  Documentation: update and rename api-sha1-array.txt

 .../{api-sha1-array.txt => api-oid-array.txt}      |  44 +++----
 bisect.c                                           |  43 ++++---
 builtin/blame.c                                    |   4 +-
 builtin/cat-file.c                                 |  14 +--
 builtin/diff.c                                     |  40 +++---
 builtin/fetch-pack.c                               |   2 +-
 builtin/fetch.c                                    |   6 +-
 builtin/merge-index.c                              |   2 +-
 builtin/merge.c                                    |   2 +-
 builtin/pack-objects.c                             |  24 ++--
 builtin/patch-id.c                                 |   2 +-
 builtin/pull.c                                     |  98 +++++++--------
 builtin/receive-pack.c                             | 136 ++++++++++-----------
 builtin/rev-list.c                                 |   2 +-
 builtin/rev-parse.c                                |   4 +-
 builtin/send-pack.c                                |   4 +-
 cache.h                                            |  10 +-
 combine-diff.c                                     |  18 +--
 commit.h                                           |  14 +--
 connect.c                                          |   8 +-
 diff.c                                             |   4 +-
 diff.h                                             |   4 +-
 fetch-pack.c                                       |  32 ++---
 fetch-pack.h                                       |   4 +-
 fsck.c                                             |  17 +--
 fsck.h                                             |   2 +-
 hex.c                                              |   2 +-
 parse-options-cb.c                                 |   8 +-
 patch-ids.c                                        |   2 +-
 patch-ids.h                                        |   2 +-
 ref-filter.c                                       |  22 ++--
 ref-filter.h                                       |   2 +-
 remote-curl.c                                      |   4 +-
 remote.h                                           |   6 +-
 send-pack.c                                        |   6 +-
 send-pack.h                                        |   2 +-
 sha1-array.c                                       |  38 +++---
 sha1-array.h                                       |  20 +--
 sha1_file.c                                        |   6 +-
 sha1_name.c                                        |  94 +++++++-------
 shallow.c                                          |  38 +++---
 submodule.c                                        |  66 +++++-----
 submodule.h                                        |   8 +-
 t/helper/test-sha1-array.c                         |  20 +--
 transport.c                                        |  24 ++--
 wt-status.h                                        |   2 +-
 46 files changed, 460 insertions(+), 452 deletions(-)
 rename Documentation/technical/{api-sha1-array.txt => api-oid-array.txt} (61%)

[PATCH v2 01/21] Define new hash-size constants for allocating memory

[brian m. carlson]

Since we will want to transition to a new hash at some point in the
future, and that hash may be larger in size than 160 bits, introduce two
constants that can be used for allocating a sufficient amount of memory.
They can be increased to reflect the largest supported hash size.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 cache.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/cache.h b/cache.h
index 2214d52f61..af4b2c78cf 100644
--- a/cache.h
+++ b/cache.h
@@ -66,8 +66,12 @@ unsigned long git_deflate_bound(git_zstream *, unsigned long);
 #define GIT_SHA1_RAWSZ 20
 #define GIT_SHA1_HEXSZ (2 * GIT_SHA1_RAWSZ)
 
+/* The length in byte and in hex digits of the largest possible hash value. */
+#define GIT_MAX_RAWSZ GIT_SHA1_RAWSZ
+#define GIT_MAX_HEXSZ GIT_SHA1_HEXSZ
+
 struct object_id {
-	unsigned char hash[GIT_SHA1_RAWSZ];
+	unsigned char hash[GIT_MAX_RAWSZ];
 };
 
 #if defined(DT_UNKNOWN) && !defined(NO_D_TYPE_IN_DIRENT)

[PATCH v2 02/21] Convert GIT_SHA1_HEXSZ used for allocation to GIT_MAX_HEXSZ

[brian m. carlson]

Since we will likely be introducing a new hash function at some point,
and that hash function might be longer than 40 hex characters, use the
constant GIT_MAX_HEXSZ, which is designed to be suitable for
allocations, instead of GIT_SHA1_HEXSZ.  This will ease the transition
down the line by distinguishing between places where we need to allocate
memory suitable for the largest hash from those where we need to handle
the current hash.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 bisect.c              | 2 +-
 builtin/blame.c       | 4 ++--
 builtin/merge-index.c | 2 +-
 builtin/merge.c       | 2 +-
 builtin/rev-list.c    | 2 +-
 diff.c                | 4 ++--
 hex.c                 | 2 +-
 sha1_file.c           | 2 +-
 sha1_name.c           | 6 +++---
 transport.c           | 2 +-
 10 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/bisect.c b/bisect.c
index 30808cadf7..21c3e34636 100644
--- a/bisect.c
+++ b/bisect.c
@@ -682,7 +682,7 @@ static int is_expected_rev(const struct object_id *oid)
 
 static int bisect_checkout(const unsigned char *bisect_rev, int no_checkout)
 {
-	char bisect_rev_hex[GIT_SHA1_HEXSZ + 1];
+	char bisect_rev_hex[GIT_MAX_HEXSZ + 1];
 
 	memcpy(bisect_rev_hex, sha1_to_hex(bisect_rev), GIT_SHA1_HEXSZ + 1);
 	update_ref(NULL, "BISECT_EXPECTED_REV", bisect_rev, NULL, 0, UPDATE_REFS_DIE_ON_ERR);
diff --git a/builtin/blame.c b/builtin/blame.c
index f7aa95f4ba..07506a3e45 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -1890,7 +1890,7 @@ static void emit_porcelain(struct scoreboard *sb, struct blame_entry *ent,
 	int cnt;
 	const char *cp;
 	struct origin *suspect = ent->suspect;
-	char hex[GIT_SHA1_HEXSZ + 1];
+	char hex[GIT_MAX_HEXSZ + 1];
 
 	oid_to_hex_r(hex, &suspect->commit->object.oid);
 	printf("%s %d %d %d\n",
@@ -1928,7 +1928,7 @@ static void emit_other(struct scoreboard *sb, struct blame_entry *ent, int opt)
 	const char *cp;
 	struct origin *suspect = ent->suspect;
 	struct commit_info ci;
-	char hex[GIT_SHA1_HEXSZ + 1];
+	char hex[GIT_MAX_HEXSZ + 1];
 	int show_raw_time = !!(opt & OUTPUT_RAW_TIMESTAMP);
 
 	get_commit_info(suspect->commit, &ci, 1);
diff --git a/builtin/merge-index.c b/builtin/merge-index.c
index 2d1b6db6bd..c99443b095 100644
--- a/builtin/merge-index.c
+++ b/builtin/merge-index.c
@@ -9,7 +9,7 @@ static int merge_entry(int pos, const char *path)
 {
 	int found;
 	const char *arguments[] = { pgm, "", "", "", path, "", "", "", NULL };
-	char hexbuf[4][GIT_SHA1_HEXSZ + 1];
+	char hexbuf[4][GIT_MAX_HEXSZ + 1];
 	char ownbuf[4][60];
 
 	if (pos >= active_nr)
diff --git a/builtin/merge.c b/builtin/merge.c
index 7554b8d412..a2cceea3fb 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1296,7 +1296,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	if (verify_signatures) {
 		for (p = remoteheads; p; p = p->next) {
 			struct commit *commit = p->item;
-			char hex[GIT_SHA1_HEXSZ + 1];
+			char hex[GIT_MAX_HEXSZ + 1];
 			struct signature_check signature_check;
 			memset(&signature_check, 0, sizeof(signature_check));
 
diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 0aa93d5891..bcf77f0b8a 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -212,7 +212,7 @@ static void print_var_int(const char *var, int val)
 static int show_bisect_vars(struct rev_list_info *info, int reaches, int all)
 {
 	int cnt, flags = info->flags;
-	char hex[GIT_SHA1_HEXSZ + 1] = "";
+	char hex[GIT_MAX_HEXSZ + 1] = "";
 	struct commit_list *tried;
 	struct rev_info *revs = info->revs;
 
diff --git a/diff.c b/diff.c
index a628ac3a95..330b640c68 100644
--- a/diff.c
+++ b/diff.c
@@ -398,7 +398,7 @@ static struct diff_tempfile {
 	 */
 	const char *name;
 
-	char hex[GIT_SHA1_HEXSZ + 1];
+	char hex[GIT_MAX_HEXSZ + 1];
 	char mode[10];
 
 	/*
@@ -4219,7 +4219,7 @@ const char *diff_aligned_abbrev(const struct object_id *oid, int len)
 	 * uniqueness across all objects (statistically speaking).
 	 */
 	if (abblen < GIT_SHA1_HEXSZ - 3) {
-		static char hex[GIT_SHA1_HEXSZ + 1];
+		static char hex[GIT_MAX_HEXSZ + 1];
 		if (len < abblen && abblen <= len + 2)
 			xsnprintf(hex, sizeof(hex), "%s%.*s", abbrev, len+3-abblen, "..");
 		else
diff --git a/hex.c b/hex.c
index eab7b626ee..28b44118cb 100644
--- a/hex.c
+++ b/hex.c
@@ -85,7 +85,7 @@ char *oid_to_hex_r(char *buffer, const struct object_id *oid)
 char *sha1_to_hex(const unsigned char *sha1)
 {
 	static int bufno;
-	static char hexbuffer[4][GIT_SHA1_HEXSZ + 1];
+	static char hexbuffer[4][GIT_MAX_HEXSZ + 1];
 	bufno = (bufno + 1) % ARRAY_SIZE(hexbuffer);
 	return sha1_to_hex_r(hexbuffer[bufno], sha1);
 }
diff --git a/sha1_file.c b/sha1_file.c
index 71063890ff..e763000d34 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3759,7 +3759,7 @@ static int for_each_file_in_obj_subdir(int subdir_nr,
 		strbuf_addf(path, "/%s", de->d_name);
 
 		if (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2)  {
-			char hex[GIT_SHA1_HEXSZ+1];
+			char hex[GIT_MAX_HEXSZ+1];
 			struct object_id oid;
 
 			snprintf(hex, sizeof(hex), "%02x%s",
diff --git a/sha1_name.c b/sha1_name.c
index cda9e49b12..964201bc26 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -15,7 +15,7 @@ typedef int (*disambiguate_hint_fn)(const unsigned char *, void *);
 
 struct disambiguate_state {
 	int len; /* length of prefix in hex chars */
-	char hex_pfx[GIT_SHA1_HEXSZ + 1];
+	char hex_pfx[GIT_MAX_HEXSZ + 1];
 	unsigned char bin_pfx[GIT_SHA1_RAWSZ];
 
 	disambiguate_hint_fn fn;
@@ -80,7 +80,7 @@ static void update_candidates(struct disambiguate_state *ds, const unsigned char
 static void find_short_object_filename(struct disambiguate_state *ds)
 {
 	struct alternate_object_database *alt;
-	char hex[GIT_SHA1_HEXSZ];
+	char hex[GIT_MAX_HEXSZ];
 	static struct alternate_object_database *fakeent;
 
 	if (!fakeent) {
@@ -509,7 +509,7 @@ int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len)
 const char *find_unique_abbrev(const unsigned char *sha1, int len)
 {
 	static int bufno;
-	static char hexbuffer[4][GIT_SHA1_HEXSZ + 1];
+	static char hexbuffer[4][GIT_MAX_HEXSZ + 1];
 	char *hex = hexbuffer[bufno];
 	bufno = (bufno + 1) % ARRAY_SIZE(hexbuffer);
 	find_unique_abbrev_r(hex, sha1, len);
diff --git a/transport.c b/transport.c
index 417ed7f19f..8a90b0c29b 100644
--- a/transport.c
+++ b/transport.c
@@ -447,7 +447,7 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count,
 
 static int measure_abbrev(const struct object_id *oid, int sofar)
 {
-	char hex[GIT_SHA1_HEXSZ + 1];
+	char hex[GIT_MAX_HEXSZ + 1];
 	int w = find_unique_abbrev_r(hex, oid->hash, DEFAULT_ABBREV);
 
 	return (w < sofar) ? sofar : w;

[PATCH v2 03/21] Convert GIT_SHA1_RAWSZ used for allocation to GIT_MAX_RAWSZ

[brian m. carlson]

Since we will likely be introducing a new hash function at some point,
and that hash function might be longer than 20 bytes, use the constant
GIT_MAX_RAWSZ, which is designed to be suitable for allocations, instead
of GIT_SHA1_RAWSZ.  This will ease the transition down the line by
distinguishing between places where we need to allocate memory suitable
for the largest hash from those where we need to handle the current
hash.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/patch-id.c     | 2 +-
 builtin/receive-pack.c | 2 +-
 cache.h                | 2 +-
 patch-ids.c            | 2 +-
 patch-ids.h            | 2 +-
 sha1_file.c            | 4 ++--
 sha1_name.c            | 4 ++--
 wt-status.h            | 2 +-
 8 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/builtin/patch-id.c b/builtin/patch-id.c
index a84d0003a3..81552e02e4 100644
--- a/builtin/patch-id.c
+++ b/builtin/patch-id.c
@@ -55,7 +55,7 @@ static int scan_hunk_header(const char *p, int *p_before, int *p_after)
 
 static void flush_one_hunk(struct object_id *result, git_SHA_CTX *ctx)
 {
-	unsigned char hash[GIT_SHA1_RAWSZ];
+	unsigned char hash[GIT_MAX_RAWSZ];
 	unsigned short carry = 0;
 	int i;
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index fb2a090a0c..feafb076a4 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1162,7 +1162,7 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
 	const char *dst_name;
 	struct string_list_item *item;
 	struct command *dst_cmd;
-	unsigned char sha1[GIT_SHA1_RAWSZ];
+	unsigned char sha1[GIT_MAX_RAWSZ];
 	int flag;
 
 	strbuf_addf(&buf, "%s%s", get_git_namespace(), cmd->ref_name);
diff --git a/cache.h b/cache.h
index af4b2c78cf..179bdc5da2 100644
--- a/cache.h
+++ b/cache.h
@@ -968,7 +968,7 @@ extern char *sha1_pack_index_name(const unsigned char *sha1);
 extern const char *find_unique_abbrev(const unsigned char *sha1, int len);
 extern int find_unique_abbrev_r(char *hex, const unsigned char *sha1, int len);
 
-extern const unsigned char null_sha1[GIT_SHA1_RAWSZ];
+extern const unsigned char null_sha1[GIT_MAX_RAWSZ];
 extern const struct object_id null_oid;
 
 static inline int hashcmp(const unsigned char *sha1, const unsigned char *sha2)
diff --git a/patch-ids.c b/patch-ids.c
index ce285c2e0c..fa8f11de82 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -71,7 +71,7 @@ static int init_patch_id_entry(struct patch_id *patch,
 			       struct commit *commit,
 			       struct patch_ids *ids)
 {
-	unsigned char header_only_patch_id[GIT_SHA1_RAWSZ];
+	unsigned char header_only_patch_id[GIT_MAX_RAWSZ];
 
 	patch->commit = commit;
 	if (commit_patch_id(commit, &ids->diffopts, header_only_patch_id, 1))
diff --git a/patch-ids.h b/patch-ids.h
index 0f34ea11ea..b9e5751f8e 100644
--- a/patch-ids.h
+++ b/patch-ids.h
@@ -3,7 +3,7 @@
 
 struct patch_id {
 	struct hashmap_entry ent;
-	unsigned char patch_id[GIT_SHA1_RAWSZ];
+	unsigned char patch_id[GIT_MAX_RAWSZ];
 	struct commit *commit;
 };
 
diff --git a/sha1_file.c b/sha1_file.c
index e763000d34..b4666ee5c2 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -1606,7 +1606,7 @@ static void mark_bad_packed_object(struct packed_git *p,
 		if (!hashcmp(sha1, p->bad_object_sha1 + GIT_SHA1_RAWSZ * i))
 			return;
 	p->bad_object_sha1 = xrealloc(p->bad_object_sha1,
-				      st_mult(GIT_SHA1_RAWSZ,
+				      st_mult(GIT_MAX_RAWSZ,
 					      st_add(p->num_bad_objects, 1)));
 	hashcpy(p->bad_object_sha1 + GIT_SHA1_RAWSZ * p->num_bad_objects, sha1);
 	p->num_bad_objects++;
@@ -3913,7 +3913,7 @@ static int check_stream_sha1(git_zstream *stream,
 			     const unsigned char *expected_sha1)
 {
 	git_SHA_CTX c;
-	unsigned char real_sha1[GIT_SHA1_RAWSZ];
+	unsigned char real_sha1[GIT_MAX_RAWSZ];
 	unsigned char buf[4096];
 	unsigned long total_read;
 	int status = Z_OK;
diff --git a/sha1_name.c b/sha1_name.c
index 964201bc26..3db166b40b 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -16,11 +16,11 @@ typedef int (*disambiguate_hint_fn)(const unsigned char *, void *);
 struct disambiguate_state {
 	int len; /* length of prefix in hex chars */
 	char hex_pfx[GIT_MAX_HEXSZ + 1];
-	unsigned char bin_pfx[GIT_SHA1_RAWSZ];
+	unsigned char bin_pfx[GIT_MAX_RAWSZ];
 
 	disambiguate_hint_fn fn;
 	void *cb_data;
-	unsigned char candidate[GIT_SHA1_RAWSZ];
+	unsigned char candidate[GIT_MAX_RAWSZ];
 	unsigned candidate_exists:1;
 	unsigned candidate_checked:1;
 	unsigned candidate_ok:1;
diff --git a/wt-status.h b/wt-status.h
index 54fec77032..6018c627b1 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -80,7 +80,7 @@ struct wt_status {
 	int hints;
 
 	enum wt_status_format status_format;
-	unsigned char sha1_commit[GIT_SHA1_RAWSZ]; /* when not Initial */
+	unsigned char sha1_commit[GIT_MAX_RAWSZ]; /* when not Initial */
 
 	/* These are computed during processing of the individual sections */
 	int commitable;

[PATCH v2 04/21] builtin/diff: convert to struct object_id

[brian m. carlson]

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/diff.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index 3d64b85337..398eee00d5 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -21,7 +21,7 @@
 #define DIFF_NO_INDEX_IMPLICIT 2
 
 struct blobinfo {
-	unsigned char sha1[20];
+	struct object_id oid;
 	const char *name;
 	unsigned mode;
 };
@@ -31,22 +31,22 @@ static const char builtin_diff_usage[] =
 
 static void stuff_change(struct diff_options *opt,
 			 unsigned old_mode, unsigned new_mode,
-			 const unsigned char *old_sha1,
-			 const unsigned char *new_sha1,
-			 int old_sha1_valid,
-			 int new_sha1_valid,
+			 const struct object_id *old_oid,
+			 const struct object_id *new_oid,
+			 int old_oid_valid,
+			 int new_oid_valid,
 			 const char *old_name,
 			 const char *new_name)
 {
 	struct diff_filespec *one, *two;
 
-	if (!is_null_sha1(old_sha1) && !is_null_sha1(new_sha1) &&
-	    !hashcmp(old_sha1, new_sha1) && (old_mode == new_mode))
+	if (!is_null_oid(old_oid) && !is_null_oid(new_oid) &&
+	    !oidcmp(old_oid, new_oid) && (old_mode == new_mode))
 		return;
 
 	if (DIFF_OPT_TST(opt, REVERSE_DIFF)) {
 		SWAP(old_mode, new_mode);
-		SWAP(old_sha1, new_sha1);
+		SWAP(old_oid, new_oid);
 		SWAP(old_name, new_name);
 	}
 
@@ -57,8 +57,8 @@ static void stuff_change(struct diff_options *opt,
 
 	one = alloc_filespec(old_name);
 	two = alloc_filespec(new_name);
-	fill_filespec(one, old_sha1, old_sha1_valid, old_mode);
-	fill_filespec(two, new_sha1, new_sha1_valid, new_mode);
+	fill_filespec(one, old_oid->hash, old_oid_valid, old_mode);
+	fill_filespec(two, new_oid->hash, new_oid_valid, new_mode);
 
 	diff_queue(&diff_queued_diff, one, two);
 }
@@ -89,7 +89,7 @@ static int builtin_diff_b_f(struct rev_info *revs,
 
 	stuff_change(&revs->diffopt,
 		     blob[0].mode, canon_mode(st.st_mode),
-		     blob[0].sha1, null_sha1,
+		     &blob[0].oid, &null_oid,
 		     1, 0,
 		     path, path);
 	diffcore_std(&revs->diffopt);
@@ -114,7 +114,7 @@ static int builtin_diff_blobs(struct rev_info *revs,
 
 	stuff_change(&revs->diffopt,
 		     blob[0].mode, blob[1].mode,
-		     blob[0].sha1, blob[1].sha1,
+		     &blob[0].oid, &blob[1].oid,
 		     1, 1,
 		     blob[0].name, blob[1].name);
 	diffcore_std(&revs->diffopt);
@@ -160,7 +160,7 @@ static int builtin_diff_tree(struct rev_info *revs,
 			     struct object_array_entry *ent0,
 			     struct object_array_entry *ent1)
 {
-	const unsigned char *(sha1[2]);
+	const struct object_id *(oid[2]);
 	int swap = 0;
 
 	if (argc > 1)
@@ -172,9 +172,9 @@ static int builtin_diff_tree(struct rev_info *revs,
 	 */
 	if (ent1->item->flags & UNINTERESTING)
 		swap = 1;
-	sha1[swap] = ent0->item->oid.hash;
-	sha1[1 - swap] = ent1->item->oid.hash;
-	diff_tree_sha1(sha1[0], sha1[1], "", &revs->diffopt);
+	oid[swap] = &ent0->item->oid;
+	oid[1 - swap] = &ent1->item->oid;
+	diff_tree_sha1(oid[0]->hash, oid[1]->hash, "", &revs->diffopt);
 	log_tree_diff_flush(revs);
 	return 0;
 }
@@ -408,7 +408,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
 		} else if (obj->type == OBJ_BLOB) {
 			if (2 <= blobs)
 				die(_("more than two blobs given: '%s'"), name);
-			hashcpy(blob[blobs].sha1, obj->oid.hash);
+			hashcpy(blob[blobs].oid.hash, obj->oid.hash);
 			blob[blobs].name = name;
 			blob[blobs].mode = entry->mode;
 			blobs++;

[PATCH v2 05/21] builtin/pull: convert portions to struct object_id

[brian m. carlson]

Convert the caller of sha1_array_append to struct object_id.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/pull.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index 3ecb881b0b..a9f7553f30 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -335,16 +335,16 @@ static void get_merge_heads(struct sha1_array *merge_heads)
 	const char *filename = git_path("FETCH_HEAD");
 	FILE *fp;
 	struct strbuf sb = STRBUF_INIT;
-	unsigned char sha1[GIT_SHA1_RAWSZ];
+	struct object_id oid;
 
 	if (!(fp = fopen(filename, "r")))
 		die_errno(_("could not open '%s' for reading"), filename);
 	while (strbuf_getline_lf(&sb, fp) != EOF) {
-		if (get_sha1_hex(sb.buf, sha1))
+		if (get_oid_hex(sb.buf, &oid))
 			continue;  /* invalid line: does not start with SHA1 */
 		if (starts_with(sb.buf + GIT_SHA1_HEXSZ, "\tnot-for-merge\t"))
 			continue;  /* ref is not-for-merge */
-		sha1_array_append(merge_heads, sha1);
+		sha1_array_append(merge_heads, oid.hash);
 	}
 	fclose(fp);
 	strbuf_release(&sb);

[PATCH v2 06/21] builtin/receive-pack: fix incorrect pointer arithmetic

[brian m. carlson]

If we had already processed the last newline in a push certificate, we
would end up subtracting NULL from the end-of-certificate pointer when
computing the length of the line.  This would have resulted in an
absurdly large length, and possibly a buffer overflow.  Instead,
subtract the beginning-of-certificate pointer from the
end-of-certificate pointer, which is what's expected.

Note that this situation should never occur, since not only do we
require the certificate to be newline terminated, but the signature will
only be read from the beginning of a line.  Nevertheless, it seems
prudent to correct it.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/receive-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index feafb076a4..116f3177a1 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1524,7 +1524,7 @@ static void queue_commands_from_cert(struct command **tail,
 
 	while (boc < eoc) {
 		const char *eol = memchr(boc, '\n', eoc - boc);
-		tail = queue_command(tail, boc, eol ? eol - boc : eoc - eol);
+		tail = queue_command(tail, boc, eol ? eol - boc : eoc - boc);
 		boc = eol ? eol + 1 : eoc;
 	}
 }

[PATCH v2 07/21] builtin/receive-pack: convert portions to struct object_id

[brian m. carlson]

Convert some hardcoded constants into uses of parse_oid_hex.
Additionally, convert all uses of struct command, and miscellaneous
other functions necessary for that.  This work is necessary to be able
to convert sha1_array_append later on.

To avoid needing to specify a constant, reject shallow lines with the
wrong length instead of simply ignoring them.

Note that in queue_command we are guaranteed to have a NUL-terminated
buffer or at least one byte of overflow that we can safely read, so the
linelen check can be elided.  We would die in such a case, but not read
invalid memory.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/receive-pack.c | 98 +++++++++++++++++++++++++-------------------------
 1 file changed, 49 insertions(+), 49 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 116f3177a1..e3dc3e184d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -309,8 +309,8 @@ struct command {
 	unsigned int skip_update:1,
 		     did_not_exist:1;
 	int index;
-	unsigned char old_sha1[20];
-	unsigned char new_sha1[20];
+	struct object_id old_oid;
+	struct object_id new_oid;
 	char ref_name[FLEX_ARRAY]; /* more */
 };
 
@@ -723,7 +723,7 @@ static int feed_receive_hook(void *state_, const char **bufp, size_t *sizep)
 		return -1; /* EOF */
 	strbuf_reset(&state->buf);
 	strbuf_addf(&state->buf, "%s %s %s\n",
-		    sha1_to_hex(cmd->old_sha1), sha1_to_hex(cmd->new_sha1),
+		    oid_to_hex(&cmd->old_oid), oid_to_hex(&cmd->new_oid),
 		    cmd->ref_name);
 	state->cmd = cmd->next;
 	if (bufp) {
@@ -764,8 +764,8 @@ static int run_update_hook(struct command *cmd)
 		return 0;
 
 	argv[1] = cmd->ref_name;
-	argv[2] = sha1_to_hex(cmd->old_sha1);
-	argv[3] = sha1_to_hex(cmd->new_sha1);
+	argv[2] = oid_to_hex(&cmd->old_oid);
+	argv[3] = oid_to_hex(&cmd->new_oid);
 	argv[4] = NULL;
 
 	proc.no_stdin = 1;
@@ -988,8 +988,8 @@ static const char *update(struct command *cmd, struct shallow_info *si)
 	const char *name = cmd->ref_name;
 	struct strbuf namespaced_name_buf = STRBUF_INIT;
 	const char *namespaced_name, *ret;
-	unsigned char *old_sha1 = cmd->old_sha1;
-	unsigned char *new_sha1 = cmd->new_sha1;
+	struct object_id *old_oid = &cmd->old_oid;
+	struct object_id *new_oid = &cmd->new_oid;
 
 	/* only refs/... are allowed */
 	if (!starts_with(name, "refs/") || check_refname_format(name + 5, 0)) {
@@ -1014,20 +1014,20 @@ static const char *update(struct command *cmd, struct shallow_info *si)
 				refuse_unconfigured_deny();
 			return "branch is currently checked out";
 		case DENY_UPDATE_INSTEAD:
-			ret = update_worktree(new_sha1);
+			ret = update_worktree(new_oid->hash);
 			if (ret)
 				return ret;
 			break;
 		}
 	}
 
-	if (!is_null_sha1(new_sha1) && !has_sha1_file(new_sha1)) {
+	if (!is_null_oid(new_oid) && !has_object_file(new_oid)) {
 		error("unpack should have generated %s, "
-		      "but I can't find it!", sha1_to_hex(new_sha1));
+		      "but I can't find it!", oid_to_hex(new_oid));
 		return "bad pack";
 	}
 
-	if (!is_null_sha1(old_sha1) && is_null_sha1(new_sha1)) {
+	if (!is_null_oid(old_oid) && is_null_oid(new_oid)) {
 		if (deny_deletes && starts_with(name, "refs/heads/")) {
 			rp_error("denying ref deletion for %s", name);
 			return "deletion prohibited";
@@ -1053,14 +1053,14 @@ static const char *update(struct command *cmd, struct shallow_info *si)
 		}
 	}
 
-	if (deny_non_fast_forwards && !is_null_sha1(new_sha1) &&
-	    !is_null_sha1(old_sha1) &&
+	if (deny_non_fast_forwards && !is_null_oid(new_oid) &&
+	    !is_null_oid(old_oid) &&
 	    starts_with(name, "refs/heads/")) {
 		struct object *old_object, *new_object;
 		struct commit *old_commit, *new_commit;
 
-		old_object = parse_object(old_sha1);
-		new_object = parse_object(new_sha1);
+		old_object = parse_object(old_oid->hash);
+		new_object = parse_object(new_oid->hash);
 
 		if (!old_object || !new_object ||
 		    old_object->type != OBJ_COMMIT ||
@@ -1081,10 +1081,10 @@ static const char *update(struct command *cmd, struct shallow_info *si)
 		return "hook declined";
 	}
 
-	if (is_null_sha1(new_sha1)) {
+	if (is_null_oid(new_oid)) {
 		struct strbuf err = STRBUF_INIT;
-		if (!parse_object(old_sha1)) {
-			old_sha1 = NULL;
+		if (!parse_object(old_oid->hash)) {
+			old_oid = NULL;
 			if (ref_exists(name)) {
 				rp_warning("Allowing deletion of corrupt ref.");
 			} else {
@@ -1094,7 +1094,7 @@ static const char *update(struct command *cmd, struct shallow_info *si)
 		}
 		if (ref_transaction_delete(transaction,
 					   namespaced_name,
-					   old_sha1,
+					   old_oid->hash,
 					   0, "push", &err)) {
 			rp_error("%s", err.buf);
 			strbuf_release(&err);
@@ -1111,7 +1111,7 @@ static const char *update(struct command *cmd, struct shallow_info *si)
 
 		if (ref_transaction_update(transaction,
 					   namespaced_name,
-					   new_sha1, old_sha1,
+					   new_oid->hash, old_oid->hash,
 					   0, "push",
 					   &err)) {
 			rp_error("%s", err.buf);
@@ -1187,8 +1187,8 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
 
 	dst_cmd = (struct command *) item->util;
 
-	if (!hashcmp(cmd->old_sha1, dst_cmd->old_sha1) &&
-	    !hashcmp(cmd->new_sha1, dst_cmd->new_sha1))
+	if (!oidcmp(&cmd->old_oid, &dst_cmd->old_oid) &&
+	    !oidcmp(&cmd->new_oid, &dst_cmd->new_oid))
 		return;
 
 	dst_cmd->skip_update = 1;
@@ -1196,11 +1196,11 @@ static void check_aliased_update(struct command *cmd, struct string_list *list)
 	rp_error("refusing inconsistent update between symref '%s' (%s..%s) and"
 		 " its target '%s' (%s..%s)",
 		 cmd->ref_name,
-		 find_unique_abbrev(cmd->old_sha1, DEFAULT_ABBREV),
-		 find_unique_abbrev(cmd->new_sha1, DEFAULT_ABBREV),
+		 find_unique_abbrev(cmd->old_oid.hash, DEFAULT_ABBREV),
+		 find_unique_abbrev(cmd->new_oid.hash, DEFAULT_ABBREV),
 		 dst_cmd->ref_name,
-		 find_unique_abbrev(dst_cmd->old_sha1, DEFAULT_ABBREV),
-		 find_unique_abbrev(dst_cmd->new_sha1, DEFAULT_ABBREV));
+		 find_unique_abbrev(dst_cmd->old_oid.hash, DEFAULT_ABBREV),
+		 find_unique_abbrev(dst_cmd->new_oid.hash, DEFAULT_ABBREV));
 
 	cmd->error_string = dst_cmd->error_string =
 		"inconsistent aliased update";
@@ -1231,10 +1231,10 @@ static int command_singleton_iterator(void *cb_data, unsigned char sha1[20])
 	struct command **cmd_list = cb_data;
 	struct command *cmd = *cmd_list;
 
-	if (!cmd || is_null_sha1(cmd->new_sha1))
+	if (!cmd || is_null_oid(&cmd->new_oid))
 		return -1; /* end of list */
 	*cmd_list = NULL; /* this returns only one */
-	hashcpy(sha1, cmd->new_sha1);
+	hashcpy(sha1, cmd->new_oid.hash);
 	return 0;
 }
 
@@ -1275,8 +1275,8 @@ static int iterate_receive_command_list(void *cb_data, unsigned char sha1[20])
 		if (shallow_update && data->si->shallow_ref[cmd->index])
 			/* to be checked in update_shallow_ref() */
 			continue;
-		if (!is_null_sha1(cmd->new_sha1) && !cmd->skip_update) {
-			hashcpy(sha1, cmd->new_sha1);
+		if (!is_null_oid(&cmd->new_oid) && !cmd->skip_update) {
+			hashcpy(sha1, cmd->new_oid.hash);
 			*cmd_list = cmd->next;
 			return 0;
 		}
@@ -1303,7 +1303,7 @@ static void reject_updates_to_hidden(struct command *commands)
 
 		if (!ref_is_hidden(cmd->ref_name, refname_full.buf))
 			continue;
-		if (is_null_sha1(cmd->new_sha1))
+		if (is_null_oid(&cmd->new_oid))
 			cmd->error_string = "deny deleting a hidden ref";
 		else
 			cmd->error_string = "deny updating a hidden ref";
@@ -1486,23 +1486,23 @@ static struct command **queue_command(struct command **tail,
 				      const char *line,
 				      int linelen)
 {
-	unsigned char old_sha1[20], new_sha1[20];
+	struct object_id old_oid, new_oid;
 	struct command *cmd;
 	const char *refname;
 	int reflen;
+	const char *p;
 
-	if (linelen < 83 ||
-	    line[40] != ' ' ||
-	    line[81] != ' ' ||
-	    get_sha1_hex(line, old_sha1) ||
-	    get_sha1_hex(line + 41, new_sha1))
+	if (parse_oid_hex(line, &old_oid, &p) ||
+	    *p++ != ' ' ||
+	    parse_oid_hex(p, &new_oid, &p) ||
+	    *p++ != ' ')
 		die("protocol error: expected old/new/ref, got '%s'", line);
 
-	refname = line + 82;
-	reflen = linelen - 82;
+	refname = p;
+	reflen = linelen - (p - line);
 	FLEX_ALLOC_MEM(cmd, ref_name, refname, reflen);
-	hashcpy(cmd->old_sha1, old_sha1);
-	hashcpy(cmd->new_sha1, new_sha1);
+	oidcpy(&cmd->old_oid, &old_oid);
+	oidcpy(&cmd->new_oid, &new_oid);
 	*tail = cmd;
 	return &cmd->next;
 }
@@ -1541,12 +1541,12 @@ static struct command *read_head_info(struct sha1_array *shallow)
 		if (!line)
 			break;
 
-		if (len == 48 && starts_with(line, "shallow ")) {
-			unsigned char sha1[20];
-			if (get_sha1_hex(line + 8, sha1))
+		if (len > 8 && starts_with(line, "shallow ")) {
+			struct object_id oid;
+			if (get_oid_hex(line + 8, &oid))
 				die("protocol error: expected shallow sha, got '%s'",
 				    line + 8);
-			sha1_array_append(shallow, sha1);
+			sha1_array_append(shallow, oid.hash);
 			continue;
 		}
 
@@ -1815,9 +1815,9 @@ static void update_shallow_info(struct command *commands,
 	}
 
 	for (cmd = commands; cmd; cmd = cmd->next) {
-		if (is_null_sha1(cmd->new_sha1))
+		if (is_null_oid(&cmd->new_oid))
 			continue;
-		sha1_array_append(ref, cmd->new_sha1);
+		sha1_array_append(ref, cmd->new_oid.hash);
 		cmd->index = ref->nr - 1;
 	}
 	si->ref = ref;
@@ -1830,7 +1830,7 @@ static void update_shallow_info(struct command *commands,
 	ALLOC_ARRAY(ref_status, ref->nr);
 	assign_shallow_commits_to_refs(si, NULL, ref_status);
 	for (cmd = commands; cmd; cmd = cmd->next) {
-		if (is_null_sha1(cmd->new_sha1))
+		if (is_null_oid(&cmd->new_oid))
 			continue;
 		if (ref_status[cmd->index]) {
 			cmd->error_string = "shallow update not allowed";
@@ -1868,7 +1868,7 @@ static int delete_only(struct command *commands)
 {
 	struct command *cmd;
 	for (cmd = commands; cmd; cmd = cmd->next) {
-		if (!is_null_sha1(cmd->new_sha1))
+		if (!is_null_oid(&cmd->new_oid))
 			return 0;
 	}
 	return 1;

[PATCH v2 08/21] fsck: convert init_skiplist to struct object_id

[brian m. carlson]

Convert a hardcoded constant buffer size to a use of GIT_MAX_HEXSZ, and
use parse_oid_hex to reduce the dependency on the size of the hash.
This function is a caller of sha1_array_append, which will be converted
later.

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

diff --git a/fsck.c b/fsck.c
index 939792752b..aff4ae6fd4 100644
--- a/fsck.c
+++ b/fsck.c
@@ -134,8 +134,8 @@ static void init_skiplist(struct fsck_options *options, const char *path)
 {
 	static struct sha1_array skiplist = SHA1_ARRAY_INIT;
 	int sorted, fd;
-	char buffer[41];
-	unsigned char sha1[20];
+	char buffer[GIT_MAX_HEXSZ + 1];
+	struct object_id oid;
 
 	if (options->skiplist)
 		sorted = options->skiplist->sorted;
@@ -148,17 +148,18 @@ static void init_skiplist(struct fsck_options *options, const char *path)
 	if (fd < 0)
 		die("Could not open skip list: %s", path);
 	for (;;) {
+		const char *p;
 		int result = read_in_full(fd, buffer, sizeof(buffer));
 		if (result < 0)
 			die_errno("Could not read '%s'", path);
 		if (!result)
 			break;
-		if (get_sha1_hex(buffer, sha1) || buffer[40] != '\n')
+		if (parse_oid_hex(buffer, &oid, &p) || *p != '\n')
 			die("Invalid SHA-1: %s", buffer);
-		sha1_array_append(&skiplist, sha1);
+		sha1_array_append(&skiplist, oid.hash);
 		if (sorted && skiplist.nr > 1 &&
 				hashcmp(skiplist.sha1[skiplist.nr - 2],
-					sha1) > 0)
+					oid.hash) > 0)
 			sorted = 0;
 	}
 	close(fd);

[PATCH v2 09/21] parse-options-cb: convert sha1_array_append caller to struct object_id

[brian m. carlson]

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 parse-options-cb.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/parse-options-cb.c b/parse-options-cb.c
index b7d8f7dcb2..40ece4d8c2 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -96,7 +96,7 @@ int parse_opt_commits(const struct option *opt, const char *arg, int unset)
 
 int parse_opt_object_name(const struct option *opt, const char *arg, int unset)
 {
-	unsigned char sha1[20];
+	struct object_id oid;
 
 	if (unset) {
 		sha1_array_clear(opt->value);
@@ -104,9 +104,9 @@ int parse_opt_object_name(const struct option *opt, const char *arg, int unset)
 	}
 	if (!arg)
 		return -1;
-	if (get_sha1(arg, sha1))
+	if (get_oid(arg, &oid))
 		return error(_("malformed object name '%s'"), arg);
-	sha1_array_append(opt->value, sha1);
+	sha1_array_append(opt->value, oid.hash);
 	return 0;
 }
 

[PATCH v2 10/21] test-sha1-array: convert most code to struct object_id

[brian m. carlson]

This helper is very small, so convert the entire thing.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 t/helper/test-sha1-array.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/t/helper/test-sha1-array.c b/t/helper/test-sha1-array.c
index f7a53c4ad6..b4bb97fccc 100644
--- a/t/helper/test-sha1-array.c
+++ b/t/helper/test-sha1-array.c
@@ -14,16 +14,16 @@ int cmd_main(int argc, const char **argv)
 
 	while (strbuf_getline(&line, stdin) != EOF) {
 		const char *arg;
-		unsigned char sha1[20];
+		struct object_id oid;
 
 		if (skip_prefix(line.buf, "append ", &arg)) {
-			if (get_sha1_hex(arg, sha1))
+			if (get_oid_hex(arg, &oid))
 				die("not a hexadecimal SHA1: %s", arg);
-			sha1_array_append(&array, sha1);
+			sha1_array_append(&array, oid.hash);
 		} else if (skip_prefix(line.buf, "lookup ", &arg)) {
-			if (get_sha1_hex(arg, sha1))
+			if (get_oid_hex(arg, &oid))
 				die("not a hexadecimal SHA1: %s", arg);
-			printf("%d\n", sha1_array_lookup(&array, sha1));
+			printf("%d\n", sha1_array_lookup(&array, oid.hash));
 		} else if (!strcmp(line.buf, "clear"))
 			sha1_array_clear(&array);
 		else if (!strcmp(line.buf, "for_each_unique"))

[PATCH v2 11/21] sha1_name: convert struct disambiguate_state to object_id

[brian m. carlson]

Convert struct disambiguate_state to use struct object_id by changing
the structure definition and applying the following semantic patch:

@@
struct disambiguate_state E1;
@@
- E1.bin_pfx
+ E1.bin_pfx.hash

@@
struct disambiguate_state *E1;
@@
- E1->bin_pfx
+ E1->bin_pfx.hash

@@
struct disambiguate_state E1;
@@
- E1.candidate
+ E1.candidate.hash

@@
struct disambiguate_state *E1;
@@
- E1->candidate
+ E1->candidate.hash

This conversion is needed so we can convert disambiguate_hint_fn later.

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

diff --git a/sha1_name.c b/sha1_name.c
index 3db166b40b..cf6f4be0c6 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -16,11 +16,11 @@ typedef int (*disambiguate_hint_fn)(const unsigned char *, void *);
 struct disambiguate_state {
 	int len; /* length of prefix in hex chars */
 	char hex_pfx[GIT_MAX_HEXSZ + 1];
-	unsigned char bin_pfx[GIT_MAX_RAWSZ];
+	struct object_id bin_pfx;
 
 	disambiguate_hint_fn fn;
 	void *cb_data;
-	unsigned char candidate[GIT_MAX_RAWSZ];
+	struct object_id candidate;
 	unsigned candidate_exists:1;
 	unsigned candidate_checked:1;
 	unsigned candidate_ok:1;
@@ -37,10 +37,10 @@ static void update_candidates(struct disambiguate_state *ds, const unsigned char
 	}
 	if (!ds->candidate_exists) {
 		/* this is the first candidate */
-		hashcpy(ds->candidate, current);
+		hashcpy(ds->candidate.hash, current);
 		ds->candidate_exists = 1;
 		return;
-	} else if (!hashcmp(ds->candidate, current)) {
+	} else if (!hashcmp(ds->candidate.hash, current)) {
 		/* the same as what we already have seen */
 		return;
 	}
@@ -52,14 +52,14 @@ static void update_candidates(struct disambiguate_state *ds, const unsigned char
 	}
 
 	if (!ds->candidate_checked) {
-		ds->candidate_ok = ds->fn(ds->candidate, ds->cb_data);
+		ds->candidate_ok = ds->fn(ds->candidate.hash, ds->cb_data);
 		ds->disambiguate_fn_used = 1;
 		ds->candidate_checked = 1;
 	}
 
 	if (!ds->candidate_ok) {
 		/* discard the candidate; we know it does not satisfy fn */
-		hashcpy(ds->candidate, current);
+		hashcpy(ds->candidate.hash, current);
 		ds->candidate_checked = 0;
 		return;
 	}
@@ -151,7 +151,7 @@ static void unique_in_pack(struct packed_git *p,
 		int cmp;
 
 		current = nth_packed_object_sha1(p, mid);
-		cmp = hashcmp(ds->bin_pfx, current);
+		cmp = hashcmp(ds->bin_pfx.hash, current);
 		if (!cmp) {
 			first = mid;
 			break;
@@ -170,7 +170,7 @@ static void unique_in_pack(struct packed_git *p,
 	 */
 	for (i = first; i < num && !ds->ambiguous; i++) {
 		current = nth_packed_object_sha1(p, i);
-		if (!match_sha(ds->len, ds->bin_pfx, current))
+		if (!match_sha(ds->len, ds->bin_pfx.hash, current))
 			break;
 		update_candidates(ds, current);
 	}
@@ -213,12 +213,12 @@ static int finish_object_disambiguation(struct disambiguate_state *ds,
 		 * same repository!
 		 */
 		ds->candidate_ok = (!ds->disambiguate_fn_used ||
-				    ds->fn(ds->candidate, ds->cb_data));
+				    ds->fn(ds->candidate.hash, ds->cb_data));
 
 	if (!ds->candidate_ok)
 		return SHORT_NAME_AMBIGUOUS;
 
-	hashcpy(sha1, ds->candidate);
+	hashcpy(sha1, ds->candidate.hash);
 	return 0;
 }
 
@@ -332,7 +332,7 @@ static int init_object_disambiguation(const char *name, int len,
 		ds->hex_pfx[i] = c;
 		if (!(i & 1))
 			val <<= 4;
-		ds->bin_pfx[i >> 1] |= val;
+		ds->bin_pfx.hash[i >> 1] |= val;
 	}
 
 	ds->len = len;

[PATCH v2 12/21] sha1_name: convert disambiguate_hint_fn to take object_id

[brian m. carlson]

Convert this function pointer type and the functions that implement it
to take a struct object_id.  Introduce a temporary in
show_ambiguous_object to avoid having to convert for_each_abbrev at this
point.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 sha1_name.c | 64 ++++++++++++++++++++++++++++++++-----------------------------
 1 file changed, 34 insertions(+), 30 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index cf6f4be0c6..2e38aedfa5 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -11,7 +11,7 @@
 
 static int get_sha1_oneline(const char *, unsigned char *, struct commit_list *);
 
-typedef int (*disambiguate_hint_fn)(const unsigned char *, void *);
+typedef int (*disambiguate_hint_fn)(const struct object_id *, void *);
 
 struct disambiguate_state {
 	int len; /* length of prefix in hex chars */
@@ -29,7 +29,7 @@ struct disambiguate_state {
 	unsigned always_call_fn:1;
 };
 
-static void update_candidates(struct disambiguate_state *ds, const unsigned char *current)
+static void update_candidates(struct disambiguate_state *ds, const struct object_id *current)
 {
 	if (ds->always_call_fn) {
 		ds->ambiguous = ds->fn(current, ds->cb_data) ? 1 : 0;
@@ -37,10 +37,10 @@ static void update_candidates(struct disambiguate_state *ds, const unsigned char
 	}
 	if (!ds->candidate_exists) {
 		/* this is the first candidate */
-		hashcpy(ds->candidate.hash, current);
+		oidcpy(&ds->candidate, current);
 		ds->candidate_exists = 1;
 		return;
-	} else if (!hashcmp(ds->candidate.hash, current)) {
+	} else if (!oidcmp(&ds->candidate, current)) {
 		/* the same as what we already have seen */
 		return;
 	}
@@ -52,14 +52,14 @@ static void update_candidates(struct disambiguate_state *ds, const unsigned char
 	}
 
 	if (!ds->candidate_checked) {
-		ds->candidate_ok = ds->fn(ds->candidate.hash, ds->cb_data);
+		ds->candidate_ok = ds->fn(&ds->candidate, ds->cb_data);
 		ds->disambiguate_fn_used = 1;
 		ds->candidate_checked = 1;
 	}
 
 	if (!ds->candidate_ok) {
 		/* discard the candidate; we know it does not satisfy fn */
-		hashcpy(ds->candidate.hash, current);
+		oidcpy(&ds->candidate, current);
 		ds->candidate_checked = 0;
 		return;
 	}
@@ -107,15 +107,15 @@ static void find_short_object_filename(struct disambiguate_state *ds)
 			continue;
 
 		while (!ds->ambiguous && (de = readdir(dir)) != NULL) {
-			unsigned char sha1[20];
+			struct object_id oid;
 
-			if (strlen(de->d_name) != 38)
+			if (strlen(de->d_name) != GIT_SHA1_HEXSZ - 2)
 				continue;
 			if (memcmp(de->d_name, ds->hex_pfx + 2, ds->len - 2))
 				continue;
-			memcpy(hex + 2, de->d_name, 38);
-			if (!get_sha1_hex(hex, sha1))
-				update_candidates(ds, sha1);
+			memcpy(hex + 2, de->d_name, GIT_SHA1_HEXSZ - 2);
+			if (!get_oid_hex(hex, &oid))
+				update_candidates(ds, &oid);
 		}
 		closedir(dir);
 	}
@@ -140,7 +140,7 @@ static void unique_in_pack(struct packed_git *p,
 			   struct disambiguate_state *ds)
 {
 	uint32_t num, last, i, first = 0;
-	const unsigned char *current = NULL;
+	const struct object_id *current = NULL;
 
 	open_pack_index(p);
 	num = p->num_objects;
@@ -169,8 +169,9 @@ static void unique_in_pack(struct packed_git *p,
 	 * 0, 1 or more objects that actually match(es).
 	 */
 	for (i = first; i < num && !ds->ambiguous; i++) {
-		current = nth_packed_object_sha1(p, i);
-		if (!match_sha(ds->len, ds->bin_pfx.hash, current))
+		struct object_id oid;
+		current = nth_packed_object_oid(&oid, p, i);
+		if (!match_sha(ds->len, ds->bin_pfx.hash, current->hash))
 			break;
 		update_candidates(ds, current);
 	}
@@ -213,7 +214,7 @@ static int finish_object_disambiguation(struct disambiguate_state *ds,
 		 * same repository!
 		 */
 		ds->candidate_ok = (!ds->disambiguate_fn_used ||
-				    ds->fn(ds->candidate.hash, ds->cb_data));
+				    ds->fn(&ds->candidate, ds->cb_data));
 
 	if (!ds->candidate_ok)
 		return SHORT_NAME_AMBIGUOUS;
@@ -222,57 +223,57 @@ static int finish_object_disambiguation(struct disambiguate_state *ds,
 	return 0;
 }
 
-static int disambiguate_commit_only(const unsigned char *sha1, void *cb_data_unused)
+static int disambiguate_commit_only(const struct object_id *oid, void *cb_data_unused)
 {
-	int kind = sha1_object_info(sha1, NULL);
+	int kind = sha1_object_info(oid->hash, NULL);
 	return kind == OBJ_COMMIT;
 }
 
-static int disambiguate_committish_only(const unsigned char *sha1, void *cb_data_unused)
+static int disambiguate_committish_only(const struct object_id *oid, void *cb_data_unused)
 {
 	struct object *obj;
 	int kind;
 
-	kind = sha1_object_info(sha1, NULL);
+	kind = sha1_object_info(oid->hash, NULL);
 	if (kind == OBJ_COMMIT)
 		return 1;
 	if (kind != OBJ_TAG)
 		return 0;
 
 	/* We need to do this the hard way... */
-	obj = deref_tag(parse_object(sha1), NULL, 0);
+	obj = deref_tag(parse_object(oid->hash), NULL, 0);
 	if (obj && obj->type == OBJ_COMMIT)
 		return 1;
 	return 0;
 }
 
-static int disambiguate_tree_only(const unsigned char *sha1, void *cb_data_unused)
+static int disambiguate_tree_only(const struct object_id *oid, void *cb_data_unused)
 {
-	int kind = sha1_object_info(sha1, NULL);
+	int kind = sha1_object_info(oid->hash, NULL);
 	return kind == OBJ_TREE;
 }
 
-static int disambiguate_treeish_only(const unsigned char *sha1, void *cb_data_unused)
+static int disambiguate_treeish_only(const struct object_id *oid, void *cb_data_unused)
 {
 	struct object *obj;
 	int kind;
 
-	kind = sha1_object_info(sha1, NULL);
+	kind = sha1_object_info(oid->hash, NULL);
 	if (kind == OBJ_TREE || kind == OBJ_COMMIT)
 		return 1;
 	if (kind != OBJ_TAG)
 		return 0;
 
 	/* We need to do this the hard way... */
-	obj = deref_tag(parse_object(sha1), NULL, 0);
+	obj = deref_tag(parse_object(oid->hash), NULL, 0);
 	if (obj && (obj->type == OBJ_TREE || obj->type == OBJ_COMMIT))
 		return 1;
 	return 0;
 }
 
-static int disambiguate_blob_only(const unsigned char *sha1, void *cb_data_unused)
+static int disambiguate_blob_only(const struct object_id *oid, void *cb_data_unused)
 {
-	int kind = sha1_object_info(sha1, NULL);
+	int kind = sha1_object_info(oid->hash, NULL);
 	return kind == OBJ_BLOB;
 }
 
@@ -344,10 +345,13 @@ static int init_object_disambiguation(const char *name, int len,
 static int show_ambiguous_object(const unsigned char *sha1, void *data)
 {
 	const struct disambiguate_state *ds = data;
+	struct object_id oid;
 	struct strbuf desc = STRBUF_INIT;
 	int type;
 
-	if (ds->fn && !ds->fn(sha1, ds->cb_data))
+
+	hashcpy(oid.hash, sha1);
+	if (ds->fn && !ds->fn(&oid, ds->cb_data))
 		return 0;
 
 	type = sha1_object_info(sha1, NULL);
@@ -422,9 +426,9 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,
 	return status;
 }
 
-static int collect_ambiguous(const unsigned char *sha1, void *data)
+static int collect_ambiguous(const struct object_id *oid, void *data)
 {
-	sha1_array_append(data, sha1);
+	sha1_array_append(data, oid->hash);
 	return 0;
 }
 

[PATCH v2 13/21] submodule: convert check_for_new_submodule_commits to object_id

[brian m. carlson]

All of the callers of this function have been converted, so convert this
function and update the callers.  This function also calls
sha1_array_append, which we'll convert shortly.

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

diff --git a/builtin/fetch.c b/builtin/fetch.c
index b5ad09d046..a41b892dcc 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -659,7 +659,7 @@ static int update_local_ref(struct ref *ref,
 
 		if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
-			check_for_new_submodule_commits(ref->new_oid.hash);
+			check_for_new_submodule_commits(&ref->new_oid);
 		r = s_update_ref(msg, ref, 0);
 		format_display(display, r ? '!' : '*', what,
 			       r ? _("unable to update local ref") : NULL,
@@ -675,7 +675,7 @@ static int update_local_ref(struct ref *ref,
 		strbuf_add_unique_abbrev(&quickref, ref->new_oid.hash, DEFAULT_ABBREV);
 		if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
-			check_for_new_submodule_commits(ref->new_oid.hash);
+			check_for_new_submodule_commits(&ref->new_oid);
 		r = s_update_ref("fast-forward", ref, 1);
 		format_display(display, r ? '!' : ' ', quickref.buf,
 			       r ? _("unable to update local ref") : NULL,
@@ -690,7 +690,7 @@ static int update_local_ref(struct ref *ref,
 		strbuf_add_unique_abbrev(&quickref, ref->new_oid.hash, DEFAULT_ABBREV);
 		if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
 		    (recurse_submodules != RECURSE_SUBMODULES_ON))
-			check_for_new_submodule_commits(ref->new_oid.hash);
+			check_for_new_submodule_commits(&ref->new_oid);
 		r = s_update_ref("forced-update", ref, 1);
 		format_display(display, r ? '!' : '+', quickref.buf,
 			       r ? _("unable to update local ref") : _("forced update"),
diff --git a/submodule.c b/submodule.c
index 3200b7bb2b..5c5c18ec3d 100644
--- a/submodule.c
+++ b/submodule.c
@@ -821,14 +821,14 @@ static int add_sha1_to_array(const char *ref, const struct object_id *oid,
 	return 0;
 }
 
-void check_for_new_submodule_commits(unsigned char new_sha1[20])
+void check_for_new_submodule_commits(struct object_id *oid)
 {
 	if (!initialized_fetch_ref_tips) {
 		for_each_ref(add_sha1_to_array, &ref_tips_before_fetch);
 		initialized_fetch_ref_tips = 1;
 	}
 
-	sha1_array_append(&ref_tips_after_fetch, new_sha1);
+	sha1_array_append(&ref_tips_after_fetch, oid->hash);
 }
 
 static int add_sha1_to_argv(const unsigned char sha1[20], void *data)
diff --git a/submodule.h b/submodule.h
index c8a0c9cb29..9c32b28b12 100644
--- a/submodule.h
+++ b/submodule.h
@@ -58,7 +58,7 @@ extern void show_submodule_inline_diff(FILE *f, const char *path,
 		const char *del, const char *add, const char *reset,
 		const struct diff_options *opt);
 extern void set_config_fetch_recurse_submodules(int value);
-extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
+extern void check_for_new_submodule_commits(struct object_id *oid);
 extern int fetch_populated_submodules(const struct argv_array *options,
 			       const char *prefix, int command_line_option,
 			       int quiet, int max_parallel_jobs);

[PATCH v2 14/21] builtin/pull: convert to struct object_id

[brian m. carlson]

Convert virtually all uses of unsigned char [20] to struct object_id.
Leave all the arguments that come from struct sha1_array, as these will
be converted in a later patch.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/pull.c | 72 +++++++++++++++++++++++++++++-----------------------------
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/builtin/pull.c b/builtin/pull.c
index a9f7553f30..704ce1f042 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -515,7 +515,7 @@ static int run_fetch(const char *repo, const char **refspecs)
  * "Pulls into void" by branching off merge_head.
  */
 static int pull_into_void(const unsigned char *merge_head,
-		const unsigned char *curr_head)
+		const struct object_id *curr_head)
 {
 	/*
 	 * Two-way merge: we treat the index as based on an empty tree,
@@ -526,7 +526,7 @@ static int pull_into_void(const unsigned char *merge_head,
 	if (checkout_fast_forward(EMPTY_TREE_SHA1_BIN, merge_head, 0))
 		return 1;
 
-	if (update_ref("initial pull", "HEAD", merge_head, curr_head, 0, UPDATE_REFS_DIE_ON_ERR))
+	if (update_ref("initial pull", "HEAD", merge_head, curr_head->hash, 0, UPDATE_REFS_DIE_ON_ERR))
 		return 1;
 
 	return 0;
@@ -647,7 +647,7 @@ static const char *get_tracking_branch(const char *remote, const char *refspec)
  * current branch forked from its remote tracking branch. Returns 0 on success,
  * -1 on failure.
  */
-static int get_rebase_fork_point(unsigned char *fork_point, const char *repo,
+static int get_rebase_fork_point(struct object_id *fork_point, const char *repo,
 		const char *refspec)
 {
 	int ret;
@@ -678,7 +678,7 @@ static int get_rebase_fork_point(unsigned char *fork_point, const char *repo,
 	if (ret)
 		goto cleanup;
 
-	ret = get_sha1_hex(sb.buf, fork_point);
+	ret = get_oid_hex(sb.buf, fork_point);
 	if (ret)
 		goto cleanup;
 
@@ -691,24 +691,24 @@ static int get_rebase_fork_point(unsigned char *fork_point, const char *repo,
  * Sets merge_base to the octopus merge base of curr_head, merge_head and
  * fork_point. Returns 0 if a merge base is found, 1 otherwise.
  */
-static int get_octopus_merge_base(unsigned char *merge_base,
-		const unsigned char *curr_head,
+static int get_octopus_merge_base(struct object_id *merge_base,
+		const struct object_id *curr_head,
 		const unsigned char *merge_head,
-		const unsigned char *fork_point)
+		const struct object_id *fork_point)
 {
 	struct commit_list *revs = NULL, *result;
 
-	commit_list_insert(lookup_commit_reference(curr_head), &revs);
+	commit_list_insert(lookup_commit_reference(curr_head->hash), &revs);
 	commit_list_insert(lookup_commit_reference(merge_head), &revs);
-	if (!is_null_sha1(fork_point))
-		commit_list_insert(lookup_commit_reference(fork_point), &revs);
+	if (!is_null_oid(fork_point))
+		commit_list_insert(lookup_commit_reference(fork_point->hash), &revs);
 
 	result = reduce_heads(get_octopus_merge_bases(revs));
 	free_commit_list(revs);
 	if (!result)
 		return 1;
 
-	hashcpy(merge_base, result->item->object.oid.hash);
+	oidcpy(merge_base, &result->item->object.oid);
 	return 0;
 }
 
@@ -717,16 +717,16 @@ static int get_octopus_merge_base(unsigned char *merge_base,
  * fork point calculated by get_rebase_fork_point(), runs git-rebase with the
  * appropriate arguments and returns its exit status.
  */
-static int run_rebase(const unsigned char *curr_head,
+static int run_rebase(const struct object_id *curr_head,
 		const unsigned char *merge_head,
-		const unsigned char *fork_point)
+		const struct object_id *fork_point)
 {
 	int ret;
-	unsigned char oct_merge_base[GIT_SHA1_RAWSZ];
+	struct object_id oct_merge_base;
 	struct argv_array args = ARGV_ARRAY_INIT;
 
-	if (!get_octopus_merge_base(oct_merge_base, curr_head, merge_head, fork_point))
-		if (!is_null_sha1(fork_point) && !hashcmp(oct_merge_base, fork_point))
+	if (!get_octopus_merge_base(&oct_merge_base, curr_head, merge_head, fork_point))
+		if (!is_null_oid(fork_point) && !oidcmp(&oct_merge_base, fork_point))
 			fork_point = NULL;
 
 	argv_array_push(&args, "rebase");
@@ -756,8 +756,8 @@ static int run_rebase(const unsigned char *curr_head,
 	argv_array_push(&args, "--onto");
 	argv_array_push(&args, sha1_to_hex(merge_head));
 
-	if (fork_point && !is_null_sha1(fork_point))
-		argv_array_push(&args, sha1_to_hex(fork_point));
+	if (fork_point && !is_null_oid(fork_point))
+		argv_array_push(&args, oid_to_hex(fork_point));
 	else
 		argv_array_push(&args, sha1_to_hex(merge_head));
 
@@ -770,8 +770,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 {
 	const char *repo, **refspecs;
 	struct sha1_array merge_heads = SHA1_ARRAY_INIT;
-	unsigned char orig_head[GIT_SHA1_RAWSZ], curr_head[GIT_SHA1_RAWSZ];
-	unsigned char rebase_fork_point[GIT_SHA1_RAWSZ];
+	struct object_id orig_head, curr_head;
+	struct object_id rebase_fork_point;
 
 	if (!getenv("GIT_REFLOG_ACTION"))
 		set_reflog_message(argc, argv);
@@ -794,8 +794,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	if (file_exists(git_path("MERGE_HEAD")))
 		die_conclude_merge();
 
-	if (get_sha1("HEAD", orig_head))
-		hashclr(orig_head);
+	if (get_oid("HEAD", &orig_head))
+		oidclr(&orig_head);
 
 	if (!opt_rebase && opt_autostash != -1)
 		die(_("--[no-]autostash option is only valid with --rebase."));
@@ -805,15 +805,15 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 		if (opt_autostash != -1)
 			autostash = opt_autostash;
 
-		if (is_null_sha1(orig_head) && !is_cache_unborn())
+		if (is_null_oid(&orig_head) && !is_cache_unborn())
 			die(_("Updating an unborn branch with changes added to the index."));
 
 		if (!autostash)
 			require_clean_work_tree(N_("pull with rebase"),
 				_("please commit or stash them."), 1, 0);
 
-		if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs))
-			hashclr(rebase_fork_point);
+		if (get_rebase_fork_point(&rebase_fork_point, repo, *refspecs))
+			oidclr(&rebase_fork_point);
 	}
 
 	if (run_fetch(repo, refspecs))
@@ -822,11 +822,11 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	if (opt_dry_run)
 		return 0;
 
-	if (get_sha1("HEAD", curr_head))
-		hashclr(curr_head);
+	if (get_oid("HEAD", &curr_head))
+		oidclr(&curr_head);
 
-	if (!is_null_sha1(orig_head) && !is_null_sha1(curr_head) &&
-			hashcmp(orig_head, curr_head)) {
+	if (!is_null_oid(&orig_head) && !is_null_oid(&curr_head) &&
+			oidcmp(&orig_head, &curr_head)) {
 		/*
 		 * The fetch involved updating the current branch.
 		 *
@@ -837,15 +837,15 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 		warning(_("fetch updated the current branch head.\n"
 			"fast-forwarding your working tree from\n"
-			"commit %s."), sha1_to_hex(orig_head));
+			"commit %s."), oid_to_hex(&orig_head));
 
-		if (checkout_fast_forward(orig_head, curr_head, 0))
+		if (checkout_fast_forward(orig_head.hash, curr_head.hash, 0))
 			die(_("Cannot fast-forward your working tree.\n"
 				"After making sure that you saved anything precious from\n"
 				"$ git diff %s\n"
 				"output, run\n"
 				"$ git reset --hard\n"
-				"to recover."), sha1_to_hex(orig_head));
+				"to recover."), oid_to_hex(&orig_head));
 	}
 
 	get_merge_heads(&merge_heads);
@@ -853,10 +853,10 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	if (!merge_heads.nr)
 		die_no_merge_candidates(repo, refspecs);
 
-	if (is_null_sha1(orig_head)) {
+	if (is_null_oid(&orig_head)) {
 		if (merge_heads.nr > 1)
 			die(_("Cannot merge multiple branches into empty head."));
-		return pull_into_void(*merge_heads.sha1, curr_head);
+		return pull_into_void(*merge_heads.sha1, &curr_head);
 	}
 	if (opt_rebase && merge_heads.nr > 1)
 		die(_("Cannot rebase onto multiple branches."));
@@ -865,7 +865,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 		struct commit_list *list = NULL;
 		struct commit *merge_head, *head;
 
-		head = lookup_commit_reference(orig_head);
+		head = lookup_commit_reference(orig_head.hash);
 		commit_list_insert(head, &list);
 		merge_head = lookup_commit_reference(merge_heads.sha1[0]);
 		if (is_descendant_of(merge_head, list)) {
@@ -873,7 +873,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 			opt_ff = "--ff-only";
 			return run_merge();
 		}
-		return run_rebase(curr_head, *merge_heads.sha1, rebase_fork_point);
+		return run_rebase(&curr_head, *merge_heads.sha1, &rebase_fork_point);
 	} else {
 		return run_merge();
 	}

[PATCH v2 15/21] sha1-array: convert internal storage for struct sha1_array to object_id

[brian m. carlson]

Make the internal storage for struct sha1_array use an array of struct
object_id internally.  Update the users of this struct which inspect its
internals.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 bisect.c               | 14 +++++++-------
 builtin/pull.c         | 22 +++++++++++-----------
 builtin/receive-pack.c |  4 ++--
 combine-diff.c         |  6 +++---
 fetch-pack.c           | 12 ++++++------
 fsck.c                 |  4 ++--
 remote-curl.c          |  2 +-
 send-pack.c            |  2 +-
 sha1-array.c           | 22 +++++++++++-----------
 sha1-array.h           |  2 +-
 shallow.c              | 26 +++++++++++++-------------
 11 files changed, 58 insertions(+), 58 deletions(-)

diff --git a/bisect.c b/bisect.c
index 21c3e34636..ebaf7b05ba 100644
--- a/bisect.c
+++ b/bisect.c
@@ -457,7 +457,7 @@ static char *join_sha1_array_hex(struct sha1_array *array, char delim)
 	int i;
 
 	for (i = 0; i < array->nr; i++) {
-		strbuf_addstr(&joined_hexs, sha1_to_hex(array->sha1[i]));
+		strbuf_addstr(&joined_hexs, oid_to_hex(array->oid + i));
 		if (i + 1 < array->nr)
 			strbuf_addch(&joined_hexs, delim);
 	}
@@ -621,7 +621,7 @@ static void bisect_rev_setup(struct rev_info *revs, const char *prefix,
 	argv_array_pushf(&rev_argv, bad_format, oid_to_hex(current_bad_oid));
 	for (i = 0; i < good_revs.nr; i++)
 		argv_array_pushf(&rev_argv, good_format,
-				 sha1_to_hex(good_revs.sha1[i]));
+				 oid_to_hex(good_revs.oid + i));
 	argv_array_push(&rev_argv, "--");
 	if (read_paths)
 		read_bisect_paths(&rev_argv);
@@ -701,11 +701,11 @@ static int bisect_checkout(const unsigned char *bisect_rev, int no_checkout)
 	return run_command_v_opt(argv_show_branch, RUN_GIT_CMD);
 }
 
-static struct commit *get_commit_reference(const unsigned char *sha1)
+static struct commit *get_commit_reference(const struct object_id *oid)
 {
-	struct commit *r = lookup_commit_reference(sha1);
+	struct commit *r = lookup_commit_reference(oid->hash);
 	if (!r)
-		die(_("Not a valid commit name %s"), sha1_to_hex(sha1));
+		die(_("Not a valid commit name %s"), oid_to_hex(oid));
 	return r;
 }
 
@@ -715,9 +715,9 @@ static struct commit **get_bad_and_good_commits(int *rev_nr)
 	int i, n = 0;
 
 	ALLOC_ARRAY(rev, 1 + good_revs.nr);
-	rev[n++] = get_commit_reference(current_bad_oid->hash);
+	rev[n++] = get_commit_reference(current_bad_oid);
 	for (i = 0; i < good_revs.nr; i++)
-		rev[n++] = get_commit_reference(good_revs.sha1[i]);
+		rev[n++] = get_commit_reference(good_revs.oid + i);
 	*rev_nr = n;
 
 	return rev;
diff --git a/builtin/pull.c b/builtin/pull.c
index 704ce1f042..c007900ab5 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -514,7 +514,7 @@ static int run_fetch(const char *repo, const char **refspecs)
 /**
  * "Pulls into void" by branching off merge_head.
  */
-static int pull_into_void(const unsigned char *merge_head,
+static int pull_into_void(const struct object_id *merge_head,
 		const struct object_id *curr_head)
 {
 	/*
@@ -523,10 +523,10 @@ static int pull_into_void(const unsigned char *merge_head,
 	 * index/worktree changes that the user already made on the unborn
 	 * branch.
 	 */
-	if (checkout_fast_forward(EMPTY_TREE_SHA1_BIN, merge_head, 0))
+	if (checkout_fast_forward(EMPTY_TREE_SHA1_BIN, merge_head->hash, 0))
 		return 1;
 
-	if (update_ref("initial pull", "HEAD", merge_head, curr_head->hash, 0, UPDATE_REFS_DIE_ON_ERR))
+	if (update_ref("initial pull", "HEAD", merge_head->hash, curr_head->hash, 0, UPDATE_REFS_DIE_ON_ERR))
 		return 1;
 
 	return 0;
@@ -693,13 +693,13 @@ static int get_rebase_fork_point(struct object_id *fork_point, const char *repo,
  */
 static int get_octopus_merge_base(struct object_id *merge_base,
 		const struct object_id *curr_head,
-		const unsigned char *merge_head,
+		const struct object_id *merge_head,
 		const struct object_id *fork_point)
 {
 	struct commit_list *revs = NULL, *result;
 
 	commit_list_insert(lookup_commit_reference(curr_head->hash), &revs);
-	commit_list_insert(lookup_commit_reference(merge_head), &revs);
+	commit_list_insert(lookup_commit_reference(merge_head->hash), &revs);
 	if (!is_null_oid(fork_point))
 		commit_list_insert(lookup_commit_reference(fork_point->hash), &revs);
 
@@ -718,7 +718,7 @@ static int get_octopus_merge_base(struct object_id *merge_base,
  * appropriate arguments and returns its exit status.
  */
 static int run_rebase(const struct object_id *curr_head,
-		const unsigned char *merge_head,
+		const struct object_id *merge_head,
 		const struct object_id *fork_point)
 {
 	int ret;
@@ -754,12 +754,12 @@ static int run_rebase(const struct object_id *curr_head,
 		warning(_("ignoring --verify-signatures for rebase"));
 
 	argv_array_push(&args, "--onto");
-	argv_array_push(&args, sha1_to_hex(merge_head));
+	argv_array_push(&args, oid_to_hex(merge_head));
 
 	if (fork_point && !is_null_oid(fork_point))
 		argv_array_push(&args, oid_to_hex(fork_point));
 	else
-		argv_array_push(&args, sha1_to_hex(merge_head));
+		argv_array_push(&args, oid_to_hex(merge_head));
 
 	ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
 	argv_array_clear(&args);
@@ -856,7 +856,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 	if (is_null_oid(&orig_head)) {
 		if (merge_heads.nr > 1)
 			die(_("Cannot merge multiple branches into empty head."));
-		return pull_into_void(*merge_heads.sha1, &curr_head);
+		return pull_into_void(merge_heads.oid, &curr_head);
 	}
 	if (opt_rebase && merge_heads.nr > 1)
 		die(_("Cannot rebase onto multiple branches."));
@@ -867,13 +867,13 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
 
 		head = lookup_commit_reference(orig_head.hash);
 		commit_list_insert(head, &list);
-		merge_head = lookup_commit_reference(merge_heads.sha1[0]);
+		merge_head = lookup_commit_reference(merge_heads.oid[0].hash);
 		if (is_descendant_of(merge_head, list)) {
 			/* we can fast-forward this without invoking rebase */
 			opt_ff = "--ff-only";
 			return run_merge();
 		}
-		return run_rebase(&curr_head, *merge_heads.sha1, &rebase_fork_point);
+		return run_rebase(&curr_head, merge_heads.oid, &rebase_fork_point);
 	} else {
 		return run_merge();
 	}
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index e3dc3e184d..85046607fe 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -842,7 +842,7 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
 		if (si->used_shallow[i] &&
 		    (si->used_shallow[i][cmd->index / 32] & mask) &&
 		    !delayed_reachability_test(si, i))
-			sha1_array_append(&extra, si->shallow->sha1[i]);
+			sha1_array_append(&extra, si->shallow->oid[i].hash);
 
 	opt.env = tmp_objdir_env(tmp_objdir);
 	setup_alternate_shallow(&shallow_lock, &opt.shallow_file, &extra);
@@ -859,7 +859,7 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
 	 * not lose these new roots..
 	 */
 	for (i = 0; i < extra.nr; i++)
-		register_shallow(extra.sha1[i]);
+		register_shallow(extra.oid[i].hash);
 
 	si->shallow_ref[cmd->index] = 0;
 	sha1_array_clear(&extra);
diff --git a/combine-diff.c b/combine-diff.c
index 59501db99a..a5b86d7eb9 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1335,7 +1335,7 @@ static struct combine_diff_path *find_paths_generic(const unsigned char *sha1,
 			opt->output_format = stat_opt;
 		else
 			opt->output_format = DIFF_FORMAT_NO_OUTPUT;
-		diff_tree_sha1(parents->sha1[i], sha1, "", opt);
+		diff_tree_sha1(parents->oid[i].hash, sha1, "", opt);
 		diffcore_std(opt);
 		paths = intersect_paths(paths, i, num_parent);
 
@@ -1369,7 +1369,7 @@ static struct combine_diff_path *find_paths_multitree(
 
 	ALLOC_ARRAY(parents_sha1, nparent);
 	for (i = 0; i < nparent; i++)
-		parents_sha1[i] = parents->sha1[i];
+		parents_sha1[i] = parents->oid[i].hash;
 
 	/* fake list head, so worker can assume it is non-NULL */
 	paths_head.next = NULL;
@@ -1462,7 +1462,7 @@ void diff_tree_combined(const unsigned char *sha1,
 		if (stat_opt) {
 			diffopts.output_format = stat_opt;
 
-			diff_tree_sha1(parents->sha1[0], sha1, "", &diffopts);
+			diff_tree_sha1(parents->oid[0].hash, sha1, "", &diffopts);
 			diffcore_std(&diffopts);
 			if (opt->orderfile)
 				diffcore_order(opt->orderfile);
diff --git a/fetch-pack.c b/fetch-pack.c
index d07d85ce30..543e8aa9e1 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1039,10 +1039,10 @@ static void update_shallow(struct fetch_pack_args *args,
 		 * after get_pack() and reprepare_packed_git())
 		 */
 		struct sha1_array extra = SHA1_ARRAY_INIT;
-		unsigned char (*sha1)[20] = si->shallow->sha1;
+		struct object_id *oid = si->shallow->oid;
 		for (i = 0; i < si->shallow->nr; i++)
-			if (has_sha1_file(sha1[i]))
-				sha1_array_append(&extra, sha1[i]);
+			if (has_object_file(&oid[i]))
+				sha1_array_append(&extra, oid[i].hash);
 		if (extra.nr) {
 			setup_alternate_shallow(&shallow_lock,
 						&alternate_shallow_file,
@@ -1071,16 +1071,16 @@ static void update_shallow(struct fetch_pack_args *args,
 		 * refs.
 		 */
 		struct sha1_array extra = SHA1_ARRAY_INIT;
-		unsigned char (*sha1)[20] = si->shallow->sha1;
+		struct object_id *oid = si->shallow->oid;
 		assign_shallow_commits_to_refs(si, NULL, NULL);
 		if (!si->nr_ours && !si->nr_theirs) {
 			sha1_array_clear(&ref);
 			return;
 		}
 		for (i = 0; i < si->nr_ours; i++)
-			sha1_array_append(&extra, sha1[si->ours[i]]);
+			sha1_array_append(&extra, oid[si->ours[i]].hash);
 		for (i = 0; i < si->nr_theirs; i++)
-			sha1_array_append(&extra, sha1[si->theirs[i]]);
+			sha1_array_append(&extra, oid[si->theirs[i]].hash);
 		setup_alternate_shallow(&shallow_lock,
 					&alternate_shallow_file,
 					&extra);
diff --git a/fsck.c b/fsck.c
index aff4ae6fd4..8f41e692bb 100644
--- a/fsck.c
+++ b/fsck.c
@@ -158,8 +158,8 @@ static void init_skiplist(struct fsck_options *options, const char *path)
 			die("Invalid SHA-1: %s", buffer);
 		sha1_array_append(&skiplist, oid.hash);
 		if (sorted && skiplist.nr > 1 &&
-				hashcmp(skiplist.sha1[skiplist.nr - 2],
-					oid.hash) > 0)
+				oidcmp(&skiplist.oid[skiplist.nr - 2],
+				       &oid) > 0)
 			sorted = 0;
 	}
 	close(fd);
diff --git a/remote-curl.c b/remote-curl.c
index 34a97e7328..5e712e4aa1 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -230,7 +230,7 @@ static void free_discovery(struct discovery *d)
 	if (d) {
 		if (d == last_discovery)
 			last_discovery = NULL;
-		free(d->shallow.sha1);
+		free(d->shallow.oid);
 		free(d->buf_alloc);
 		free_refs(d->refs);
 		free(d);
diff --git a/send-pack.c b/send-pack.c
index d2d2a49a02..b3040391cd 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -98,7 +98,7 @@ static int pack_objects(int fd, struct ref *refs, struct sha1_array *extra, stru
 	 */
 	po_in = xfdopen(po.in, "w");
 	for (i = 0; i < extra->nr; i++)
-		feed_object(extra->sha1[i], po_in, 1);
+		feed_object(extra->oid[i].hash, po_in, 1);
 
 	while (refs) {
 		if (!is_null_oid(&refs->old_oid))
diff --git a/sha1-array.c b/sha1-array.c
index c1cc25cd95..093d158003 100644
--- a/sha1-array.c
+++ b/sha1-array.c
@@ -4,39 +4,39 @@
 
 void sha1_array_append(struct sha1_array *array, const unsigned char *sha1)
 {
-	ALLOC_GROW(array->sha1, array->nr + 1, array->alloc);
-	hashcpy(array->sha1[array->nr++], sha1);
+	ALLOC_GROW(array->oid, array->nr + 1, array->alloc);
+	hashcpy(array->oid[array->nr++].hash, sha1);
 	array->sorted = 0;
 }
 
 static int void_hashcmp(const void *a, const void *b)
 {
-	return hashcmp(a, b);
+	return oidcmp(a, b);
 }
 
 static void sha1_array_sort(struct sha1_array *array)
 {
-	QSORT(array->sha1, array->nr, void_hashcmp);
+	QSORT(array->oid, array->nr, void_hashcmp);
 	array->sorted = 1;
 }
 
 static const unsigned char *sha1_access(size_t index, void *table)
 {
-	unsigned char (*array)[20] = table;
-	return array[index];
+	struct object_id *array = table;
+	return array[index].hash;
 }
 
 int sha1_array_lookup(struct sha1_array *array, const unsigned char *sha1)
 {
 	if (!array->sorted)
 		sha1_array_sort(array);
-	return sha1_pos(sha1, array->sha1, array->nr, sha1_access);
+	return sha1_pos(sha1, array->oid, array->nr, sha1_access);
 }
 
 void sha1_array_clear(struct sha1_array *array)
 {
-	free(array->sha1);
-	array->sha1 = NULL;
+	free(array->oid);
+	array->oid = NULL;
 	array->nr = 0;
 	array->alloc = 0;
 	array->sorted = 0;
@@ -53,9 +53,9 @@ int sha1_array_for_each_unique(struct sha1_array *array,
 
 	for (i = 0; i < array->nr; i++) {
 		int ret;
-		if (i > 0 && !hashcmp(array->sha1[i], array->sha1[i-1]))
+		if (i > 0 && !oidcmp(array->oid + i, array->oid + i - 1))
 			continue;
-		ret = fn(array->sha1[i], data);
+		ret = fn(array->oid[i].hash, data);
 		if (ret)
 			return ret;
 	}
diff --git a/sha1-array.h b/sha1-array.h
index b3230be0dd..c1f706acba 100644
--- a/sha1-array.h
+++ b/sha1-array.h
@@ -2,7 +2,7 @@
 #define SHA1_ARRAY_H
 
 struct sha1_array {
-	unsigned char (*sha1)[20];
+	struct object_id *oid;
 	int nr;
 	int alloc;
 	int sorted;
diff --git a/shallow.c b/shallow.c
index 11f7dde9d9..dc7b67a294 100644
--- a/shallow.c
+++ b/shallow.c
@@ -273,7 +273,7 @@ static int write_shallow_commits_1(struct strbuf *out, int use_pack_protocol,
 	if (!extra)
 		return data.count;
 	for (i = 0; i < extra->nr; i++) {
-		strbuf_addstr(out, sha1_to_hex(extra->sha1[i]));
+		strbuf_addstr(out, oid_to_hex(extra->oid + i));
 		strbuf_addch(out, '\n');
 		data.count++;
 	}
@@ -396,9 +396,9 @@ void prepare_shallow_info(struct shallow_info *info, struct sha1_array *sa)
 	ALLOC_ARRAY(info->ours, sa->nr);
 	ALLOC_ARRAY(info->theirs, sa->nr);
 	for (i = 0; i < sa->nr; i++) {
-		if (has_sha1_file(sa->sha1[i])) {
+		if (has_object_file(sa->oid + i)) {
 			struct commit_graft *graft;
-			graft = lookup_commit_graft(sa->sha1[i]);
+			graft = lookup_commit_graft(sa->oid[i].hash);
 			if (graft && graft->nr_parent < 0)
 				continue;
 			info->ours[info->nr_ours++] = i;
@@ -417,13 +417,13 @@ void clear_shallow_info(struct shallow_info *info)
 
 void remove_nonexistent_theirs_shallow(struct shallow_info *info)
 {
-	unsigned char (*sha1)[20] = info->shallow->sha1;
+	struct object_id *oid = info->shallow->oid;
 	int i, dst;
 	trace_printf_key(&trace_shallow, "shallow: remove_nonexistent_theirs_shallow\n");
 	for (i = dst = 0; i < info->nr_theirs; i++) {
 		if (i != dst)
 			info->theirs[dst] = info->theirs[i];
-		if (has_sha1_file(sha1[info->theirs[i]]))
+		if (has_object_file(oid + info->theirs[i]))
 			dst++;
 	}
 	info->nr_theirs = dst;
@@ -559,7 +559,7 @@ static void post_assign_shallow(struct shallow_info *info,
 void assign_shallow_commits_to_refs(struct shallow_info *info,
 				    uint32_t **used, int *ref_status)
 {
-	unsigned char (*sha1)[20] = info->shallow->sha1;
+	struct object_id *oid = info->shallow->oid;
 	struct sha1_array *ref = info->ref;
 	unsigned int i, nr;
 	int *shallow, nr_shallow = 0;
@@ -599,18 +599,18 @@ void assign_shallow_commits_to_refs(struct shallow_info *info,
 
 	/* Mark potential bottoms so we won't go out of bound */
 	for (i = 0; i < nr_shallow; i++) {
-		struct commit *c = lookup_commit(sha1[shallow[i]]);
+		struct commit *c = lookup_commit(oid[shallow[i]].hash);
 		c->object.flags |= BOTTOM;
 	}
 
 	for (i = 0; i < ref->nr; i++)
-		paint_down(&pi, ref->sha1[i], i);
+		paint_down(&pi, ref->oid[i].hash, i);
 
 	if (used) {
 		int bitmap_size = ((pi.nr_bits + 31) / 32) * sizeof(uint32_t);
 		memset(used, 0, sizeof(*used) * info->shallow->nr);
 		for (i = 0; i < nr_shallow; i++) {
-			const struct commit *c = lookup_commit(sha1[shallow[i]]);
+			const struct commit *c = lookup_commit(oid[shallow[i]].hash);
 			uint32_t **map = ref_bitmap_at(&pi.ref_bitmap, c);
 			if (*map)
 				used[shallow[i]] = xmemdupz(*map, bitmap_size);
@@ -664,7 +664,7 @@ static void post_assign_shallow(struct shallow_info *info,
 				struct ref_bitmap *ref_bitmap,
 				int *ref_status)
 {
-	unsigned char (*sha1)[20] = info->shallow->sha1;
+	struct object_id *oid = info->shallow->oid;
 	struct commit *c;
 	uint32_t **bitmap;
 	int dst, i, j;
@@ -679,7 +679,7 @@ static void post_assign_shallow(struct shallow_info *info,
 	for (i = dst = 0; i < info->nr_theirs; i++) {
 		if (i != dst)
 			info->theirs[dst] = info->theirs[i];
-		c = lookup_commit(sha1[info->theirs[i]]);
+		c = lookup_commit(oid[info->theirs[i]].hash);
 		bitmap = ref_bitmap_at(ref_bitmap, c);
 		if (!*bitmap)
 			continue;
@@ -700,7 +700,7 @@ static void post_assign_shallow(struct shallow_info *info,
 	for (i = dst = 0; i < info->nr_ours; i++) {
 		if (i != dst)
 			info->ours[dst] = info->ours[i];
-		c = lookup_commit(sha1[info->ours[i]]);
+		c = lookup_commit(oid[info->ours[i]].hash);
 		bitmap = ref_bitmap_at(ref_bitmap, c);
 		if (!*bitmap)
 			continue;
@@ -722,7 +722,7 @@ static void post_assign_shallow(struct shallow_info *info,
 int delayed_reachability_test(struct shallow_info *si, int c)
 {
 	if (si->need_reachability_test[c]) {
-		struct commit *commit = lookup_commit(si->shallow->sha1[c]);
+		struct commit *commit = lookup_commit(si->shallow->oid[c].hash);
 
 		if (!si->commits) {
 			struct commit_array ca;

[PATCH v2 16/21] Make sha1_array_append take a struct object_id *

[brian m. carlson]

Convert the callers to pass struct object_id by changing the function
declaration and definition and applying the following semantic patch:

@@
expression E1, E2, E3;
@@
- sha1_array_append(E1, E2[E3].hash)
+ sha1_array_append(E1, E2 + E3)

@@
expression E1, E2;
@@
- sha1_array_append(E1, E2.hash)
+ sha1_array_append(E1, &E2)

@@
expression E1, E2;
@@
- sha1_array_append(E1, E2->hash)
+ sha1_array_append(E1, E2)

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 bisect.c                   | 4 ++--
 builtin/cat-file.c         | 4 ++--
 builtin/diff.c             | 2 +-
 builtin/pack-objects.c     | 4 ++--
 builtin/pull.c             | 2 +-
 builtin/receive-pack.c     | 6 +++---
 combine-diff.c             | 2 +-
 connect.c                  | 4 ++--
 fetch-pack.c               | 8 ++++----
 fsck.c                     | 2 +-
 parse-options-cb.c         | 2 +-
 sha1-array.c               | 4 ++--
 sha1-array.h               | 2 +-
 sha1_name.c                | 2 +-
 submodule.c                | 6 +++---
 t/helper/test-sha1-array.c | 2 +-
 transport.c                | 6 ++++--
 17 files changed, 32 insertions(+), 30 deletions(-)

diff --git a/bisect.c b/bisect.c
index ebaf7b05ba..886e630884 100644
--- a/bisect.c
+++ b/bisect.c
@@ -413,9 +413,9 @@ static int register_ref(const char *refname, const struct object_id *oid,
 		current_bad_oid = xmalloc(sizeof(*current_bad_oid));
 		oidcpy(current_bad_oid, oid);
 	} else if (starts_with(refname, good_prefix.buf)) {
-		sha1_array_append(&good_revs, oid->hash);
+		sha1_array_append(&good_revs, oid);
 	} else if (starts_with(refname, "skip-")) {
-		sha1_array_append(&skipped_revs, oid->hash);
+		sha1_array_append(&skipped_revs, oid);
 	}
 
 	strbuf_release(&good_prefix);
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 8b85cb8cf0..8fbb667170 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -413,7 +413,7 @@ static int batch_loose_object(const struct object_id *oid,
 			      const char *path,
 			      void *data)
 {
-	sha1_array_append(data, oid->hash);
+	sha1_array_append(data, oid);
 	return 0;
 }
 
@@ -422,7 +422,7 @@ static int batch_packed_object(const struct object_id *oid,
 			       uint32_t pos,
 			       void *data)
 {
-	sha1_array_append(data, oid->hash);
+	sha1_array_append(data, oid);
 	return 0;
 }
 
diff --git a/builtin/diff.c b/builtin/diff.c
index 398eee00d5..a5b34eb156 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -193,7 +193,7 @@ static int builtin_diff_combined(struct rev_info *revs,
 	if (!revs->dense_combined_merges && !revs->combine_merges)
 		revs->dense_combined_merges = revs->combine_merges = 1;
 	for (i = 1; i < ents; i++)
-		sha1_array_append(&parents, ent[i].item->oid.hash);
+		sha1_array_append(&parents, &ent[i].item->oid);
 	diff_tree_combined(ent[0].item->oid.hash, &parents,
 			   revs->dense_combined_merges, revs);
 	sha1_array_clear(&parents);
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 16517f2637..dfeacd5c37 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2739,12 +2739,12 @@ static void record_recent_object(struct object *obj,
 				 const char *name,
 				 void *data)
 {
-	sha1_array_append(&recent_objects, obj->oid.hash);
+	sha1_array_append(&recent_objects, &obj->oid);
 }
 
 static void record_recent_commit(struct commit *commit, void *data)
 {
-	sha1_array_append(&recent_objects, commit->object.oid.hash);
+	sha1_array_append(&recent_objects, &commit->object.oid);
 }
 
 static void get_object_list(int ac, const char **av)
diff --git a/builtin/pull.c b/builtin/pull.c
index c007900ab5..183e377147 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -344,7 +344,7 @@ static void get_merge_heads(struct sha1_array *merge_heads)
 			continue;  /* invalid line: does not start with SHA1 */
 		if (starts_with(sb.buf + GIT_SHA1_HEXSZ, "\tnot-for-merge\t"))
 			continue;  /* ref is not-for-merge */
-		sha1_array_append(merge_heads, oid.hash);
+		sha1_array_append(merge_heads, &oid);
 	}
 	fclose(fp);
 	strbuf_release(&sb);
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 85046607fe..56d1a59922 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -842,7 +842,7 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
 		if (si->used_shallow[i] &&
 		    (si->used_shallow[i][cmd->index / 32] & mask) &&
 		    !delayed_reachability_test(si, i))
-			sha1_array_append(&extra, si->shallow->oid[i].hash);
+			sha1_array_append(&extra, si->shallow->oid + i);
 
 	opt.env = tmp_objdir_env(tmp_objdir);
 	setup_alternate_shallow(&shallow_lock, &opt.shallow_file, &extra);
@@ -1546,7 +1546,7 @@ static struct command *read_head_info(struct sha1_array *shallow)
 			if (get_oid_hex(line + 8, &oid))
 				die("protocol error: expected shallow sha, got '%s'",
 				    line + 8);
-			sha1_array_append(shallow, oid.hash);
+			sha1_array_append(shallow, &oid);
 			continue;
 		}
 
@@ -1817,7 +1817,7 @@ static void update_shallow_info(struct command *commands,
 	for (cmd = commands; cmd; cmd = cmd->next) {
 		if (is_null_oid(&cmd->new_oid))
 			continue;
-		sha1_array_append(ref, cmd->new_oid.hash);
+		sha1_array_append(ref, &cmd->new_oid);
 		cmd->index = ref->nr - 1;
 	}
 	si->ref = ref;
diff --git a/combine-diff.c b/combine-diff.c
index a5b86d7eb9..c92029484c 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1535,7 +1535,7 @@ void diff_tree_combined_merge(const struct commit *commit, int dense,
 	struct sha1_array parents = SHA1_ARRAY_INIT;
 
 	while (parent) {
-		sha1_array_append(&parents, parent->item->object.oid.hash);
+		sha1_array_append(&parents, &parent->item->object.oid);
 		parent = parent->next;
 	}
 	diff_tree_combined(commit->object.oid.hash, &parents, dense, rev);
diff --git a/connect.c b/connect.c
index 7d65c1c736..50b503da0d 100644
--- a/connect.c
+++ b/connect.c
@@ -153,7 +153,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
 				die("protocol error: expected shallow sha-1, got '%s'", arg);
 			if (!shallow_points)
 				die("repository on the other end cannot be shallow");
-			sha1_array_append(shallow_points, old_oid.hash);
+			sha1_array_append(shallow_points, &old_oid);
 			continue;
 		}
 
@@ -169,7 +169,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
 		}
 
 		if (extra_have && !strcmp(name, ".have")) {
-			sha1_array_append(extra_have, old_oid.hash);
+			sha1_array_append(extra_have, &old_oid);
 			continue;
 		}
 
diff --git a/fetch-pack.c b/fetch-pack.c
index 543e8aa9e1..f4bbd2892a 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1042,7 +1042,7 @@ static void update_shallow(struct fetch_pack_args *args,
 		struct object_id *oid = si->shallow->oid;
 		for (i = 0; i < si->shallow->nr; i++)
 			if (has_object_file(&oid[i]))
-				sha1_array_append(&extra, oid[i].hash);
+				sha1_array_append(&extra, oid + i);
 		if (extra.nr) {
 			setup_alternate_shallow(&shallow_lock,
 						&alternate_shallow_file,
@@ -1060,7 +1060,7 @@ static void update_shallow(struct fetch_pack_args *args,
 	if (!si->nr_ours && !si->nr_theirs)
 		return;
 	for (i = 0; i < nr_sought; i++)
-		sha1_array_append(&ref, sought[i]->old_oid.hash);
+		sha1_array_append(&ref, &sought[i]->old_oid);
 	si->ref = &ref;
 
 	if (args->update_shallow) {
@@ -1078,9 +1078,9 @@ static void update_shallow(struct fetch_pack_args *args,
 			return;
 		}
 		for (i = 0; i < si->nr_ours; i++)
-			sha1_array_append(&extra, oid[si->ours[i]].hash);
+			sha1_array_append(&extra, oid + si->ours[i]);
 		for (i = 0; i < si->nr_theirs; i++)
-			sha1_array_append(&extra, oid[si->theirs[i]].hash);
+			sha1_array_append(&extra, oid + si->theirs[i]);
 		setup_alternate_shallow(&shallow_lock,
 					&alternate_shallow_file,
 					&extra);
diff --git a/fsck.c b/fsck.c
index 8f41e692bb..6682de1de5 100644
--- a/fsck.c
+++ b/fsck.c
@@ -156,7 +156,7 @@ static void init_skiplist(struct fsck_options *options, const char *path)
 			break;
 		if (parse_oid_hex(buffer, &oid, &p) || *p != '\n')
 			die("Invalid SHA-1: %s", buffer);
-		sha1_array_append(&skiplist, oid.hash);
+		sha1_array_append(&skiplist, &oid);
 		if (sorted && skiplist.nr > 1 &&
 				oidcmp(&skiplist.oid[skiplist.nr - 2],
 				       &oid) > 0)
diff --git a/parse-options-cb.c b/parse-options-cb.c
index 40ece4d8c2..7baecdc864 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -106,7 +106,7 @@ int parse_opt_object_name(const struct option *opt, const char *arg, int unset)
 		return -1;
 	if (get_oid(arg, &oid))
 		return error(_("malformed object name '%s'"), arg);
-	sha1_array_append(opt->value, oid.hash);
+	sha1_array_append(opt->value, &oid);
 	return 0;
 }
 
diff --git a/sha1-array.c b/sha1-array.c
index 093d158003..26e596b264 100644
--- a/sha1-array.c
+++ b/sha1-array.c
@@ -2,10 +2,10 @@
 #include "sha1-array.h"
 #include "sha1-lookup.h"
 
-void sha1_array_append(struct sha1_array *array, const unsigned char *sha1)
+void sha1_array_append(struct sha1_array *array, const struct object_id *oid)
 {
 	ALLOC_GROW(array->oid, array->nr + 1, array->alloc);
-	hashcpy(array->oid[array->nr++].hash, sha1);
+	oidcpy(&array->oid[array->nr++], oid);
 	array->sorted = 0;
 }
 
diff --git a/sha1-array.h b/sha1-array.h
index c1f706acba..7b06fbf1c1 100644
--- a/sha1-array.h
+++ b/sha1-array.h
@@ -10,7 +10,7 @@ struct sha1_array {
 
 #define SHA1_ARRAY_INIT { NULL, 0, 0, 0 }
 
-void sha1_array_append(struct sha1_array *array, const unsigned char *sha1);
+void sha1_array_append(struct sha1_array *array, const struct object_id *sha1);
 int sha1_array_lookup(struct sha1_array *array, const unsigned char *sha1);
 void sha1_array_clear(struct sha1_array *array);
 
diff --git a/sha1_name.c b/sha1_name.c
index 2e38aedfa5..1316832d73 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -428,7 +428,7 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,
 
 static int collect_ambiguous(const struct object_id *oid, void *data)
 {
-	sha1_array_append(data, oid->hash);
+	sha1_array_append(data, oid);
 	return 0;
 }
 
diff --git a/submodule.c b/submodule.c
index 5c5c18ec3d..7912cba4f6 100644
--- a/submodule.c
+++ b/submodule.c
@@ -650,7 +650,7 @@ static void collect_submodules_from_diff(struct diff_queue_struct *q,
 		if (!S_ISGITLINK(p->two->mode))
 			continue;
 		commits = submodule_commits(submodules, p->two->path);
-		sha1_array_append(commits, p->two->oid.hash);
+		sha1_array_append(commits, &p->two->oid);
 	}
 }
 
@@ -817,7 +817,7 @@ static void submodule_collect_changed_cb(struct diff_queue_struct *q,
 static int add_sha1_to_array(const char *ref, const struct object_id *oid,
 			     int flags, void *data)
 {
-	sha1_array_append(data, oid->hash);
+	sha1_array_append(data, oid);
 	return 0;
 }
 
@@ -828,7 +828,7 @@ void check_for_new_submodule_commits(struct object_id *oid)
 		initialized_fetch_ref_tips = 1;
 	}
 
-	sha1_array_append(&ref_tips_after_fetch, oid->hash);
+	sha1_array_append(&ref_tips_after_fetch, oid);
 }
 
 static int add_sha1_to_argv(const unsigned char sha1[20], void *data)
diff --git a/t/helper/test-sha1-array.c b/t/helper/test-sha1-array.c
index b4bb97fccc..181c36e0a5 100644
--- a/t/helper/test-sha1-array.c
+++ b/t/helper/test-sha1-array.c
@@ -19,7 +19,7 @@ int cmd_main(int argc, const char **argv)
 		if (skip_prefix(line.buf, "append ", &arg)) {
 			if (get_oid_hex(arg, &oid))
 				die("not a hexadecimal SHA1: %s", arg);
-			sha1_array_append(&array, oid.hash);
+			sha1_array_append(&array, &oid);
 		} else if (skip_prefix(line.buf, "lookup ", &arg)) {
 			if (get_oid_hex(arg, &oid))
 				die("not a hexadecimal SHA1: %s", arg);
diff --git a/transport.c b/transport.c
index 8a90b0c29b..e492757726 100644
--- a/transport.c
+++ b/transport.c
@@ -1027,7 +1027,8 @@ int transport_push(struct transport *transport,
 
 			for (; ref; ref = ref->next)
 				if (!is_null_oid(&ref->new_oid))
-					sha1_array_append(&commits, ref->new_oid.hash);
+					sha1_array_append(&commits,
+							  &ref->new_oid);
 
 			if (!push_unpushed_submodules(&commits,
 						      transport->remote->name,
@@ -1048,7 +1049,8 @@ int transport_push(struct transport *transport,
 
 			for (; ref; ref = ref->next)
 				if (!is_null_oid(&ref->new_oid))
-					sha1_array_append(&commits, ref->new_oid.hash);
+					sha1_array_append(&commits,
+							  &ref->new_oid);
 
 			if (find_unpushed_submodules(&commits, transport->remote->name,
 						&needs_pushing)) {

[PATCH v2 17/21] Convert remaining callers of sha1_array_lookup to object_id

[brian m. carlson]

There are a very small number of callers which don't already use struct
object_id.  Convert them.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 bisect.c               | 14 +++++++-------
 builtin/pack-objects.c | 16 ++++++++--------
 ref-filter.c           | 22 +++++++++++-----------
 3 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/bisect.c b/bisect.c
index 886e630884..a25d008693 100644
--- a/bisect.c
+++ b/bisect.c
@@ -754,9 +754,9 @@ static void handle_bad_merge_base(void)
 	exit(1);
 }
 
-static void handle_skipped_merge_base(const unsigned char *mb)
+static void handle_skipped_merge_base(const struct object_id *mb)
 {
-	char *mb_hex = sha1_to_hex(mb);
+	char *mb_hex = oid_to_hex(mb);
 	char *bad_hex = oid_to_hex(current_bad_oid);
 	char *good_hex = join_sha1_array_hex(&good_revs, ' ');
 
@@ -787,16 +787,16 @@ static void check_merge_bases(int no_checkout)
 	result = get_merge_bases_many(rev[0], rev_nr - 1, rev + 1);
 
 	for (; result; result = result->next) {
-		const unsigned char *mb = result->item->object.oid.hash;
-		if (!hashcmp(mb, current_bad_oid->hash)) {
+		const struct object_id *mb = &result->item->object.oid;
+		if (!oidcmp(mb, current_bad_oid)) {
 			handle_bad_merge_base();
-		} else if (0 <= sha1_array_lookup(&good_revs, mb)) {
+		} else if (0 <= sha1_array_lookup(&good_revs, mb->hash)) {
 			continue;
-		} else if (0 <= sha1_array_lookup(&skipped_revs, mb)) {
+		} else if (0 <= sha1_array_lookup(&skipped_revs, mb->hash)) {
 			handle_skipped_merge_base(mb);
 		} else {
 			printf(_("Bisecting: a merge base must be tested\n"));
-			exit(bisect_checkout(mb, no_checkout));
+			exit(bisect_checkout(mb->hash, no_checkout));
 		}
 	}
 
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index dfeacd5c37..dca1b68e69 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2670,14 +2670,14 @@ static int has_sha1_pack_kept_or_nonlocal(const unsigned char *sha1)
  */
 static struct sha1_array recent_objects;
 
-static int loosened_object_can_be_discarded(const unsigned char *sha1,
+static int loosened_object_can_be_discarded(const struct object_id *oid,
 					    unsigned long mtime)
 {
 	if (!unpack_unreachable_expiration)
 		return 0;
 	if (mtime > unpack_unreachable_expiration)
 		return 0;
-	if (sha1_array_lookup(&recent_objects, sha1) >= 0)
+	if (sha1_array_lookup(&recent_objects, oid->hash) >= 0)
 		return 0;
 	return 1;
 }
@@ -2686,7 +2686,7 @@ static void loosen_unused_packed_objects(struct rev_info *revs)
 {
 	struct packed_git *p;
 	uint32_t i;
-	const unsigned char *sha1;
+	struct object_id oid;
 
 	for (p = packed_git; p; p = p->next) {
 		if (!p->pack_local || p->pack_keep)
@@ -2696,11 +2696,11 @@ static void loosen_unused_packed_objects(struct rev_info *revs)
 			die("cannot open pack index");
 
 		for (i = 0; i < p->num_objects; i++) {
-			sha1 = nth_packed_object_sha1(p, i);
-			if (!packlist_find(&to_pack, sha1, NULL) &&
-			    !has_sha1_pack_kept_or_nonlocal(sha1) &&
-			    !loosened_object_can_be_discarded(sha1, p->mtime))
-				if (force_object_loose(sha1, p->mtime))
+			nth_packed_object_oid(&oid, p, i);
+			if (!packlist_find(&to_pack, oid.hash, NULL) &&
+			    !has_sha1_pack_kept_or_nonlocal(oid.hash) &&
+			    !loosened_object_can_be_discarded(&oid, p->mtime))
+				if (force_object_loose(oid.hash, p->mtime))
 					die("unable to force loose object");
 		}
 	}
diff --git a/ref-filter.c b/ref-filter.c
index 9c82b5b9d6..d3dcb53dd5 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1677,22 +1677,22 @@ static int filter_pattern_match(struct ref_filter *filter, const char *refname)
  * the need to parse the object via parse_object(). peel_ref() might be a
  * more efficient alternative to obtain the pointee.
  */
-static const unsigned char *match_points_at(struct sha1_array *points_at,
-					    const unsigned char *sha1,
-					    const char *refname)
+static const struct object_id *match_points_at(struct sha1_array *points_at,
+					       const struct object_id *oid,
+					       const char *refname)
 {
-	const unsigned char *tagged_sha1 = NULL;
+	const struct object_id *tagged_oid = NULL;
 	struct object *obj;
 
-	if (sha1_array_lookup(points_at, sha1) >= 0)
-		return sha1;
-	obj = parse_object(sha1);
+	if (sha1_array_lookup(points_at, oid->hash) >= 0)
+		return oid;
+	obj = parse_object(oid->hash);
 	if (!obj)
 		die(_("malformed object at '%s'"), refname);
 	if (obj->type == OBJ_TAG)
-		tagged_sha1 = ((struct tag *)obj)->tagged->oid.hash;
-	if (tagged_sha1 && sha1_array_lookup(points_at, tagged_sha1) >= 0)
-		return tagged_sha1;
+		tagged_oid = &((struct tag *)obj)->tagged->oid;
+	if (tagged_oid && sha1_array_lookup(points_at, tagged_oid->hash) >= 0)
+		return tagged_oid;
 	return NULL;
 }
 
@@ -1772,7 +1772,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
 	if (!filter_pattern_match(filter, refname))
 		return 0;
 
-	if (filter->points_at.nr && !match_points_at(&filter->points_at, oid->hash, refname))
+	if (filter->points_at.nr && !match_points_at(&filter->points_at, oid, refname))
 		return 0;
 
 	/*

[PATCH v2 18/21] Convert sha1_array_lookup to take struct object_id

[brian m. carlson]

Convert this function by changing the declaration and definition and
applying the following semantic patch to update the callers:

@@
expression E1, E2;
@@
- sha1_array_lookup(E1, E2.hash)
+ sha1_array_lookup(E1, &E2)

@@
expression E1, E2;
@@
- sha1_array_lookup(E1, E2->hash)
+ sha1_array_lookup(E1, E2)

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 bisect.c                   | 7 +++----
 builtin/pack-objects.c     | 2 +-
 fsck.c                     | 2 +-
 ref-filter.c               | 4 ++--
 sha1-array.c               | 4 ++--
 sha1-array.h               | 2 +-
 t/helper/test-sha1-array.c | 2 +-
 7 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/bisect.c b/bisect.c
index a25d008693..f193257509 100644
--- a/bisect.c
+++ b/bisect.c
@@ -499,8 +499,7 @@ struct commit_list *filter_skipped(struct commit_list *list,
 	while (list) {
 		struct commit_list *next = list->next;
 		list->next = NULL;
-		if (0 <= sha1_array_lookup(&skipped_revs,
-					   list->item->object.oid.hash)) {
+		if (0 <= sha1_array_lookup(&skipped_revs, &list->item->object.oid)) {
 			if (skipped_first && !*skipped_first)
 				*skipped_first = 1;
 			/* Move current to tried list */
@@ -790,9 +789,9 @@ static void check_merge_bases(int no_checkout)
 		const struct object_id *mb = &result->item->object.oid;
 		if (!oidcmp(mb, current_bad_oid)) {
 			handle_bad_merge_base();
-		} else if (0 <= sha1_array_lookup(&good_revs, mb->hash)) {
+		} else if (0 <= sha1_array_lookup(&good_revs, mb)) {
 			continue;
-		} else if (0 <= sha1_array_lookup(&skipped_revs, mb->hash)) {
+		} else if (0 <= sha1_array_lookup(&skipped_revs, mb)) {
 			handle_skipped_merge_base(mb);
 		} else {
 			printf(_("Bisecting: a merge base must be tested\n"));
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index dca1b68e69..028c7be9a2 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2677,7 +2677,7 @@ static int loosened_object_can_be_discarded(const struct object_id *oid,
 		return 0;
 	if (mtime > unpack_unreachable_expiration)
 		return 0;
-	if (sha1_array_lookup(&recent_objects, oid->hash) >= 0)
+	if (sha1_array_lookup(&recent_objects, oid) >= 0)
 		return 0;
 	return 1;
 }
diff --git a/fsck.c b/fsck.c
index 6682de1de5..24daedd6cc 100644
--- a/fsck.c
+++ b/fsck.c
@@ -280,7 +280,7 @@ static int report(struct fsck_options *options, struct object *object,
 		return 0;
 
 	if (options->skiplist && object &&
-			sha1_array_lookup(options->skiplist, object->oid.hash) >= 0)
+			sha1_array_lookup(options->skiplist, &object->oid) >= 0)
 		return 0;
 
 	if (msg_type == FSCK_FATAL)
diff --git a/ref-filter.c b/ref-filter.c
index d3dcb53dd5..4ee7ebcda3 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1684,14 +1684,14 @@ static const struct object_id *match_points_at(struct sha1_array *points_at,
 	const struct object_id *tagged_oid = NULL;
 	struct object *obj;
 
-	if (sha1_array_lookup(points_at, oid->hash) >= 0)
+	if (sha1_array_lookup(points_at, oid) >= 0)
 		return oid;
 	obj = parse_object(oid->hash);
 	if (!obj)
 		die(_("malformed object at '%s'"), refname);
 	if (obj->type == OBJ_TAG)
 		tagged_oid = &((struct tag *)obj)->tagged->oid;
-	if (tagged_oid && sha1_array_lookup(points_at, tagged_oid->hash) >= 0)
+	if (tagged_oid && sha1_array_lookup(points_at, tagged_oid) >= 0)
 		return tagged_oid;
 	return NULL;
 }
diff --git a/sha1-array.c b/sha1-array.c
index 26e596b264..1082b3dc11 100644
--- a/sha1-array.c
+++ b/sha1-array.c
@@ -26,11 +26,11 @@ static const unsigned char *sha1_access(size_t index, void *table)
 	return array[index].hash;
 }
 
-int sha1_array_lookup(struct sha1_array *array, const unsigned char *sha1)
+int sha1_array_lookup(struct sha1_array *array, const struct object_id *oid)
 {
 	if (!array->sorted)
 		sha1_array_sort(array);
-	return sha1_pos(sha1, array->oid, array->nr, sha1_access);
+	return sha1_pos(oid->hash, array->oid, array->nr, sha1_access);
 }
 
 void sha1_array_clear(struct sha1_array *array)
diff --git a/sha1-array.h b/sha1-array.h
index 7b06fbf1c1..4cc55b15af 100644
--- a/sha1-array.h
+++ b/sha1-array.h
@@ -11,7 +11,7 @@ struct sha1_array {
 #define SHA1_ARRAY_INIT { NULL, 0, 0, 0 }
 
 void sha1_array_append(struct sha1_array *array, const struct object_id *sha1);
-int sha1_array_lookup(struct sha1_array *array, const unsigned char *sha1);
+int sha1_array_lookup(struct sha1_array *array, const struct object_id *oid);
 void sha1_array_clear(struct sha1_array *array);
 
 typedef int (*for_each_sha1_fn)(const unsigned char sha1[20],
diff --git a/t/helper/test-sha1-array.c b/t/helper/test-sha1-array.c
index 181c36e0a5..3680511849 100644
--- a/t/helper/test-sha1-array.c
+++ b/t/helper/test-sha1-array.c
@@ -23,7 +23,7 @@ int cmd_main(int argc, const char **argv)
 		} else if (skip_prefix(line.buf, "lookup ", &arg)) {
 			if (get_oid_hex(arg, &oid))
 				die("not a hexadecimal SHA1: %s", arg);
-			printf("%d\n", sha1_array_lookup(&array, oid.hash));
+			printf("%d\n", sha1_array_lookup(&array, &oid));
 		} else if (!strcmp(line.buf, "clear"))
 			sha1_array_clear(&array);
 		else if (!strcmp(line.buf, "for_each_unique"))

[PATCH v2 19/21] Convert sha1_array_for_each_unique and for_each_abbrev to object_id

[brian m. carlson]

Make sha1_array_for_each_unique take a callback using struct object_id.
Since one of these callbacks is an argument to for_each_abbrev, convert
those as well.  Rename various functions, replacing "sha1" with "oid".

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/cat-file.c         |  4 ++--
 builtin/receive-pack.c     | 12 ++++++------
 builtin/rev-parse.c        |  4 ++--
 cache.h                    |  2 +-
 sha1-array.c               |  4 ++--
 sha1-array.h               |  6 +++---
 sha1_name.c                | 14 ++++++--------
 submodule.c                | 20 ++++++++++----------
 t/helper/test-sha1-array.c |  6 +++---
 9 files changed, 35 insertions(+), 37 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 8fbb667170..eb0043231d 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -401,10 +401,10 @@ struct object_cb_data {
 	struct expand_data *expand;
 };
 
-static int batch_object_cb(const unsigned char sha1[20], void *vdata)
+static int batch_object_cb(const struct object_id *oid, void *vdata)
 {
 	struct object_cb_data *data = vdata;
-	hashcpy(data->expand->oid.hash, sha1);
+	oidcpy(&data->expand->oid, oid);
 	batch_object_write(NULL, data->opt, data->expand);
 	return 0;
 }
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 56d1a59922..5b83e4c87b 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -225,10 +225,10 @@ static int receive_pack_config(const char *var, const char *value, void *cb)
 	return git_default_config(var, value, cb);
 }
 
-static void show_ref(const char *path, const unsigned char *sha1)
+static void show_ref(const char *path, const struct object_id *oid)
 {
 	if (sent_capabilities) {
-		packet_write_fmt(1, "%s %s\n", sha1_to_hex(sha1), path);
+		packet_write_fmt(1, "%s %s\n", oid_to_hex(oid), path);
 	} else {
 		struct strbuf cap = STRBUF_INIT;
 
@@ -244,7 +244,7 @@ static void show_ref(const char *path, const unsigned char *sha1)
 			strbuf_addstr(&cap, " push-options");
 		strbuf_addf(&cap, " agent=%s", git_user_agent_sanitized());
 		packet_write_fmt(1, "%s %s%c%s\n",
-			     sha1_to_hex(sha1), path, 0, cap.buf);
+			     oid_to_hex(oid), path, 0, cap.buf);
 		strbuf_release(&cap);
 		sent_capabilities = 1;
 	}
@@ -271,7 +271,7 @@ static int show_ref_cb(const char *path_full, const struct object_id *oid,
 	} else {
 		oidset_insert(seen, oid);
 	}
-	show_ref(path, oid->hash);
+	show_ref(path, oid);
 	return 0;
 }
 
@@ -284,7 +284,7 @@ static void show_one_alternate_ref(const char *refname,
 	if (oidset_insert(seen, oid))
 		return;
 
-	show_ref(".have", oid->hash);
+	show_ref(".have", oid);
 }
 
 static void write_head_info(void)
@@ -295,7 +295,7 @@ static void write_head_info(void)
 	for_each_alternate_ref(show_one_alternate_ref, &seen);
 	oidset_clear(&seen);
 	if (!sent_capabilities)
-		show_ref("capabilities^{}", null_sha1);
+		show_ref("capabilities^{}", &null_oid);
 
 	advertise_shallow_grafts(1);
 
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 1e5bdea0d5..8d635add8f 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -205,9 +205,9 @@ static int anti_reference(const char *refname, const struct object_id *oid, int
 	return 0;
 }
 
-static int show_abbrev(const unsigned char *sha1, void *cb_data)
+static int show_abbrev(const struct object_id *oid, void *cb_data)
 {
-	show_rev(NORMAL, sha1, NULL);
+	show_rev(NORMAL, oid->hash, NULL);
 	return 0;
 }
 
diff --git a/cache.h b/cache.h
index 179bdc5da2..93e7f0d000 100644
--- a/cache.h
+++ b/cache.h
@@ -1350,7 +1350,7 @@ extern int get_sha1_with_context(const char *str, unsigned flags, unsigned char
 
 extern int get_oid(const char *str, struct object_id *oid);
 
-typedef int each_abbrev_fn(const unsigned char *sha1, void *);
+typedef int each_abbrev_fn(const struct object_id *oid, void *);
 extern int for_each_abbrev(const char *prefix, each_abbrev_fn, void *);
 
 extern int set_disambiguate_hint_config(const char *var, const char *value);
diff --git a/sha1-array.c b/sha1-array.c
index 1082b3dc11..82a7f4435c 100644
--- a/sha1-array.c
+++ b/sha1-array.c
@@ -43,7 +43,7 @@ void sha1_array_clear(struct sha1_array *array)
 }
 
 int sha1_array_for_each_unique(struct sha1_array *array,
-				for_each_sha1_fn fn,
+				for_each_oid_fn fn,
 				void *data)
 {
 	int i;
@@ -55,7 +55,7 @@ int sha1_array_for_each_unique(struct sha1_array *array,
 		int ret;
 		if (i > 0 && !oidcmp(array->oid + i, array->oid + i - 1))
 			continue;
-		ret = fn(array->oid[i].hash, data);
+		ret = fn(array->oid + i, data);
 		if (ret)
 			return ret;
 	}
diff --git a/sha1-array.h b/sha1-array.h
index 4cc55b15af..e2bb139efb 100644
--- a/sha1-array.h
+++ b/sha1-array.h
@@ -14,10 +14,10 @@ void sha1_array_append(struct sha1_array *array, const struct object_id *sha1);
 int sha1_array_lookup(struct sha1_array *array, const struct object_id *oid);
 void sha1_array_clear(struct sha1_array *array);
 
-typedef int (*for_each_sha1_fn)(const unsigned char sha1[20],
-				void *data);
+typedef int (*for_each_oid_fn)(const struct object_id *oid,
+			       void *data);
 int sha1_array_for_each_unique(struct sha1_array *array,
-			       for_each_sha1_fn fn,
+			       for_each_oid_fn fn,
 			       void *data);
 
 #endif /* SHA1_ARRAY_H */
diff --git a/sha1_name.c b/sha1_name.c
index 1316832d73..a9501fb964 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -342,34 +342,32 @@ static int init_object_disambiguation(const char *name, int len,
 	return 0;
 }
 
-static int show_ambiguous_object(const unsigned char *sha1, void *data)
+static int show_ambiguous_object(const struct object_id *oid, void *data)
 {
 	const struct disambiguate_state *ds = data;
-	struct object_id oid;
 	struct strbuf desc = STRBUF_INIT;
 	int type;
 
 
-	hashcpy(oid.hash, sha1);
-	if (ds->fn && !ds->fn(&oid, ds->cb_data))
+	if (ds->fn && !ds->fn(oid, ds->cb_data))
 		return 0;
 
-	type = sha1_object_info(sha1, NULL);
+	type = sha1_object_info(oid->hash, NULL);
 	if (type == OBJ_COMMIT) {
-		struct commit *commit = lookup_commit(sha1);
+		struct commit *commit = lookup_commit(oid->hash);
 		if (commit) {
 			struct pretty_print_context pp = {0};
 			pp.date_mode.type = DATE_SHORT;
 			format_commit_message(commit, " %ad - %s", &desc, &pp);
 		}
 	} else if (type == OBJ_TAG) {
-		struct tag *tag = lookup_tag(sha1);
+		struct tag *tag = lookup_tag(oid->hash);
 		if (!parse_tag(tag) && tag->tag)
 			strbuf_addf(&desc, " %s", tag->tag);
 	}
 
 	advise("  %s %s%s",
-	       find_unique_abbrev(sha1, DEFAULT_ABBREV),
+	       find_unique_abbrev(oid->hash, DEFAULT_ABBREV),
 	       typename(type) ? typename(type) : "unknown type",
 	       desc.buf);
 
diff --git a/submodule.c b/submodule.c
index 7912cba4f6..29dd91c793 100644
--- a/submodule.c
+++ b/submodule.c
@@ -551,18 +551,18 @@ static int has_remote(const char *refname, const struct object_id *oid,
 	return 1;
 }
 
-static int append_sha1_to_argv(const unsigned char sha1[20], void *data)
+static int append_oid_to_argv(const struct object_id *oid, void *data)
 {
 	struct argv_array *argv = data;
-	argv_array_push(argv, sha1_to_hex(sha1));
+	argv_array_push(argv, oid_to_hex(oid));
 	return 0;
 }
 
-static int check_has_commit(const unsigned char sha1[20], void *data)
+static int check_has_commit(const struct object_id *oid, void *data)
 {
 	int *has_commit = data;
 
-	if (!lookup_commit_reference(sha1))
+	if (!lookup_commit_reference(oid->hash))
 		*has_commit = 0;
 
 	return 0;
@@ -601,7 +601,7 @@ static int submodule_needs_pushing(const char *path, struct sha1_array *commits)
 		int needs_pushing = 0;
 
 		argv_array_push(&cp.args, "rev-list");
-		sha1_array_for_each_unique(commits, append_sha1_to_argv, &cp.args);
+		sha1_array_for_each_unique(commits, append_oid_to_argv, &cp.args);
 		argv_array_pushl(&cp.args, "--not", "--remotes", "-n", "1" , NULL);
 
 		prepare_submodule_repo_env(&cp.env_array);
@@ -687,7 +687,7 @@ int find_unpushed_submodules(struct sha1_array *commits,
 
 	/* argv.argv[0] will be ignored by setup_revisions */
 	argv_array_push(&argv, "find_unpushed_submodules");
-	sha1_array_for_each_unique(commits, append_sha1_to_argv, &argv);
+	sha1_array_for_each_unique(commits, append_oid_to_argv, &argv);
 	argv_array_push(&argv, "--not");
 	argv_array_pushf(&argv, "--remotes=%s", remotes_name);
 
@@ -831,9 +831,9 @@ void check_for_new_submodule_commits(struct object_id *oid)
 	sha1_array_append(&ref_tips_after_fetch, oid);
 }
 
-static int add_sha1_to_argv(const unsigned char sha1[20], void *data)
+static int add_oid_to_argv(const struct object_id *oid, void *data)
 {
-	argv_array_push(data, sha1_to_hex(sha1));
+	argv_array_push(data, oid_to_hex(oid));
 	return 0;
 }
 
@@ -850,10 +850,10 @@ static void calculate_changed_submodule_paths(void)
 	init_revisions(&rev, NULL);
 	argv_array_push(&argv, "--"); /* argv[0] program name */
 	sha1_array_for_each_unique(&ref_tips_after_fetch,
-				   add_sha1_to_argv, &argv);
+				   add_oid_to_argv, &argv);
 	argv_array_push(&argv, "--not");
 	sha1_array_for_each_unique(&ref_tips_before_fetch,
-				   add_sha1_to_argv, &argv);
+				   add_oid_to_argv, &argv);
 	setup_revisions(argv.argc, argv.argv, &rev, NULL);
 	if (prepare_revision_walk(&rev))
 		die("revision walk setup failed");
diff --git a/t/helper/test-sha1-array.c b/t/helper/test-sha1-array.c
index 3680511849..2f3d406b63 100644
--- a/t/helper/test-sha1-array.c
+++ b/t/helper/test-sha1-array.c
@@ -1,9 +1,9 @@
 #include "cache.h"
 #include "sha1-array.h"
 
-static int print_sha1(const unsigned char sha1[20], void *data)
+static int print_oid(const struct object_id *oid, void *data)
 {
-	puts(sha1_to_hex(sha1));
+	puts(oid_to_hex(oid));
 	return 0;
 }
 
@@ -27,7 +27,7 @@ int cmd_main(int argc, const char **argv)
 		} else if (!strcmp(line.buf, "clear"))
 			sha1_array_clear(&array);
 		else if (!strcmp(line.buf, "for_each_unique"))
-			sha1_array_for_each_unique(&array, print_sha1, NULL);
+			sha1_array_for_each_unique(&array, print_oid, NULL);
 		else
 			die("unknown command: %s", line.buf);
 	}

[PATCH v2 20/21] Rename sha1_array to oid_array

[brian m. carlson]

Since this structure handles an array of object IDs, rename it to struct
oid_array.  Also rename the accessor functions and the initialization
constant.

This commit was produced mechanically by providing non-Documentation
files to the following Perl one-liners:

    perl -pi -E 's/struct sha1_array/struct oid_array/g'
    perl -pi -E 's/\bsha1_array_/oid_array_/g'
    perl -pi -E 's/SHA1_ARRAY_INIT/OID_ARRAY_INIT/g'

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 bisect.c                   | 16 ++++++++--------
 builtin/cat-file.c         | 10 +++++-----
 builtin/diff.c             |  6 +++---
 builtin/fetch-pack.c       |  2 +-
 builtin/pack-objects.c     | 10 +++++-----
 builtin/pull.c             |  6 +++---
 builtin/receive-pack.c     | 24 +++++++++++------------
 builtin/send-pack.c        |  4 ++--
 combine-diff.c             | 12 ++++++------
 commit.h                   | 14 +++++++-------
 connect.c                  |  8 ++++----
 diff.h                     |  4 ++--
 fetch-pack.c               | 26 ++++++++++++-------------
 fetch-pack.h               |  4 ++--
 fsck.c                     |  6 +++---
 fsck.h                     |  2 +-
 parse-options-cb.c         |  4 ++--
 ref-filter.c               |  6 +++---
 ref-filter.h               |  2 +-
 remote-curl.c              |  2 +-
 remote.h                   |  6 +++---
 send-pack.c                |  4 ++--
 send-pack.h                |  2 +-
 sha1-array.c               | 14 +++++++-------
 sha1-array.h               | 12 ++++++------
 sha1_name.c                |  8 ++++----
 shallow.c                  | 12 ++++++------
 submodule.c                | 48 +++++++++++++++++++++++-----------------------
 submodule.h                |  6 +++---
 t/helper/test-sha1-array.c | 10 +++++-----
 transport.c                | 20 +++++++++----------
 31 files changed, 155 insertions(+), 155 deletions(-)

diff --git a/bisect.c b/bisect.c
index f193257509..54d69e77b9 100644
--- a/bisect.c
+++ b/bisect.c
@@ -12,8 +12,8 @@
 #include "sha1-array.h"
 #include "argv-array.h"
 
-static struct sha1_array good_revs;
-static struct sha1_array skipped_revs;
+static struct oid_array good_revs;
+static struct oid_array skipped_revs;
 
 static struct object_id *current_bad_oid;
 
@@ -413,9 +413,9 @@ static int register_ref(const char *refname, const struct object_id *oid,
 		current_bad_oid = xmalloc(sizeof(*current_bad_oid));
 		oidcpy(current_bad_oid, oid);
 	} else if (starts_with(refname, good_prefix.buf)) {
-		sha1_array_append(&good_revs, oid);
+		oid_array_append(&good_revs, oid);
 	} else if (starts_with(refname, "skip-")) {
-		sha1_array_append(&skipped_revs, oid);
+		oid_array_append(&skipped_revs, oid);
 	}
 
 	strbuf_release(&good_prefix);
@@ -451,7 +451,7 @@ static void read_bisect_paths(struct argv_array *array)
 	fclose(fp);
 }
 
-static char *join_sha1_array_hex(struct sha1_array *array, char delim)
+static char *join_sha1_array_hex(struct oid_array *array, char delim)
 {
 	struct strbuf joined_hexs = STRBUF_INIT;
 	int i;
@@ -499,7 +499,7 @@ struct commit_list *filter_skipped(struct commit_list *list,
 	while (list) {
 		struct commit_list *next = list->next;
 		list->next = NULL;
-		if (0 <= sha1_array_lookup(&skipped_revs, &list->item->object.oid)) {
+		if (0 <= oid_array_lookup(&skipped_revs, &list->item->object.oid)) {
 			if (skipped_first && !*skipped_first)
 				*skipped_first = 1;
 			/* Move current to tried list */
@@ -789,9 +789,9 @@ static void check_merge_bases(int no_checkout)
 		const struct object_id *mb = &result->item->object.oid;
 		if (!oidcmp(mb, current_bad_oid)) {
 			handle_bad_merge_base();
-		} else if (0 <= sha1_array_lookup(&good_revs, mb)) {
+		} else if (0 <= oid_array_lookup(&good_revs, mb)) {
 			continue;
-		} else if (0 <= sha1_array_lookup(&skipped_revs, mb)) {
+		} else if (0 <= oid_array_lookup(&skipped_revs, mb)) {
 			handle_skipped_merge_base(mb);
 		} else {
 			printf(_("Bisecting: a merge base must be tested\n"));
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index eb0043231d..1890d7a639 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -413,7 +413,7 @@ static int batch_loose_object(const struct object_id *oid,
 			      const char *path,
 			      void *data)
 {
-	sha1_array_append(data, oid);
+	oid_array_append(data, oid);
 	return 0;
 }
 
@@ -422,7 +422,7 @@ static int batch_packed_object(const struct object_id *oid,
 			       uint32_t pos,
 			       void *data)
 {
-	sha1_array_append(data, oid);
+	oid_array_append(data, oid);
 	return 0;
 }
 
@@ -462,7 +462,7 @@ static int batch_objects(struct batch_options *opt)
 		data.info.typep = &data.type;
 
 	if (opt->all_objects) {
-		struct sha1_array sa = SHA1_ARRAY_INIT;
+		struct oid_array sa = OID_ARRAY_INIT;
 		struct object_cb_data cb;
 
 		for_each_loose_object(batch_loose_object, &sa, 0);
@@ -470,9 +470,9 @@ static int batch_objects(struct batch_options *opt)
 
 		cb.opt = opt;
 		cb.expand = &data;
-		sha1_array_for_each_unique(&sa, batch_object_cb, &cb);
+		oid_array_for_each_unique(&sa, batch_object_cb, &cb);
 
-		sha1_array_clear(&sa);
+		oid_array_clear(&sa);
 		return 0;
 	}
 
diff --git a/builtin/diff.c b/builtin/diff.c
index a5b34eb156..d184aafab9 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -184,7 +184,7 @@ static int builtin_diff_combined(struct rev_info *revs,
 				 struct object_array_entry *ent,
 				 int ents)
 {
-	struct sha1_array parents = SHA1_ARRAY_INIT;
+	struct oid_array parents = OID_ARRAY_INIT;
 	int i;
 
 	if (argc > 1)
@@ -193,10 +193,10 @@ static int builtin_diff_combined(struct rev_info *revs,
 	if (!revs->dense_combined_merges && !revs->combine_merges)
 		revs->dense_combined_merges = revs->combine_merges = 1;
 	for (i = 1; i < ents; i++)
-		sha1_array_append(&parents, &ent[i].item->oid);
+		oid_array_append(&parents, &ent[i].item->oid);
 	diff_tree_combined(ent[0].item->oid.hash, &parents,
 			   revs->dense_combined_merges, revs);
-	sha1_array_clear(&parents);
+	oid_array_clear(&parents);
 	return 0;
 }
 
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 2a1c1c213f..366b9d13f9 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -50,7 +50,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
 	char **pack_lockfile_ptr = NULL;
 	struct child_process *conn;
 	struct fetch_pack_args args;
-	struct sha1_array shallow = SHA1_ARRAY_INIT;
+	struct oid_array shallow = OID_ARRAY_INIT;
 	struct string_list deepen_not = STRING_LIST_INIT_DUP;
 
 	packet_trace_identity("fetch-pack");
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 028c7be9a2..4fc032e3ec 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2668,7 +2668,7 @@ static int has_sha1_pack_kept_or_nonlocal(const unsigned char *sha1)
  *
  * This is filled by get_object_list.
  */
-static struct sha1_array recent_objects;
+static struct oid_array recent_objects;
 
 static int loosened_object_can_be_discarded(const struct object_id *oid,
 					    unsigned long mtime)
@@ -2677,7 +2677,7 @@ static int loosened_object_can_be_discarded(const struct object_id *oid,
 		return 0;
 	if (mtime > unpack_unreachable_expiration)
 		return 0;
-	if (sha1_array_lookup(&recent_objects, oid) >= 0)
+	if (oid_array_lookup(&recent_objects, oid) >= 0)
 		return 0;
 	return 1;
 }
@@ -2739,12 +2739,12 @@ static void record_recent_object(struct object *obj,
 				 const char *name,
 				 void *data)
 {
-	sha1_array_append(&recent_objects, &obj->oid);
+	oid_array_append(&recent_objects, &obj->oid);
 }
 
 static void record_recent_commit(struct commit *commit, void *data)
 {
-	sha1_array_append(&recent_objects, &commit->object.oid);
+	oid_array_append(&recent_objects, &commit->object.oid);
 }
 
 static void get_object_list(int ac, const char **av)
@@ -2812,7 +2812,7 @@ static void get_object_list(int ac, const char **av)
 	if (unpack_unreachable)
 		loosen_unused_packed_objects(&revs);
 
-	sha1_array_clear(&recent_objects);
+	oid_array_clear(&recent_objects);
 }
 
 static int option_parse_index_version(const struct option *opt,
diff --git a/builtin/pull.c b/builtin/pull.c
index 183e377147..d8aa26d8ab 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -330,7 +330,7 @@ static int git_pull_config(const char *var, const char *value, void *cb)
  * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge
  * into merge_heads.
  */
-static void get_merge_heads(struct sha1_array *merge_heads)
+static void get_merge_heads(struct oid_array *merge_heads)
 {
 	const char *filename = git_path("FETCH_HEAD");
 	FILE *fp;
@@ -344,7 +344,7 @@ static void get_merge_heads(struct sha1_array *merge_heads)
 			continue;  /* invalid line: does not start with SHA1 */
 		if (starts_with(sb.buf + GIT_SHA1_HEXSZ, "\tnot-for-merge\t"))
 			continue;  /* ref is not-for-merge */
-		sha1_array_append(merge_heads, &oid);
+		oid_array_append(merge_heads, &oid);
 	}
 	fclose(fp);
 	strbuf_release(&sb);
@@ -769,7 +769,7 @@ static int run_rebase(const struct object_id *curr_head,
 int cmd_pull(int argc, const char **argv, const char *prefix)
 {
 	const char *repo, **refspecs;
-	struct sha1_array merge_heads = SHA1_ARRAY_INIT;
+	struct oid_array merge_heads = OID_ARRAY_INIT;
 	struct object_id orig_head, curr_head;
 	struct object_id rebase_fork_point;
 
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 5b83e4c87b..5d9e4da0a7 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -831,7 +831,7 @@ static int command_singleton_iterator(void *cb_data, unsigned char sha1[20]);
 static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
 {
 	static struct lock_file shallow_lock;
-	struct sha1_array extra = SHA1_ARRAY_INIT;
+	struct oid_array extra = OID_ARRAY_INIT;
 	struct check_connected_options opt = CHECK_CONNECTED_INIT;
 	uint32_t mask = 1 << (cmd->index % 32);
 	int i;
@@ -842,13 +842,13 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
 		if (si->used_shallow[i] &&
 		    (si->used_shallow[i][cmd->index / 32] & mask) &&
 		    !delayed_reachability_test(si, i))
-			sha1_array_append(&extra, si->shallow->oid + i);
+			oid_array_append(&extra, si->shallow->oid + i);
 
 	opt.env = tmp_objdir_env(tmp_objdir);
 	setup_alternate_shallow(&shallow_lock, &opt.shallow_file, &extra);
 	if (check_connected(command_singleton_iterator, cmd, &opt)) {
 		rollback_lock_file(&shallow_lock);
-		sha1_array_clear(&extra);
+		oid_array_clear(&extra);
 		return -1;
 	}
 
@@ -862,7 +862,7 @@ static int update_shallow_ref(struct command *cmd, struct shallow_info *si)
 		register_shallow(extra.oid[i].hash);
 
 	si->shallow_ref[cmd->index] = 0;
-	sha1_array_clear(&extra);
+	oid_array_clear(&extra);
 	return 0;
 }
 
@@ -1529,7 +1529,7 @@ static void queue_commands_from_cert(struct command **tail,
 	}
 }
 
-static struct command *read_head_info(struct sha1_array *shallow)
+static struct command *read_head_info(struct oid_array *shallow)
 {
 	struct command *commands = NULL;
 	struct command **p = &commands;
@@ -1546,7 +1546,7 @@ static struct command *read_head_info(struct sha1_array *shallow)
 			if (get_oid_hex(line + 8, &oid))
 				die("protocol error: expected shallow sha, got '%s'",
 				    line + 8);
-			sha1_array_append(shallow, &oid);
+			oid_array_append(shallow, &oid);
 			continue;
 		}
 
@@ -1804,7 +1804,7 @@ static void prepare_shallow_update(struct command *commands,
 
 static void update_shallow_info(struct command *commands,
 				struct shallow_info *si,
-				struct sha1_array *ref)
+				struct oid_array *ref)
 {
 	struct command *cmd;
 	int *ref_status;
@@ -1817,7 +1817,7 @@ static void update_shallow_info(struct command *commands,
 	for (cmd = commands; cmd; cmd = cmd->next) {
 		if (is_null_oid(&cmd->new_oid))
 			continue;
-		sha1_array_append(ref, &cmd->new_oid);
+		oid_array_append(ref, &cmd->new_oid);
 		cmd->index = ref->nr - 1;
 	}
 	si->ref = ref;
@@ -1878,8 +1878,8 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 {
 	int advertise_refs = 0;
 	struct command *commands;
-	struct sha1_array shallow = SHA1_ARRAY_INIT;
-	struct sha1_array ref = SHA1_ARRAY_INIT;
+	struct oid_array shallow = OID_ARRAY_INIT;
+	struct oid_array ref = OID_ARRAY_INIT;
 	struct shallow_info si;
 
 	struct option options[] = {
@@ -1971,8 +1971,8 @@ int cmd_receive_pack(int argc, const char **argv, const char *prefix)
 	}
 	if (use_sideband)
 		packet_flush(1);
-	sha1_array_clear(&shallow);
-	sha1_array_clear(&ref);
+	oid_array_clear(&shallow);
+	oid_array_clear(&ref);
 	free((void *)push_cert_nonce);
 	return 0;
 }
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 1ff5a67538..b646194429 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -131,8 +131,8 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 	const char *dest = NULL;
 	int fd[2];
 	struct child_process *conn;
-	struct sha1_array extra_have = SHA1_ARRAY_INIT;
-	struct sha1_array shallow = SHA1_ARRAY_INIT;
+	struct oid_array extra_have = OID_ARRAY_INIT;
+	struct oid_array shallow = OID_ARRAY_INIT;
 	struct ref *remote_refs, *local_refs;
 	int ret;
 	int helper_status = 0;
diff --git a/combine-diff.c b/combine-diff.c
index c92029484c..c5586c972d 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1311,7 +1311,7 @@ static const char *path_path(void *obj)
 
 /* find set of paths that every parent touches */
 static struct combine_diff_path *find_paths_generic(const unsigned char *sha1,
-	const struct sha1_array *parents, struct diff_options *opt)
+	const struct oid_array *parents, struct diff_options *opt)
 {
 	struct combine_diff_path *paths = NULL;
 	int i, num_parent = parents->nr;
@@ -1359,7 +1359,7 @@ static struct combine_diff_path *find_paths_generic(const unsigned char *sha1,
  * rename/copy detection, etc, comparing all trees simultaneously (= faster).
  */
 static struct combine_diff_path *find_paths_multitree(
-	const unsigned char *sha1, const struct sha1_array *parents,
+	const unsigned char *sha1, const struct oid_array *parents,
 	struct diff_options *opt)
 {
 	int i, nparent = parents->nr;
@@ -1384,7 +1384,7 @@ static struct combine_diff_path *find_paths_multitree(
 
 
 void diff_tree_combined(const unsigned char *sha1,
-			const struct sha1_array *parents,
+			const struct oid_array *parents,
 			int dense,
 			struct rev_info *rev)
 {
@@ -1532,12 +1532,12 @@ void diff_tree_combined_merge(const struct commit *commit, int dense,
 			      struct rev_info *rev)
 {
 	struct commit_list *parent = get_saved_parents(rev, commit);
-	struct sha1_array parents = SHA1_ARRAY_INIT;
+	struct oid_array parents = OID_ARRAY_INIT;
 
 	while (parent) {
-		sha1_array_append(&parents, &parent->item->object.oid);
+		oid_array_append(&parents, &parent->item->object.oid);
 		parent = parent->next;
 	}
 	diff_tree_combined(commit->object.oid.hash, &parents, dense, rev);
-	sha1_array_clear(&parents);
+	oid_array_clear(&parents);
 }
diff --git a/commit.h b/commit.h
index 528272ac9b..7b1986d5c8 100644
--- a/commit.h
+++ b/commit.h
@@ -261,7 +261,7 @@ extern struct commit_list *get_merge_bases_many_dirty(struct commit *one, int n,
 /* largest positive number a signed 32-bit integer can contain */
 #define INFINITE_DEPTH 0x7fffffff
 
-struct sha1_array;
+struct oid_array;
 struct ref;
 extern int register_shallow(const unsigned char *sha1);
 extern int unregister_shallow(const unsigned char *sha1);
@@ -273,18 +273,18 @@ extern struct commit_list *get_shallow_commits_by_rev_list(
 		int ac, const char **av, int shallow_flag, int not_shallow_flag);
 extern void set_alternate_shallow_file(const char *path, int override);
 extern int write_shallow_commits(struct strbuf *out, int use_pack_protocol,
-				 const struct sha1_array *extra);
+				 const struct oid_array *extra);
 extern void setup_alternate_shallow(struct lock_file *shallow_lock,
 				    const char **alternate_shallow_file,
-				    const struct sha1_array *extra);
-extern const char *setup_temporary_shallow(const struct sha1_array *extra);
+				    const struct oid_array *extra);
+extern const char *setup_temporary_shallow(const struct oid_array *extra);
 extern void advertise_shallow_grafts(int);
 
 struct shallow_info {
-	struct sha1_array *shallow;
+	struct oid_array *shallow;
 	int *ours, nr_ours;
 	int *theirs, nr_theirs;
-	struct sha1_array *ref;
+	struct oid_array *ref;
 
 	/* for receive-pack */
 	uint32_t **used_shallow;
@@ -295,7 +295,7 @@ struct shallow_info {
 	int nr_commits;
 };
 
-extern void prepare_shallow_info(struct shallow_info *, struct sha1_array *);
+extern void prepare_shallow_info(struct shallow_info *, struct oid_array *);
 extern void clear_shallow_info(struct shallow_info *);
 extern void remove_nonexistent_theirs_shallow(struct shallow_info *);
 extern void assign_shallow_commits_to_refs(struct shallow_info *info,
diff --git a/connect.c b/connect.c
index 50b503da0d..6afde189a3 100644
--- a/connect.c
+++ b/connect.c
@@ -111,8 +111,8 @@ static void annotate_refs_with_symref_info(struct ref *ref)
  */
 struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
 			      struct ref **list, unsigned int flags,
-			      struct sha1_array *extra_have,
-			      struct sha1_array *shallow_points)
+			      struct oid_array *extra_have,
+			      struct oid_array *shallow_points)
 {
 	struct ref **orig_list = list;
 
@@ -153,7 +153,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
 				die("protocol error: expected shallow sha-1, got '%s'", arg);
 			if (!shallow_points)
 				die("repository on the other end cannot be shallow");
-			sha1_array_append(shallow_points, &old_oid);
+			oid_array_append(shallow_points, &old_oid);
 			continue;
 		}
 
@@ -169,7 +169,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
 		}
 
 		if (extra_have && !strcmp(name, ".have")) {
-			sha1_array_append(extra_have, &old_oid);
+			oid_array_append(extra_have, &old_oid);
 			continue;
 		}
 
diff --git a/diff.h b/diff.h
index e9ccb38c26..5be1ee77a7 100644
--- a/diff.h
+++ b/diff.h
@@ -14,7 +14,7 @@ struct diff_queue_struct;
 struct strbuf;
 struct diff_filespec;
 struct userdiff_driver;
-struct sha1_array;
+struct oid_array;
 struct commit;
 struct combine_diff_path;
 
@@ -236,7 +236,7 @@ struct combine_diff_path {
 extern void show_combined_diff(struct combine_diff_path *elem, int num_parent,
 			      int dense, struct rev_info *);
 
-extern void diff_tree_combined(const unsigned char *sha1, const struct sha1_array *parents, int dense, struct rev_info *rev);
+extern void diff_tree_combined(const unsigned char *sha1, const struct oid_array *parents, int dense, struct rev_info *rev);
 
 extern void diff_tree_combined_merge(const struct commit *commit, int dense, struct rev_info *rev);
 
diff --git a/fetch-pack.c b/fetch-pack.c
index f4bbd2892a..4092f8af06 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1015,7 +1015,7 @@ static void update_shallow(struct fetch_pack_args *args,
 			   struct ref **sought, int nr_sought,
 			   struct shallow_info *si)
 {
-	struct sha1_array ref = SHA1_ARRAY_INIT;
+	struct oid_array ref = OID_ARRAY_INIT;
 	int *status;
 	int i;
 
@@ -1038,18 +1038,18 @@ static void update_shallow(struct fetch_pack_args *args,
 		 * shallow points that exist in the pack (iow in repo
 		 * after get_pack() and reprepare_packed_git())
 		 */
-		struct sha1_array extra = SHA1_ARRAY_INIT;
+		struct oid_array extra = OID_ARRAY_INIT;
 		struct object_id *oid = si->shallow->oid;
 		for (i = 0; i < si->shallow->nr; i++)
 			if (has_object_file(&oid[i]))
-				sha1_array_append(&extra, oid + i);
+				oid_array_append(&extra, oid + i);
 		if (extra.nr) {
 			setup_alternate_shallow(&shallow_lock,
 						&alternate_shallow_file,
 						&extra);
 			commit_lock_file(&shallow_lock);
 		}
-		sha1_array_clear(&extra);
+		oid_array_clear(&extra);
 		return;
 	}
 
@@ -1060,7 +1060,7 @@ static void update_shallow(struct fetch_pack_args *args,
 	if (!si->nr_ours && !si->nr_theirs)
 		return;
 	for (i = 0; i < nr_sought; i++)
-		sha1_array_append(&ref, &sought[i]->old_oid);
+		oid_array_append(&ref, &sought[i]->old_oid);
 	si->ref = &ref;
 
 	if (args->update_shallow) {
@@ -1070,23 +1070,23 @@ static void update_shallow(struct fetch_pack_args *args,
 		 * shallow roots that are actually reachable from new
 		 * refs.
 		 */
-		struct sha1_array extra = SHA1_ARRAY_INIT;
+		struct oid_array extra = OID_ARRAY_INIT;
 		struct object_id *oid = si->shallow->oid;
 		assign_shallow_commits_to_refs(si, NULL, NULL);
 		if (!si->nr_ours && !si->nr_theirs) {
-			sha1_array_clear(&ref);
+			oid_array_clear(&ref);
 			return;
 		}
 		for (i = 0; i < si->nr_ours; i++)
-			sha1_array_append(&extra, oid + si->ours[i]);
+			oid_array_append(&extra, oid + si->ours[i]);
 		for (i = 0; i < si->nr_theirs; i++)
-			sha1_array_append(&extra, oid + si->theirs[i]);
+			oid_array_append(&extra, oid + si->theirs[i]);
 		setup_alternate_shallow(&shallow_lock,
 					&alternate_shallow_file,
 					&extra);
 		commit_lock_file(&shallow_lock);
-		sha1_array_clear(&extra);
-		sha1_array_clear(&ref);
+		oid_array_clear(&extra);
+		oid_array_clear(&ref);
 		return;
 	}
 
@@ -1102,7 +1102,7 @@ static void update_shallow(struct fetch_pack_args *args,
 				sought[i]->status = REF_STATUS_REJECT_SHALLOW;
 	}
 	free(status);
-	sha1_array_clear(&ref);
+	oid_array_clear(&ref);
 }
 
 struct ref *fetch_pack(struct fetch_pack_args *args,
@@ -1110,7 +1110,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
 		       const struct ref *ref,
 		       const char *dest,
 		       struct ref **sought, int nr_sought,
-		       struct sha1_array *shallow,
+		       struct oid_array *shallow,
 		       char **pack_lockfile)
 {
 	struct ref *ref_cpy;
diff --git a/fetch-pack.h b/fetch-pack.h
index a2d46e6e75..b6aeb43a8e 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -4,7 +4,7 @@
 #include "string-list.h"
 #include "run-command.h"
 
-struct sha1_array;
+struct oid_array;
 
 struct fetch_pack_args {
 	const char *uploadpack;
@@ -42,7 +42,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
 		       const char *dest,
 		       struct ref **sought,
 		       int nr_sought,
-		       struct sha1_array *shallow,
+		       struct oid_array *shallow,
 		       char **pack_lockfile);
 
 /*
diff --git a/fsck.c b/fsck.c
index 24daedd6cc..e6152e4e6d 100644
--- a/fsck.c
+++ b/fsck.c
@@ -132,7 +132,7 @@ static int fsck_msg_type(enum fsck_msg_id msg_id,
 
 static void init_skiplist(struct fsck_options *options, const char *path)
 {
-	static struct sha1_array skiplist = SHA1_ARRAY_INIT;
+	static struct oid_array skiplist = OID_ARRAY_INIT;
 	int sorted, fd;
 	char buffer[GIT_MAX_HEXSZ + 1];
 	struct object_id oid;
@@ -156,7 +156,7 @@ static void init_skiplist(struct fsck_options *options, const char *path)
 			break;
 		if (parse_oid_hex(buffer, &oid, &p) || *p != '\n')
 			die("Invalid SHA-1: %s", buffer);
-		sha1_array_append(&skiplist, &oid);
+		oid_array_append(&skiplist, &oid);
 		if (sorted && skiplist.nr > 1 &&
 				oidcmp(&skiplist.oid[skiplist.nr - 2],
 				       &oid) > 0)
@@ -280,7 +280,7 @@ static int report(struct fsck_options *options, struct object *object,
 		return 0;
 
 	if (options->skiplist && object &&
-			sha1_array_lookup(options->skiplist, &object->oid) >= 0)
+			oid_array_lookup(options->skiplist, &object->oid) >= 0)
 		return 0;
 
 	if (msg_type == FSCK_FATAL)
diff --git a/fsck.h b/fsck.h
index 1891c1863b..4525510d99 100644
--- a/fsck.h
+++ b/fsck.h
@@ -34,7 +34,7 @@ struct fsck_options {
 	fsck_error error_func;
 	unsigned strict:1;
 	int *msg_type;
-	struct sha1_array *skiplist;
+	struct oid_array *skiplist;
 	struct decoration *object_names;
 };
 
diff --git a/parse-options-cb.c b/parse-options-cb.c
index 7baecdc864..7419780a9b 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -99,14 +99,14 @@ int parse_opt_object_name(const struct option *opt, const char *arg, int unset)
 	struct object_id oid;
 
 	if (unset) {
-		sha1_array_clear(opt->value);
+		oid_array_clear(opt->value);
 		return 0;
 	}
 	if (!arg)
 		return -1;
 	if (get_oid(arg, &oid))
 		return error(_("malformed object name '%s'"), arg);
-	sha1_array_append(opt->value, &oid);
+	oid_array_append(opt->value, &oid);
 	return 0;
 }
 
diff --git a/ref-filter.c b/ref-filter.c
index 4ee7ebcda3..fe1abf65ff 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1677,21 +1677,21 @@ static int filter_pattern_match(struct ref_filter *filter, const char *refname)
  * the need to parse the object via parse_object(). peel_ref() might be a
  * more efficient alternative to obtain the pointee.
  */
-static const struct object_id *match_points_at(struct sha1_array *points_at,
+static const struct object_id *match_points_at(struct oid_array *points_at,
 					       const struct object_id *oid,
 					       const char *refname)
 {
 	const struct object_id *tagged_oid = NULL;
 	struct object *obj;
 
-	if (sha1_array_lookup(points_at, oid) >= 0)
+	if (oid_array_lookup(points_at, oid) >= 0)
 		return oid;
 	obj = parse_object(oid->hash);
 	if (!obj)
 		die(_("malformed object at '%s'"), refname);
 	if (obj->type == OBJ_TAG)
 		tagged_oid = &((struct tag *)obj)->tagged->oid;
-	if (tagged_oid && sha1_array_lookup(points_at, tagged_oid) >= 0)
+	if (tagged_oid && oid_array_lookup(points_at, tagged_oid) >= 0)
 		return tagged_oid;
 	return NULL;
 }
diff --git a/ref-filter.h b/ref-filter.h
index e738c5dfd3..e5e177d5f1 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -51,7 +51,7 @@ struct ref_array {
 
 struct ref_filter {
 	const char **name_patterns;
-	struct sha1_array points_at;
+	struct oid_array points_at;
 	struct commit_list *with_commit;
 
 	enum {
diff --git a/remote-curl.c b/remote-curl.c
index 5e712e4aa1..82a019d7db 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -163,7 +163,7 @@ struct discovery {
 	char *buf;
 	size_t len;
 	struct ref *refs;
-	struct sha1_array shallow;
+	struct oid_array shallow;
 	unsigned proto_git : 1;
 };
 static struct discovery *last_discovery;
diff --git a/remote.h b/remote.h
index dd8c517577..eb61cc884d 100644
--- a/remote.h
+++ b/remote.h
@@ -149,11 +149,11 @@ int check_ref_type(const struct ref *ref, int flags);
  */
 void free_refs(struct ref *ref);
 
-struct sha1_array;
+struct oid_array;
 extern struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,
 				     struct ref **list, unsigned int flags,
-				     struct sha1_array *extra_have,
-				     struct sha1_array *shallow);
+				     struct oid_array *extra_have,
+				     struct oid_array *shallow);
 
 int resolve_remote_symref(struct ref *ref, struct ref *list);
 int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid);
diff --git a/send-pack.c b/send-pack.c
index b3040391cd..312cd2c282 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -50,7 +50,7 @@ static void feed_object(const unsigned char *sha1, FILE *fh, int negative)
 /*
  * Make a pack stream and spit it out into file descriptor fd
  */
-static int pack_objects(int fd, struct ref *refs, struct sha1_array *extra, struct send_pack_args *args)
+static int pack_objects(int fd, struct ref *refs, struct oid_array *extra, struct send_pack_args *args)
 {
 	/*
 	 * The child becomes pack-objects --revs; we feed
@@ -376,7 +376,7 @@ static void reject_invalid_nonce(const char *nonce, int len)
 int send_pack(struct send_pack_args *args,
 	      int fd[], struct child_process *conn,
 	      struct ref *remote_refs,
-	      struct sha1_array *extra_have)
+	      struct oid_array *extra_have)
 {
 	int in = fd[0];
 	int out = fd[1];
diff --git a/send-pack.h b/send-pack.h
index 67fc40f4ec..6af71f7008 100644
--- a/send-pack.h
+++ b/send-pack.h
@@ -32,6 +32,6 @@ int option_parse_push_signed(const struct option *opt,
 
 int send_pack(struct send_pack_args *args,
 	      int fd[], struct child_process *conn,
-	      struct ref *remote_refs, struct sha1_array *extra_have);
+	      struct ref *remote_refs, struct oid_array *extra_have);
 
 #endif
diff --git a/sha1-array.c b/sha1-array.c
index 82a7f4435c..7d646ab5b8 100644
--- a/sha1-array.c
+++ b/sha1-array.c
@@ -2,7 +2,7 @@
 #include "sha1-array.h"
 #include "sha1-lookup.h"
 
-void sha1_array_append(struct sha1_array *array, const struct object_id *oid)
+void oid_array_append(struct oid_array *array, const struct object_id *oid)
 {
 	ALLOC_GROW(array->oid, array->nr + 1, array->alloc);
 	oidcpy(&array->oid[array->nr++], oid);
@@ -14,7 +14,7 @@ static int void_hashcmp(const void *a, const void *b)
 	return oidcmp(a, b);
 }
 
-static void sha1_array_sort(struct sha1_array *array)
+static void oid_array_sort(struct oid_array *array)
 {
 	QSORT(array->oid, array->nr, void_hashcmp);
 	array->sorted = 1;
@@ -26,14 +26,14 @@ static const unsigned char *sha1_access(size_t index, void *table)
 	return array[index].hash;
 }
 
-int sha1_array_lookup(struct sha1_array *array, const struct object_id *oid)
+int oid_array_lookup(struct oid_array *array, const struct object_id *oid)
 {
 	if (!array->sorted)
-		sha1_array_sort(array);
+		oid_array_sort(array);
 	return sha1_pos(oid->hash, array->oid, array->nr, sha1_access);
 }
 
-void sha1_array_clear(struct sha1_array *array)
+void oid_array_clear(struct oid_array *array)
 {
 	free(array->oid);
 	array->oid = NULL;
@@ -42,14 +42,14 @@ void sha1_array_clear(struct sha1_array *array)
 	array->sorted = 0;
 }
 
-int sha1_array_for_each_unique(struct sha1_array *array,
+int oid_array_for_each_unique(struct oid_array *array,
 				for_each_oid_fn fn,
 				void *data)
 {
 	int i;
 
 	if (!array->sorted)
-		sha1_array_sort(array);
+		oid_array_sort(array);
 
 	for (i = 0; i < array->nr; i++) {
 		int ret;
diff --git a/sha1-array.h b/sha1-array.h
index e2bb139efb..0a6f9e64ad 100644
--- a/sha1-array.h
+++ b/sha1-array.h
@@ -1,22 +1,22 @@
 #ifndef SHA1_ARRAY_H
 #define SHA1_ARRAY_H
 
-struct sha1_array {
+struct oid_array {
 	struct object_id *oid;
 	int nr;
 	int alloc;
 	int sorted;
 };
 
-#define SHA1_ARRAY_INIT { NULL, 0, 0, 0 }
+#define OID_ARRAY_INIT { NULL, 0, 0, 0 }
 
-void sha1_array_append(struct sha1_array *array, const struct object_id *sha1);
-int sha1_array_lookup(struct sha1_array *array, const struct object_id *oid);
-void sha1_array_clear(struct sha1_array *array);
+void oid_array_append(struct oid_array *array, const struct object_id *sha1);
+int oid_array_lookup(struct oid_array *array, const struct object_id *oid);
+void oid_array_clear(struct oid_array *array);
 
 typedef int (*for_each_oid_fn)(const struct object_id *oid,
 			       void *data);
-int sha1_array_for_each_unique(struct sha1_array *array,
+int oid_array_for_each_unique(struct oid_array *array,
 			       for_each_oid_fn fn,
 			       void *data);
 
diff --git a/sha1_name.c b/sha1_name.c
index a9501fb964..323dd34c63 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -426,13 +426,13 @@ static int get_short_sha1(const char *name, int len, unsigned char *sha1,
 
 static int collect_ambiguous(const struct object_id *oid, void *data)
 {
-	sha1_array_append(data, oid);
+	oid_array_append(data, oid);
 	return 0;
 }
 
 int for_each_abbrev(const char *prefix, each_abbrev_fn fn, void *cb_data)
 {
-	struct sha1_array collect = SHA1_ARRAY_INIT;
+	struct oid_array collect = OID_ARRAY_INIT;
 	struct disambiguate_state ds;
 	int ret;
 
@@ -445,8 +445,8 @@ int for_each_abbrev(const char *prefix, each_abbrev_fn fn, void *cb_data)
 	find_short_object_filename(&ds);
 	find_short_packed_object(&ds);
 
-	ret = sha1_array_for_each_unique(&collect, fn, cb_data);
-	sha1_array_clear(&collect);
+	ret = oid_array_for_each_unique(&collect, fn, cb_data);
+	oid_array_clear(&collect);
 	return ret;
 }
 
diff --git a/shallow.c b/shallow.c
index dc7b67a294..25b6db989b 100644
--- a/shallow.c
+++ b/shallow.c
@@ -260,7 +260,7 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data)
 }
 
 static int write_shallow_commits_1(struct strbuf *out, int use_pack_protocol,
-				   const struct sha1_array *extra,
+				   const struct oid_array *extra,
 				   unsigned flags)
 {
 	struct write_shallow_data data;
@@ -281,14 +281,14 @@ static int write_shallow_commits_1(struct strbuf *out, int use_pack_protocol,
 }
 
 int write_shallow_commits(struct strbuf *out, int use_pack_protocol,
-			  const struct sha1_array *extra)
+			  const struct oid_array *extra)
 {
 	return write_shallow_commits_1(out, use_pack_protocol, extra, 0);
 }
 
 static struct tempfile temporary_shallow;
 
-const char *setup_temporary_shallow(const struct sha1_array *extra)
+const char *setup_temporary_shallow(const struct oid_array *extra)
 {
 	struct strbuf sb = STRBUF_INIT;
 	int fd;
@@ -312,7 +312,7 @@ const char *setup_temporary_shallow(const struct sha1_array *extra)
 
 void setup_alternate_shallow(struct lock_file *shallow_lock,
 			     const char **alternate_shallow_file,
-			     const struct sha1_array *extra)
+			     const struct oid_array *extra)
 {
 	struct strbuf sb = STRBUF_INIT;
 	int fd;
@@ -385,7 +385,7 @@ struct trace_key trace_shallow = TRACE_KEY_INIT(SHALLOW);
  * Step 1, split sender shallow commits into "ours" and "theirs"
  * Step 2, clean "ours" based on .git/shallow
  */
-void prepare_shallow_info(struct shallow_info *info, struct sha1_array *sa)
+void prepare_shallow_info(struct shallow_info *info, struct oid_array *sa)
 {
 	int i;
 	trace_printf_key(&trace_shallow, "shallow: prepare_shallow_info\n");
@@ -560,7 +560,7 @@ void assign_shallow_commits_to_refs(struct shallow_info *info,
 				    uint32_t **used, int *ref_status)
 {
 	struct object_id *oid = info->shallow->oid;
-	struct sha1_array *ref = info->ref;
+	struct oid_array *ref = info->ref;
 	unsigned int i, nr;
 	int *shallow, nr_shallow = 0;
 	struct paint_info pi;
diff --git a/submodule.c b/submodule.c
index 29dd91c793..b6bfb0cad9 100644
--- a/submodule.c
+++ b/submodule.c
@@ -20,8 +20,8 @@ static int config_fetch_recurse_submodules = RECURSE_SUBMODULES_ON_DEMAND;
 static int parallel_jobs = 1;
 static struct string_list changed_submodule_paths = STRING_LIST_INIT_NODUP;
 static int initialized_fetch_ref_tips;
-static struct sha1_array ref_tips_before_fetch;
-static struct sha1_array ref_tips_after_fetch;
+static struct oid_array ref_tips_before_fetch;
+static struct oid_array ref_tips_after_fetch;
 
 /*
  * The following flag is set if the .gitmodules file is unmerged. We then
@@ -568,18 +568,18 @@ static int check_has_commit(const struct object_id *oid, void *data)
 	return 0;
 }
 
-static int submodule_has_commits(const char *path, struct sha1_array *commits)
+static int submodule_has_commits(const char *path, struct oid_array *commits)
 {
 	int has_commit = 1;
 
 	if (add_submodule_odb(path))
 		return 0;
 
-	sha1_array_for_each_unique(commits, check_has_commit, &has_commit);
+	oid_array_for_each_unique(commits, check_has_commit, &has_commit);
 	return has_commit;
 }
 
-static int submodule_needs_pushing(const char *path, struct sha1_array *commits)
+static int submodule_needs_pushing(const char *path, struct oid_array *commits)
 {
 	if (!submodule_has_commits(path, commits))
 		/*
@@ -601,7 +601,7 @@ static int submodule_needs_pushing(const char *path, struct sha1_array *commits)
 		int needs_pushing = 0;
 
 		argv_array_push(&cp.args, "rev-list");
-		sha1_array_for_each_unique(commits, append_oid_to_argv, &cp.args);
+		oid_array_for_each_unique(commits, append_oid_to_argv, &cp.args);
 		argv_array_pushl(&cp.args, "--not", "--remotes", "-n", "1" , NULL);
 
 		prepare_submodule_repo_env(&cp.env_array);
@@ -623,18 +623,18 @@ static int submodule_needs_pushing(const char *path, struct sha1_array *commits)
 	return 0;
 }
 
-static struct sha1_array *submodule_commits(struct string_list *submodules,
+static struct oid_array *submodule_commits(struct string_list *submodules,
 					    const char *path)
 {
 	struct string_list_item *item;
 
 	item = string_list_insert(submodules, path);
 	if (item->util)
-		return (struct sha1_array *) item->util;
+		return (struct oid_array *) item->util;
 
-	/* NEEDSWORK: should we have sha1_array_init()? */
-	item->util = xcalloc(1, sizeof(struct sha1_array));
-	return (struct sha1_array *) item->util;
+	/* NEEDSWORK: should we have oid_array_init()? */
+	item->util = xcalloc(1, sizeof(struct oid_array));
+	return (struct oid_array *) item->util;
 }
 
 static void collect_submodules_from_diff(struct diff_queue_struct *q,
@@ -646,11 +646,11 @@ static void collect_submodules_from_diff(struct diff_queue_struct *q,
 
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
-		struct sha1_array *commits;
+		struct oid_array *commits;
 		if (!S_ISGITLINK(p->two->mode))
 			continue;
 		commits = submodule_commits(submodules, p->two->path);
-		sha1_array_append(commits, &p->two->oid);
+		oid_array_append(commits, &p->two->oid);
 	}
 }
 
@@ -670,11 +670,11 @@ static void free_submodules_sha1s(struct string_list *submodules)
 {
 	struct string_list_item *item;
 	for_each_string_list_item(item, submodules)
-		sha1_array_clear((struct sha1_array *) item->util);
+		oid_array_clear((struct oid_array *) item->util);
 	string_list_clear(submodules, 1);
 }
 
-int find_unpushed_submodules(struct sha1_array *commits,
+int find_unpushed_submodules(struct oid_array *commits,
 		const char *remotes_name, struct string_list *needs_pushing)
 {
 	struct rev_info rev;
@@ -687,7 +687,7 @@ int find_unpushed_submodules(struct sha1_array *commits,
 
 	/* argv.argv[0] will be ignored by setup_revisions */
 	argv_array_push(&argv, "find_unpushed_submodules");
-	sha1_array_for_each_unique(commits, append_oid_to_argv, &argv);
+	oid_array_for_each_unique(commits, append_oid_to_argv, &argv);
 	argv_array_push(&argv, "--not");
 	argv_array_pushf(&argv, "--remotes=%s", remotes_name);
 
@@ -702,7 +702,7 @@ int find_unpushed_submodules(struct sha1_array *commits,
 	argv_array_clear(&argv);
 
 	for_each_string_list_item(submodule, &submodules) {
-		struct sha1_array *commits = (struct sha1_array *) submodule->util;
+		struct oid_array *commits = (struct oid_array *) submodule->util;
 
 		if (submodule_needs_pushing(submodule->string, commits))
 			string_list_insert(needs_pushing, submodule->string);
@@ -735,7 +735,7 @@ static int push_submodule(const char *path, int dry_run)
 	return 1;
 }
 
-int push_unpushed_submodules(struct sha1_array *commits,
+int push_unpushed_submodules(struct oid_array *commits,
 			     const char *remotes_name,
 			     int dry_run)
 {
@@ -817,7 +817,7 @@ static void submodule_collect_changed_cb(struct diff_queue_struct *q,
 static int add_sha1_to_array(const char *ref, const struct object_id *oid,
 			     int flags, void *data)
 {
-	sha1_array_append(data, oid);
+	oid_array_append(data, oid);
 	return 0;
 }
 
@@ -828,7 +828,7 @@ void check_for_new_submodule_commits(struct object_id *oid)
 		initialized_fetch_ref_tips = 1;
 	}
 
-	sha1_array_append(&ref_tips_after_fetch, oid);
+	oid_array_append(&ref_tips_after_fetch, oid);
 }
 
 static int add_oid_to_argv(const struct object_id *oid, void *data)
@@ -849,10 +849,10 @@ static void calculate_changed_submodule_paths(void)
 
 	init_revisions(&rev, NULL);
 	argv_array_push(&argv, "--"); /* argv[0] program name */
-	sha1_array_for_each_unique(&ref_tips_after_fetch,
+	oid_array_for_each_unique(&ref_tips_after_fetch,
 				   add_oid_to_argv, &argv);
 	argv_array_push(&argv, "--not");
-	sha1_array_for_each_unique(&ref_tips_before_fetch,
+	oid_array_for_each_unique(&ref_tips_before_fetch,
 				   add_oid_to_argv, &argv);
 	setup_revisions(argv.argc, argv.argv, &rev, NULL);
 	if (prepare_revision_walk(&rev))
@@ -879,8 +879,8 @@ static void calculate_changed_submodule_paths(void)
 	}
 
 	argv_array_clear(&argv);
-	sha1_array_clear(&ref_tips_before_fetch);
-	sha1_array_clear(&ref_tips_after_fetch);
+	oid_array_clear(&ref_tips_before_fetch);
+	oid_array_clear(&ref_tips_after_fetch);
 	initialized_fetch_ref_tips = 0;
 }
 
diff --git a/submodule.h b/submodule.h
index 9c32b28b12..decca25e37 100644
--- a/submodule.h
+++ b/submodule.h
@@ -3,7 +3,7 @@
 
 struct diff_options;
 struct argv_array;
-struct sha1_array;
+struct oid_array;
 
 enum {
 	RECURSE_SUBMODULES_ONLY = -5,
@@ -73,10 +73,10 @@ extern int merge_submodule(unsigned char result[20], const char *path,
 			   const unsigned char base[20],
 			   const unsigned char a[20],
 			   const unsigned char b[20], int search);
-extern int find_unpushed_submodules(struct sha1_array *commits,
+extern int find_unpushed_submodules(struct oid_array *commits,
 				    const char *remotes_name,
 				    struct string_list *needs_pushing);
-extern int push_unpushed_submodules(struct sha1_array *commits,
+extern int push_unpushed_submodules(struct oid_array *commits,
 				    const char *remotes_name,
 				    int dry_run);
 extern void connect_work_tree_and_git_dir(const char *work_tree, const char *git_dir);
diff --git a/t/helper/test-sha1-array.c b/t/helper/test-sha1-array.c
index 2f3d406b63..edfd52d82a 100644
--- a/t/helper/test-sha1-array.c
+++ b/t/helper/test-sha1-array.c
@@ -9,7 +9,7 @@ static int print_oid(const struct object_id *oid, void *data)
 
 int cmd_main(int argc, const char **argv)
 {
-	struct sha1_array array = SHA1_ARRAY_INIT;
+	struct oid_array array = OID_ARRAY_INIT;
 	struct strbuf line = STRBUF_INIT;
 
 	while (strbuf_getline(&line, stdin) != EOF) {
@@ -19,15 +19,15 @@ int cmd_main(int argc, const char **argv)
 		if (skip_prefix(line.buf, "append ", &arg)) {
 			if (get_oid_hex(arg, &oid))
 				die("not a hexadecimal SHA1: %s", arg);
-			sha1_array_append(&array, &oid);
+			oid_array_append(&array, &oid);
 		} else if (skip_prefix(line.buf, "lookup ", &arg)) {
 			if (get_oid_hex(arg, &oid))
 				die("not a hexadecimal SHA1: %s", arg);
-			printf("%d\n", sha1_array_lookup(&array, &oid));
+			printf("%d\n", oid_array_lookup(&array, &oid));
 		} else if (!strcmp(line.buf, "clear"))
-			sha1_array_clear(&array);
+			oid_array_clear(&array);
 		else if (!strcmp(line.buf, "for_each_unique"))
-			sha1_array_for_each_unique(&array, print_oid, NULL);
+			oid_array_for_each_unique(&array, print_oid, NULL);
 		else
 			die("unknown command: %s", line.buf);
 	}
diff --git a/transport.c b/transport.c
index e492757726..c83f3b71be 100644
--- a/transport.c
+++ b/transport.c
@@ -116,8 +116,8 @@ struct git_transport_data {
 	struct child_process *conn;
 	int fd[2];
 	unsigned got_remote_heads : 1;
-	struct sha1_array extra_have;
-	struct sha1_array shallow;
+	struct oid_array extra_have;
+	struct oid_array shallow;
 };
 
 static int set_git_option(struct git_transport_options *opts,
@@ -1023,20 +1023,20 @@ int transport_push(struct transport *transport,
 			      TRANSPORT_RECURSE_SUBMODULES_ONLY)) &&
 		    !is_bare_repository()) {
 			struct ref *ref = remote_refs;
-			struct sha1_array commits = SHA1_ARRAY_INIT;
+			struct oid_array commits = OID_ARRAY_INIT;
 
 			for (; ref; ref = ref->next)
 				if (!is_null_oid(&ref->new_oid))
-					sha1_array_append(&commits,
+					oid_array_append(&commits,
 							  &ref->new_oid);
 
 			if (!push_unpushed_submodules(&commits,
 						      transport->remote->name,
 						      pretend)) {
-				sha1_array_clear(&commits);
+				oid_array_clear(&commits);
 				die("Failed to push all needed submodules!");
 			}
-			sha1_array_clear(&commits);
+			oid_array_clear(&commits);
 		}
 
 		if (((flags & TRANSPORT_RECURSE_SUBMODULES_CHECK) ||
@@ -1045,20 +1045,20 @@ int transport_push(struct transport *transport,
 		      !pretend)) && !is_bare_repository()) {
 			struct ref *ref = remote_refs;
 			struct string_list needs_pushing = STRING_LIST_INIT_DUP;
-			struct sha1_array commits = SHA1_ARRAY_INIT;
+			struct oid_array commits = OID_ARRAY_INIT;
 
 			for (; ref; ref = ref->next)
 				if (!is_null_oid(&ref->new_oid))
-					sha1_array_append(&commits,
+					oid_array_append(&commits,
 							  &ref->new_oid);
 
 			if (find_unpushed_submodules(&commits, transport->remote->name,
 						&needs_pushing)) {
-				sha1_array_clear(&commits);
+				oid_array_clear(&commits);
 				die_with_unpushed_submodules(&needs_pushing);
 			}
 			string_list_clear(&needs_pushing, 0);
-			sha1_array_clear(&commits);
+			oid_array_clear(&commits);
 		}
 
 		if (!(flags & TRANSPORT_RECURSE_SUBMODULES_ONLY))

[PATCH v2 21/21] Documentation: update and rename api-sha1-array.txt

[brian m. carlson]

Since the structure and functions have changed names, update the code
examples and the documentation.  Rename the file to match the new name
of the API.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 .../{api-sha1-array.txt => api-oid-array.txt}      | 44 +++++++++++-----------
 1 file changed, 22 insertions(+), 22 deletions(-)
 rename Documentation/technical/{api-sha1-array.txt => api-oid-array.txt} (61%)

diff --git a/Documentation/technical/api-sha1-array.txt b/Documentation/technical/api-oid-array.txt
similarity index 61%
rename from Documentation/technical/api-sha1-array.txt
rename to Documentation/technical/api-oid-array.txt
index dcc52943a5..b0c11f868d 100644
--- a/Documentation/technical/api-sha1-array.txt
+++ b/Documentation/technical/api-oid-array.txt
@@ -1,7 +1,7 @@
-sha1-array API
+oid-array API
 ==============
 
-The sha1-array API provides storage and manipulation of sets of SHA-1
+The oid-array API provides storage and manipulation of sets of object
 identifiers. The emphasis is on storage and processing efficiency,
 making them suitable for large lists. Note that the ordering of items is
 not preserved over some operations.
@@ -9,10 +9,10 @@ not preserved over some operations.
 Data Structures
 ---------------
 
-`struct sha1_array`::
+`struct oid_array`::
 
-	A single array of SHA-1 hashes. This should be initialized by
-	assignment from `SHA1_ARRAY_INIT`.  The `sha1` member contains
+	A single array of object IDs. This should be initialized by
+	assignment from `OID_ARRAY_INIT`.  The `oid` member contains
 	the actual data. The `nr` member contains the number of items in
 	the set.  The `alloc` and `sorted` members are used internally,
 	and should not be needed by API callers.
@@ -20,22 +20,22 @@ Data Structures
 Functions
 ---------
 
-`sha1_array_append`::
-	Add an item to the set. The sha1 will be placed at the end of
+`oid_array_append`::
+	Add an item to the set. The object ID will be placed at the end of
 	the array (but note that some operations below may lose this
 	ordering).
 
-`sha1_array_lookup`::
-	Perform a binary search of the array for a specific sha1.
+`oid_array_lookup`::
+	Perform a binary search of the array for a specific object ID.
 	If found, returns the offset (in number of elements) of the
-	sha1. If not found, returns a negative integer. If the array is
-	not sorted, this function has the side effect of sorting it.
+	object ID. If not found, returns a negative integer. If the array
+	is not sorted, this function has the side effect of sorting it.
 
-`sha1_array_clear`::
+`oid_array_clear`::
 	Free all memory associated with the array and return it to the
 	initial, empty state.
 
-`sha1_array_for_each_unique`::
+`oid_array_for_each_unique`::
 	Efficiently iterate over each unique element of the list,
 	executing the callback function for each one. If the array is
 	not sorted, this function has the side effect of sorting it. If
@@ -47,25 +47,25 @@ Examples
 --------
 
 -----------------------------------------
-int print_callback(const unsigned char sha1[20],
+int print_callback(const struct object_id *oid,
 		    void *data)
 {
-	printf("%s\n", sha1_to_hex(sha1));
+	printf("%s\n", oid_to_hex(oid));
 	return 0; /* always continue */
 }
 
 void some_func(void)
 {
-	struct sha1_array hashes = SHA1_ARRAY_INIT;
-	unsigned char sha1[20];
+	struct sha1_array hashes = OID_ARRAY_INIT;
+	struct object_id oid;
 
 	/* Read objects into our set */
-	while (read_object_from_stdin(sha1))
-		sha1_array_append(&hashes, sha1);
+	while (read_object_from_stdin(oid.hash))
+		oid_array_append(&hashes, &oid);
 
 	/* Check if some objects are in our set */
-	while (read_object_from_stdin(sha1)) {
-		if (sha1_array_lookup(&hashes, sha1) >= 0)
+	while (read_object_from_stdin(oid.hash)) {
+		if (oid_array_lookup(&hashes, &oid) >= 0)
 			printf("it's in there!\n");
 
 	/*
@@ -75,6 +75,6 @@ void some_func(void)
 	 * Instead, this will sort once and then skip duplicates
 	 * in linear time.
 	 */
-	sha1_array_for_each_unique(&hashes, print_callback, NULL);
+	oid_array_for_each_unique(&hashes, print_callback, NULL);
 }
 -----------------------------------------

Re: [PATCH v2 06/21] builtin/receive-pack: fix incorrect pointer arithmetic

[Jeff King]

On Sun, Mar 26, 2017 at 04:01:28PM +0000, brian m. carlson wrote:

> If we had already processed the last newline in a push certificate, we
> would end up subtracting NULL from the end-of-certificate pointer when
> computing the length of the line.  This would have resulted in an
> absurdly large length, and possibly a buffer overflow.  Instead,
> subtract the beginning-of-certificate pointer from the
> end-of-certificate pointer, which is what's expected.
> 
> Note that this situation should never occur, since not only do we
> require the certificate to be newline terminated, but the signature will
> only be read from the beginning of a line.  Nevertheless, it seems
> prudent to correct it.

I think you can trigger it with carefully-crafted input. Try this on the
client side:

diff --git a/send-pack.c b/send-pack.c
index 66e652f7e..dd18c9a33 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -311,8 +311,7 @@ static int generate_push_cert(struct strbuf *req_buf,
 	if (!update_seen)
 		goto free_return;
 
-	if (sign_buffer(&cert, &cert, signing_key))
-		die(_("failed to sign the push certificate"));
+	strbuf_rtrim(&cert);
 
 	packet_buf_write(req_buf, "push-cert%c%s", 0, cap_string);
 	for (cp = cert.buf; cp < cert.buf + cert.len; cp = np) {

We omit the signature entirely, which causes the parser to treat the
end of the string as the end-of-cert (we still find the end because the
push-cert-end is in its own pkt-line; you could also just issue a flush,
which has the same effect).

And then the rtrim means that the cert doesn't actually end in a
newline. Running t5534 with this patch causes receive-pack to segfault.
It's not an overflow on writing the buffer, though; the nonsense size is
passed into a FLEX_ALLOC_MEM(). In my test case, I ended up allocating a
1.4GB buffer (which just happened to be the mod-2^32 residue of my "eoc"
pointer), and the segfault comes from trying to read an unallocated
page.

So I don't think it's exploitable in any interesting way.

> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index feafb076a4..116f3177a1 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -1524,7 +1524,7 @@ static void queue_commands_from_cert(struct command **tail,
>  
>  	while (boc < eoc) {
>  		const char *eol = memchr(boc, '\n', eoc - boc);
> -		tail = queue_command(tail, boc, eol ? eol - boc : eoc - eol);
> +		tail = queue_command(tail, boc, eol ? eol - boc : eoc - boc);
>  		boc = eol ? eol + 1 : eoc;
>  	}
>  }

The patch itself is obviously an improvement. It may be worth graduating
separately from the rest of the series.

-Peff

Re: [PATCH v2 07/21] builtin/receive-pack: convert portions to struct object_id

[Jeff King]

On Sun, Mar 26, 2017 at 04:01:29PM +0000, brian m. carlson wrote:

> Convert some hardcoded constants into uses of parse_oid_hex.
> Additionally, convert all uses of struct command, and miscellaneous
> other functions necessary for that.  This work is necessary to be able
> to convert sha1_array_append later on.
> 
> To avoid needing to specify a constant, reject shallow lines with the
> wrong length instead of simply ignoring them.

It took me a while to find it. This is the switch from "len == 48" to
"len > 8" when matching "shallow" lines. I think this makes sense.

> Note that in queue_command we are guaranteed to have a NUL-terminated
> buffer or at least one byte of overflow that we can safely read, so the
> linelen check can be elided.  We would die in such a case, but not read
> invalid memory.

I think linelen is always just strlen(line). Since the queue_command
function no longer cares about it, perhaps we can just omit it?

> @@ -1541,12 +1541,12 @@ static struct command *read_head_info(struct sha1_array *shallow)
>  		if (!line)
>  			break;
>  
> -		if (len == 48 && starts_with(line, "shallow ")) {
> -			unsigned char sha1[20];
> -			if (get_sha1_hex(line + 8, sha1))
> +		if (len > 8 && starts_with(line, "shallow ")) {
> +			struct object_id oid;
> +			if (get_oid_hex(line + 8, &oid))
>  				die("protocol error: expected shallow sha, got '%s'",
>  				    line + 8);

This would be much nicer as:

  if (skip_prefix(line, "shallow ", &hex))

It's probably best to keep to one type of cleanup at a time here. I'm
just making a mental note.

-Peff

Re: [PATCH v2 15/21] sha1-array: convert internal storage for struct sha1_array to object_id

[Jeff King]

On Sun, Mar 26, 2017 at 04:01:37PM +0000, brian m. carlson wrote:

> Make the internal storage for struct sha1_array use an array of struct
> object_id internally.  Update the users of this struct which inspect its
> internals.

Yay. I'm happy to see all those gross (*sha1)[20] bits go away.

-Peff

Re: [PATCH v2 16/21] Make sha1_array_append take a struct object_id *

[Jeff King]

On Sun, Mar 26, 2017 at 04:01:38PM +0000, brian m. carlson wrote:

> diff --git a/transport.c b/transport.c
> index 8a90b0c29b..e492757726 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -1027,7 +1027,8 @@ int transport_push(struct transport *transport,
>  
>  			for (; ref; ref = ref->next)
>  				if (!is_null_oid(&ref->new_oid))
> -					sha1_array_append(&commits, ref->new_oid.hash);
> +					sha1_array_append(&commits,
> +							  &ref->new_oid);

Funny that this line wrapped when it got shorter. :)

I think wrapping is the right thing, though (it is longer than 80).

-Peff

Re: [PATCH v2 00/21] object_id part 7

[Jeff King]

On Sun, Mar 26, 2017 at 04:01:22PM +0000, brian m. carlson wrote:

> This is part 7 in the continuing transition to use struct object_id.
> 
> This series focuses on two main areas: adding two constants for the
> maximum hash size we'll be using (which will be suitable for allocating
> memory) and converting struct sha1_array to struct oid_array.

Both changes are very welcome. I do think it's probably worth changing
the name of sha1-array.[ch], but it doesn't need to happen immediately.

I read through the whole series and didn't find anything objectionable.
The pointer-arithmetic fix should perhaps graduate separately.

I suggested an additional cleanup around "linelen" in one patch. In the
name of keeping the number of re-rolls sane, I'm OK if we skip that for
now (the only reason I mentioned it at all is that you have to justify
the caveat in the commit message; with the fix, that justification can
go away).

-Peff

Re: [PATCH v2 00/21] object_id part 7

[brian m. carlson]

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

On Tue, Mar 28, 2017 at 03:31:59AM -0400, Jeff King wrote:
> I read through the whole series and didn't find anything objectionable.
> The pointer-arithmetic fix should perhaps graduate separately.

Junio's welcome to take that patch separately if he likes.

> I suggested an additional cleanup around "linelen" in one patch. In the
> name of keeping the number of re-rolls sane, I'm OK if we skip that for
> now (the only reason I mentioned it at all is that you have to justify
> the caveat in the commit message; with the fix, that justification can
> go away).

Let's leave it as it is, assuming Junio's okay with it.  I can send in a
few more patches to clean that up and use skip_prefix that we can drop
on top and graduate separately.

I think the justification is useful as it is, since it explains why we
no longer want to check that particular value for historical reasons.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH v2 06/21] builtin/receive-pack: fix incorrect pointer arithmetic

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> The patch itself is obviously an improvement. It may be worth graduating
> separately from the rest of the series.

Yup, I will split it out to bc/push-cert-receive-fix that builds
directly on an ancient jc/push-cert topic that was merged at
v2.2.0-rc0~64.

I'll need to drop the duplicate from bc/object-id topic, of course
(which hasn't happend).

Thanks.

Re: [PATCH v2 16/21] Make sha1_array_append take a struct object_id *

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> Convert the callers to pass struct object_id by changing the function
> declaration and definition and applying the following semantic patch:
>
> @@
> expression E1, E2, E3;
> @@
> - sha1_array_append(E1, E2[E3].hash)
> + sha1_array_append(E1, E2 + E3)
>
> @@
> expression E1, E2;
> @@
> - sha1_array_append(E1, E2.hash)
> + sha1_array_append(E1, &E2)

I noticed something similar in the change to bisect.c while reading
the previous step, and I suspect that the above two rules leave
somewhat inconsistent and harder-to-read result.  Wouldn't it make
the result more readable if the former rule were

    -sha1_array_append(E1, E2[E3].hash)
    +sha1_array_append(E1, &E2[E3])


FWIW, the bit that made me read it twice in the previous step was
this change

-		strbuf_addstr(&joined_hexs, sha1_to_hex(array->sha1[i]));
+		strbuf_addstr(&joined_hexs, oid_to_hex(array->oid + i));

which I would have written &(array->oid[i]) instead.

After all, the original written by a human said E2[E3].hash (or
array->sha1[i]) because to the human's mind, E2 is a series of
things that can be indexed with an int E3, and even though 

    *(E2 + E3)
    E2[E3]
    E3[E2]

all mean the same thing, the human decided that E2[E3] is the most
natural way to express this particular reference to an item in the
array.  &E2[E3] would keep that intention by the original author
better than E2 + E3.

The above comment does not affect the correctness of the conversion,
but I think it would affect the readability of the resulting code.

Re: [PATCH v2 00/21] object_id part 7

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Sun, Mar 26, 2017 at 04:01:22PM +0000, brian m. carlson wrote:
>
>> This is part 7 in the continuing transition to use struct object_id.
>> 
>> This series focuses on two main areas: adding two constants for the
>> maximum hash size we'll be using (which will be suitable for allocating
>> memory) and converting struct sha1_array to struct oid_array.
>
> Both changes are very welcome. I do think it's probably worth changing
> the name of sha1-array.[ch], but it doesn't need to happen immediately.
>
> I read through the whole series and didn't find anything objectionable.
> The pointer-arithmetic fix should perhaps graduate separately.

I didn't see anything incorrect when I queued the series, either,
and after I re-read it I saw a few minor readability issues, but
modulo that this looks ready.  I did split the push-cert parsing fix
and applied to an older base independently, though.

> I suggested an additional cleanup around "linelen" in one patch. In the
> name of keeping the number of re-rolls sane, I'm OK if we skip that for
> now (the only reason I mentioned it at all is that you have to justify
> the caveat in the commit message; with the fix, that justification can
> go away).

A follow-up after the dust settles could also mention "we earlier
mentioned this caveat but with this fix we no longer have to worry
about it", no?


Thanks both, anyways.

Re: [PATCH v2 00/21] object_id part 7

[Jeff King]

On Tue, Mar 28, 2017 at 11:13:15AM +0000, brian m. carlson wrote:

> > I suggested an additional cleanup around "linelen" in one patch. In the
> > name of keeping the number of re-rolls sane, I'm OK if we skip that for
> > now (the only reason I mentioned it at all is that you have to justify
> > the caveat in the commit message; with the fix, that justification can
> > go away).
> 
> Let's leave it as it is, assuming Junio's okay with it.  I can send in a
> few more patches to clean that up and use skip_prefix that we can drop
> on top and graduate separately.
> 
> I think the justification is useful as it is, since it explains why we
> no longer want to check that particular value for historical reasons.

I thought I'd knock this out quickly before I forgot about it. But it
actually isn't so simple.

The main caller in read_head_info() does indeed just pass strlen(line)
as the length in each case. But the cert parser really does need us to
respect the line length. So we either have to pass it in, or tie off the
string.

The latter looks something like the patch below (on top of a minor
tweak around "eol" handling). It's sufficiently ugly that it may not
count as an actual cleanup, though. I'm OK if we just drop the idea.

---
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 58de2a1a9..561a982e7 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1483,13 +1483,10 @@ static void execute_commands(struct command *commands,
 }
 
 static struct command **queue_command(struct command **tail,
-				      const char *line,
-				      int linelen)
+				      const char *line)
 {
 	struct object_id old_oid, new_oid;
 	struct command *cmd;
-	const char *refname;
-	int reflen;
 	const char *p;
 
 	if (parse_oid_hex(line, &old_oid, &p) ||
@@ -1498,9 +1495,7 @@ static struct command **queue_command(struct command **tail,
 	    *p++ != ' ')
 		die("protocol error: expected old/new/ref, got '%s'", line);
 
-	refname = p;
-	reflen = linelen - (p - line);
-	FLEX_ALLOC_MEM(cmd, ref_name, refname, reflen);
+	FLEX_ALLOC_STR(cmd, ref_name, p);
 	oidcpy(&cmd->old_oid, &old_oid);
 	oidcpy(&cmd->new_oid, &new_oid);
 	*tail = cmd;
@@ -1510,7 +1505,7 @@ static struct command **queue_command(struct command **tail,
 static void queue_commands_from_cert(struct command **tail,
 				     struct strbuf *push_cert)
 {
-	const char *boc, *eoc;
+	char *boc, *eoc;
 
 	if (*tail)
 		die("protocol error: got both push certificate and unsigned commands");
@@ -1523,10 +1518,17 @@ static void queue_commands_from_cert(struct command **tail,
 	eoc = push_cert->buf + parse_signature(push_cert->buf, push_cert->len);
 
 	while (boc < eoc) {
-		const char *eol = memchr(boc, '\n', eoc - boc);
+		char *eol = memchr(boc, '\n', eoc - boc);
+		char tmp;
+
 		if (!eol)
 			eol = eoc;
-		tail = queue_command(tail, boc, eol - boc);
+
+		tmp = *eol;
+		*eol = '\0';
+		tail = queue_command(tail, boc);
+		*eol = tmp;
+
 		boc = eol + 1;
 	}
 }
@@ -1590,7 +1592,7 @@ static struct command *read_head_info(struct oid_array *shallow)
 			continue;
 		}
 
-		p = queue_command(p, line, linelen);
+		p = queue_command(p, line);
 	}
 
 	if (push_cert.len)

Re: [PATCH v2 00/21] object_id part 7

[Jeff King]

On Tue, Mar 28, 2017 at 01:35:36PM -0400, Jeff King wrote:

> I thought I'd knock this out quickly before I forgot about it. But it
> actually isn't so simple.
> 
> The main caller in read_head_info() does indeed just pass strlen(line)
> as the length in each case. But the cert parser really does need us to
> respect the line length. So we either have to pass it in, or tie off the
> string.
> 
> The latter looks something like the patch below (on top of a minor
> tweak around "eol" handling). It's sufficiently ugly that it may not
> count as an actual cleanup, though. I'm OK if we just drop the idea.

Here's that minor tweak, in case anybody is interested. It's less useful
without that follow-on that touches "eol" more, but perhaps it increases
readability on its own.

-- >8 --
Subject: [PATCH] receive-pack: simplify eol handling in cert parsing

The queue_commands_from_cert() function wants to handle each
line of the cert individually. It looks for "\n" in the
to-be-parsed bytes, and special-cases each use of eol (the
end-of-line variable) when we didn't find one.  Instead, we
can just set the end-of-line variable to end-of-cert in the
latter case.

For advancing to the next line, it's OK for us to move our
pointer past end-of-cert, because our loop condition just
checks for pointer inequality. And it doesn't even violate
the ANSI C "no more than one past the end of an array" rule,
because we know in the worst case we've hit the terminating
NUL of the strbuf.

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/receive-pack.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 5d9e4da0a..58de2a1a9 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1524,8 +1524,10 @@ static void queue_commands_from_cert(struct command **tail,
 
 	while (boc < eoc) {
 		const char *eol = memchr(boc, '\n', eoc - boc);
-		tail = queue_command(tail, boc, eol ? eol - boc : eoc - boc);
-		boc = eol ? eol + 1 : eoc;
+		if (!eol)
+			eol = eoc;
+		tail = queue_command(tail, boc, eol - boc);
+		boc = eol + 1;
 	}
 }
 
-- 
2.12.2.845.g55fcf8b10

Re: [PATCH v2 00/21] object_id part 7

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> Here's that minor tweak, in case anybody is interested. It's less useful
> without that follow-on that touches "eol" more, but perhaps it increases
> readability on its own.

Yup, the only thing that the original (with Brian's fix) appears to
be more careful about is it tries very hard to avoid setting boc
past eoc.  As we are not checking "boc != eoc" but doing the
comparison, that "careful" appearance does not give us any benefit
in practice, other than having to do an extra "eol ? eol+1 : eoc";
the result of this patch is easier to read.

By the way, eoc is "one past the end" of the array that begins at
boc, so setting a pointer to eoc+1 may technically be in violation.
I do not know how much it matters, though ;-)

> -- >8 --
> Subject: [PATCH] receive-pack: simplify eol handling in cert parsing
>
> The queue_commands_from_cert() function wants to handle each
> line of the cert individually. It looks for "\n" in the
> to-be-parsed bytes, and special-cases each use of eol (the
> end-of-line variable) when we didn't find one.  Instead, we
> can just set the end-of-line variable to end-of-cert in the
> latter case.
>
> For advancing to the next line, it's OK for us to move our
> pointer past end-of-cert, because our loop condition just
> checks for pointer inequality. And it doesn't even violate
> the ANSI C "no more than one past the end of an array" rule,
> because we know in the worst case we've hit the terminating
> NUL of the strbuf.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  builtin/receive-pack.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
> index 5d9e4da0a..58de2a1a9 100644
> --- a/builtin/receive-pack.c
> +++ b/builtin/receive-pack.c
> @@ -1524,8 +1524,10 @@ static void queue_commands_from_cert(struct command **tail,
>  
>  	while (boc < eoc) {
>  		const char *eol = memchr(boc, '\n', eoc - boc);
> -		tail = queue_command(tail, boc, eol ? eol - boc : eoc - boc);
> -		boc = eol ? eol + 1 : eoc;
> +		if (!eol)
> +			eol = eoc;
> +		tail = queue_command(tail, boc, eol - boc);
> +		boc = eol + 1;
>  	}
>  }

Re: [PATCH v2 00/21] object_id part 7

[Jeff King]

On Tue, Mar 28, 2017 at 12:40:29PM -0700, Junio C Hamano wrote:

> > Here's that minor tweak, in case anybody is interested. It's less useful
> > without that follow-on that touches "eol" more, but perhaps it increases
> > readability on its own.
> 
> Yup, the only thing that the original (with Brian's fix) appears to
> be more careful about is it tries very hard to avoid setting boc
> past eoc.  As we are not checking "boc != eoc" but doing the
> comparison, that "careful" appearance does not give us any benefit
> in practice, other than having to do an extra "eol ? eol+1 : eoc";
> the result of this patch is easier to read.
> 
> By the way, eoc is "one past the end" of the array that begins at
> boc, so setting a pointer to eoc+1 may technically be in violation.
> I do not know how much it matters, though ;-)

I think that is OK. We are reading a strbuf, so eoc must either be the
first character of the PGP signature, or the terminating NUL if there
was no signature block[1]. So it's actually _inside_ the array, and
eoc+1 is our "one past".

-Peff

[1] Arguably we should bail when parse_signature() does not find a PGP
    signature at all. We already bail with "malformed push certificate"
    when there are other syntactic anomalies.

Re: [PATCH v2 16/21] Make sha1_array_append take a struct object_id *

[brian m. carlson]

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

On Tue, Mar 28, 2017 at 10:27:41AM -0700, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
> 
> > Convert the callers to pass struct object_id by changing the function
> > declaration and definition and applying the following semantic patch:
> >
> > @@
> > expression E1, E2, E3;
> > @@
> > - sha1_array_append(E1, E2[E3].hash)
> > + sha1_array_append(E1, E2 + E3)
> >
> > @@
> > expression E1, E2;
> > @@
> > - sha1_array_append(E1, E2.hash)
> > + sha1_array_append(E1, &E2)
> 
> I noticed something similar in the change to bisect.c while reading
> the previous step, and I suspect that the above two rules leave
> somewhat inconsistent and harder-to-read result.  Wouldn't it make
> the result more readable if the former rule were
> 
>     -sha1_array_append(E1, E2[E3].hash)
>     +sha1_array_append(E1, &E2[E3])
> 
> 
> FWIW, the bit that made me read it twice in the previous step was
> this change
> 
> -		strbuf_addstr(&joined_hexs, sha1_to_hex(array->sha1[i]));
> +		strbuf_addstr(&joined_hexs, oid_to_hex(array->oid + i));
> 
> which I would have written &(array->oid[i]) instead.
> 
> After all, the original written by a human said E2[E3].hash (or
> array->sha1[i]) because to the human's mind, E2 is a series of
> things that can be indexed with an int E3, and even though 
> 
>     *(E2 + E3)
>     E2[E3]
>     E3[E2]
> 
> all mean the same thing, the human decided that E2[E3] is the most
> natural way to express this particular reference to an item in the
> array.  &E2[E3] would keep that intention by the original author
> better than E2 + E3.

I'm happy to make that change.  I'm an experienced C programmer, so a
pointer addition seems very readable and natural to me, but if you think
it's better or more readable as &E2[E3], I can certainly reroll.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH v2 16/21] Make sha1_array_append take a struct object_id *

[Junio C Hamano]

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

> On Tue, Mar 28, 2017 at 10:27:41AM -0700, Junio C Hamano wrote:
>> ... 
>> After all, the original written by a human said E2[E3].hash (or
>> array->sha1[i]) because to the human's mind, E2 is a series of
>> things that can be indexed with an int E3, and even though 
>> 
>>     *(E2 + E3)
>>     E2[E3]
>>     E3[E2]
>> 
>> all mean the same thing, the human decided that E2[E3] is the most
>> natural way to express this particular reference to an item in the
>> array.  &E2[E3] would keep that intention by the original author
>> better than E2 + E3.
>
> I'm happy to make that change.  I'm an experienced C programmer, so a
> pointer addition seems very readable and natural to me, but if you think
> it's better or more readable as &E2[E3], I can certainly reroll.

The obvious three possible variants I listed above are equivalent to
the language lawyers and the compiler ;-).

This change is about dropping the need for ".hash", and I think a
faithful, boring and mechanical conversion that tries to preserve
the intent of the original author would be more appropriate.  It is
entirely possible that some places where the original said E2[E3]
were easier to understand if it were *(E2 + E3), thus we may want to
further rewrite such a place to (E2 + E3) instead of &E2[E3] after
the mechanical conversion.

Thanks.

Re: [PATCH v2 16/21] Make sha1_array_append take a struct object_id *

[brian m. carlson]

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

On Wed, Mar 29, 2017 at 08:14:19AM -0700, Junio C Hamano wrote:
> This change is about dropping the need for ".hash", and I think a
> faithful, boring and mechanical conversion that tries to preserve
> the intent of the original author would be more appropriate.  It is
> entirely possible that some places where the original said E2[E3]
> were easier to understand if it were *(E2 + E3), thus we may want to
> further rewrite such a place to (E2 + E3) instead of &E2[E3] after
> the mechanical conversion.

You've convinced me that &E2[E3] is a better choice in this situation,
so I'll reroll and fix the other issues mentioned.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH v2 07/21] builtin/receive-pack: convert portions to struct object_id

[brian m. carlson]

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

On Tue, Mar 28, 2017 at 03:07:12AM -0400, Jeff King wrote:
> It took me a while to find it. This is the switch from "len == 48" to
> "len > 8" when matching "shallow" lines. I think this makes sense.
> 
> > Note that in queue_command we are guaranteed to have a NUL-terminated
> > buffer or at least one byte of overflow that we can safely read, so the
> > linelen check can be elided.  We would die in such a case, but not read
> > invalid memory.
> 
> I think linelen is always just strlen(line). Since the queue_command
> function no longer cares about it, perhaps we can just omit it?

I've just looked at this to put in a fix, and I don't agree.  We are
guaranteed that we'll have overflow, but linelen can point to a newline,
not just a NUL, so it isn't always strlen(line).  This is the case in
queue_commands_from_cert.

We do use this value, in computing the reflen value, so I think it's
better to keep it for now.  I agree that a future refactor could convert
it to be NUL-terminated, but I'd rather not make that change here.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

Re: [PATCH v2 07/21] builtin/receive-pack: convert portions to struct object_id

[Jeff King]

On Wed, Mar 29, 2017 at 11:21:52PM +0000, brian m. carlson wrote:

> On Tue, Mar 28, 2017 at 03:07:12AM -0400, Jeff King wrote:
> > It took me a while to find it. This is the switch from "len == 48" to
> > "len > 8" when matching "shallow" lines. I think this makes sense.
> > 
> > > Note that in queue_command we are guaranteed to have a NUL-terminated
> > > buffer or at least one byte of overflow that we can safely read, so the
> > > linelen check can be elided.  We would die in such a case, but not read
> > > invalid memory.
> > 
> > I think linelen is always just strlen(line). Since the queue_command
> > function no longer cares about it, perhaps we can just omit it?
> 
> I've just looked at this to put in a fix, and I don't agree.  We are
> guaranteed that we'll have overflow, but linelen can point to a newline,
> not just a NUL, so it isn't always strlen(line).  This is the case in
> queue_commands_from_cert.
> 
> We do use this value, in computing the reflen value, so I think it's
> better to keep it for now.  I agree that a future refactor could convert
> it to be NUL-terminated, but I'd rather not make that change here.

Yeah, you might have missed my followup:

  http://public-inbox.org/git/20170328173536.ylwesrj7jbreztcy@sigill.intra.peff.net/

Basically yes, I agree it's probably not worth trying to refactor
further.

-Peff