[PATCH 00/33] object id conversion (grep and diff)

[PATCH 00/33] object id conversion (grep and diff)

[Brandon Williams]

A month or so ago I thought I would lend a hand to Brian and do a round of
conversions from sha1 -> struct object_id.  Now that Brian's latest series has
hit master I can finally send these patches out.

The first couple patches are from Brian which convert some of the notes logic
to using 'struct object_id'.  The remaining patches are to convert the grep and
diff machinery to using 'struct object_id'.

Brandon Williams (26):
  grep: convert to struct object_id
  diff: convert get_stat_data to struct object_id
  diff: convert diff_index_show_file to struct object_id
  diff: convert diff_addremove to struct object_id
  diff: convert run_diff_files to struct object_id
  diff: convert diff_change to struct object_id
  diff: convert fill_filespec to struct object_id
  diff: convert reuse_worktree_file to struct object_id
  diff: finish conversion for prepare_temp_file to struct object_id
  patch-ids: convert to struct object_id
  diff: convert diff_flush_patch_id to struct object_id
  combine-diff: convert diff_tree_combined to struct object_id
  combine-diff: convert find_paths_* to struct object_id
  tree-diff: convert diff_root_tree_sha1 to struct object_id
  notes-merge: convert notes_merge* to struct object_id
  notes-merge: convert merge_from_diffs to struct object_id
  notes-merge: convert find_notes_merge_pair_ps to struct object_id
  notes-merge: convert verify_notes_filepair to struct object_id
  notes-merge: convert write_note_to_worktree to struct object_id
  diff-tree: convert diff_tree_sha1 to struct object_id
  builtin/diff-tree: cleanup references to sha1
  tree-diff: convert try_to_follow_renames to struct object_id
  tree-diff: convert diff_tree_paths to struct object_id
  tree-diff: convert path_appendnew to object_id
  diffcore-rename: use is_empty_blob_oid
  diff: rename diff_fill_sha1_info to diff_fill_oid_info

brian m. carlson (7):
  notes: convert internal structures to struct object_id
  notes: convert internal parts to struct object_id
  notes: convert for_each_note to struct object_id
  notes: make get_note return pointer to struct object_id
  notes: convert format_display_notes to struct object_id
  builtin/notes: convert to struct object_id
  notes: convert some accessor functions to struct object_id

 builtin/am.c          |   2 +-
 builtin/blame.c       |  22 +++---
 builtin/commit.c      |   2 +-
 builtin/diff-tree.c   |  16 ++--
 builtin/diff.c        |   8 +-
 builtin/fast-export.c |   8 +-
 builtin/grep.c        |  22 +++---
 builtin/log.c         |   8 +-
 builtin/merge.c       |   2 +-
 builtin/notes.c       | 136 ++++++++++++++++-----------------
 cache.h               |   7 ++
 combine-diff.c        |  30 ++++----
 diff-lib.c            |  52 ++++++-------
 diff-no-index.c       |   2 +-
 diff.c                |  74 +++++++++---------
 diff.h                |  38 +++++-----
 diffcore-rename.c     |   6 +-
 diffcore.h            |   2 +-
 grep.c                |  19 +++--
 grep.h                |   2 +-
 line-log.c            |  10 +--
 log-tree.c            |  12 +--
 merge-recursive.c     |   2 +-
 notes-cache.c         |  10 +--
 notes-merge.c         | 165 +++++++++++++++++++++--------------------
 notes-merge.h         |  12 +--
 notes-utils.c         |   2 +-
 notes-utils.h         |   2 +-
 notes.c               | 202 ++++++++++++++++++++++++++------------------------
 notes.h               |  16 ++--
 patch-ids.c           |  26 +++----
 patch-ids.h           |   4 +-
 remote-testsvn.c      |  22 +++---
 revision.c            |  16 ++--
 sequencer.c           |   4 +-
 tree-diff.c           |  93 ++++++++++++-----------
 36 files changed, 535 insertions(+), 521 deletions(-)

-- 
2.13.0.219.gdb65acc882-goog

[PATCH 01/33] notes: convert internal structures to struct object_id

[Brandon Williams]

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

Convert the internal structures using unsigned char [20] to take
struct object_id using the following semantic patch and the standard
object_id transforms:

@@
struct leaf_node E1;
@@
- E1.key_sha1
+ E1.key_oid.hash

@@
struct leaf_node *E1;
@@
- E1->key_sha1
+ E1->key_oid.hash

@@
struct leaf_node E1;
@@
- E1.key_sha1
+ E1.key_oid.hash

@@
struct leaf_node *E1;
@@
- E1->key_sha1
+ E1->key_oid.hash

@@
struct non_note E1;
@@
- E1.sha1
+ E1.oid.hash

@@
struct non_note *E1;
@@
- E1->sha1
+ E1->oid.hash

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
 notes.c | 98 ++++++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 51 insertions(+), 47 deletions(-)

diff --git a/notes.c b/notes.c
index 542563b28..251cf11c9 100644
--- a/notes.c
+++ b/notes.c
@@ -35,8 +35,8 @@ struct int_node {
  * subtree.
  */
 struct leaf_node {
-	unsigned char key_sha1[20];
-	unsigned char val_sha1[20];
+	struct object_id key_oid;
+	struct object_id val_oid;
 };
 
 /*
@@ -51,7 +51,7 @@ struct non_note {
 	struct non_note *next; /* grounded (last->next == NULL) */
 	char *path;
 	unsigned int mode;
-	unsigned char sha1[20];
+	struct object_id oid;
 };
 
 #define PTR_TYPE_NULL     0
@@ -100,7 +100,7 @@ static void **note_tree_search(struct notes_tree *t, struct int_node **tree,
 
 	if (GET_PTR_TYPE(p) == PTR_TYPE_SUBTREE) {
 		l = (struct leaf_node *) CLR_PTR_TYPE(p);
-		if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_sha1)) {
+		if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_oid.hash)) {
 			/* unpack tree and resume search */
 			(*tree)->a[0] = NULL;
 			load_subtree(t, l, *tree, *n);
@@ -118,7 +118,7 @@ static void **note_tree_search(struct notes_tree *t, struct int_node **tree,
 		return note_tree_search(t, tree, n, key_sha1);
 	case PTR_TYPE_SUBTREE:
 		l = (struct leaf_node *) CLR_PTR_TYPE(p);
-		if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_sha1)) {
+		if (!SUBTREE_SHA1_PREFIXCMP(key_sha1, l->key_oid.hash)) {
 			/* unpack tree and resume search */
 			(*tree)->a[i] = NULL;
 			load_subtree(t, l, *tree, *n);
@@ -143,7 +143,7 @@ static struct leaf_node *note_tree_find(struct notes_tree *t,
 	void **p = note_tree_search(t, &tree, &n, key_sha1);
 	if (GET_PTR_TYPE(*p) == PTR_TYPE_NOTE) {
 		struct leaf_node *l = (struct leaf_node *) CLR_PTR_TYPE(*p);
-		if (!hashcmp(key_sha1, l->key_sha1))
+		if (!hashcmp(key_sha1, l->key_oid.hash))
 			return l;
 	}
 	return NULL;
@@ -196,17 +196,17 @@ static void note_tree_remove(struct notes_tree *t,
 	struct leaf_node *l;
 	struct int_node *parent_stack[20];
 	unsigned char i, j;
-	void **p = note_tree_search(t, &tree, &n, entry->key_sha1);
+	void **p = note_tree_search(t, &tree, &n, entry->key_oid.hash);
 
 	assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
 	if (GET_PTR_TYPE(*p) != PTR_TYPE_NOTE)
 		return; /* type mismatch, nothing to remove */
 	l = (struct leaf_node *) CLR_PTR_TYPE(*p);
-	if (hashcmp(l->key_sha1, entry->key_sha1))
+	if (oidcmp(&l->key_oid, &entry->key_oid))
 		return; /* key mismatch, nothing to remove */
 
 	/* we have found a matching entry */
-	hashcpy(entry->val_sha1, l->val_sha1);
+	oidcpy(&entry->val_oid, &l->val_oid);
 	free(l);
 	*p = SET_PTR_TYPE(NULL, PTR_TYPE_NULL);
 
@@ -216,14 +216,14 @@ static void note_tree_remove(struct notes_tree *t,
 	/* first, build stack of ancestors between root and current node */
 	parent_stack[0] = t->root;
 	for (i = 0; i < n; i++) {
-		j = GET_NIBBLE(i, entry->key_sha1);
+		j = GET_NIBBLE(i, entry->key_oid.hash);
 		parent_stack[i + 1] = CLR_PTR_TYPE(parent_stack[i]->a[j]);
 	}
 	assert(i == n && parent_stack[i] == tree);
 	/* next, unwind stack until note_tree_consolidate() is done */
 	while (i > 0 &&
 	       !note_tree_consolidate(parent_stack[i], parent_stack[i - 1],
-				      GET_NIBBLE(i - 1, entry->key_sha1)))
+				      GET_NIBBLE(i - 1, entry->key_oid.hash)))
 		i--;
 }
 
@@ -246,7 +246,7 @@ static int note_tree_insert(struct notes_tree *t, struct int_node *tree,
 {
 	struct int_node *new_node;
 	struct leaf_node *l;
-	void **p = note_tree_search(t, &tree, &n, entry->key_sha1);
+	void **p = note_tree_search(t, &tree, &n, entry->key_oid.hash);
 	int ret = 0;
 
 	assert(GET_PTR_TYPE(entry) == 0); /* no type bits set */
@@ -254,7 +254,7 @@ static int note_tree_insert(struct notes_tree *t, struct int_node *tree,
 	switch (GET_PTR_TYPE(*p)) {
 	case PTR_TYPE_NULL:
 		assert(!*p);
-		if (is_null_sha1(entry->val_sha1))
+		if (is_null_oid(&entry->val_oid))
 			free(entry);
 		else
 			*p = SET_PTR_TYPE(entry, type);
@@ -262,22 +262,22 @@ static int note_tree_insert(struct notes_tree *t, struct int_node *tree,
 	case PTR_TYPE_NOTE:
 		switch (type) {
 		case PTR_TYPE_NOTE:
-			if (!hashcmp(l->key_sha1, entry->key_sha1)) {
+			if (!oidcmp(&l->key_oid, &entry->key_oid)) {
 				/* skip concatenation if l == entry */
-				if (!hashcmp(l->val_sha1, entry->val_sha1))
+				if (!oidcmp(&l->val_oid, &entry->val_oid))
 					return 0;
 
-				ret = combine_notes(l->val_sha1,
-						    entry->val_sha1);
-				if (!ret && is_null_sha1(l->val_sha1))
+				ret = combine_notes(l->val_oid.hash,
+						    entry->val_oid.hash);
+				if (!ret && is_null_oid(&l->val_oid))
 					note_tree_remove(t, tree, n, entry);
 				free(entry);
 				return ret;
 			}
 			break;
 		case PTR_TYPE_SUBTREE:
-			if (!SUBTREE_SHA1_PREFIXCMP(l->key_sha1,
-						    entry->key_sha1)) {
+			if (!SUBTREE_SHA1_PREFIXCMP(l->key_oid.hash,
+						    entry->key_oid.hash)) {
 				/* unpack 'entry' */
 				load_subtree(t, entry, tree, n);
 				free(entry);
@@ -287,7 +287,7 @@ static int note_tree_insert(struct notes_tree *t, struct int_node *tree,
 		}
 		break;
 	case PTR_TYPE_SUBTREE:
-		if (!SUBTREE_SHA1_PREFIXCMP(entry->key_sha1, l->key_sha1)) {
+		if (!SUBTREE_SHA1_PREFIXCMP(entry->key_oid.hash, l->key_oid.hash)) {
 			/* unpack 'l' and restart insert */
 			*p = NULL;
 			load_subtree(t, l, tree, n);
@@ -301,7 +301,7 @@ static int note_tree_insert(struct notes_tree *t, struct int_node *tree,
 	/* non-matching leaf_node */
 	assert(GET_PTR_TYPE(*p) == PTR_TYPE_NOTE ||
 	       GET_PTR_TYPE(*p) == PTR_TYPE_SUBTREE);
-	if (is_null_sha1(entry->val_sha1)) { /* skip insertion of empty note */
+	if (is_null_oid(&entry->val_oid)) { /* skip insertion of empty note */
 		free(entry);
 		return 0;
 	}
@@ -373,7 +373,7 @@ static void add_non_note(struct notes_tree *t, char *path,
 	n->next = NULL;
 	n->path = path;
 	n->mode = mode;
-	hashcpy(n->sha1, sha1);
+	hashcpy(n->oid.hash, sha1);
 	t->prev_non_note = n;
 
 	if (!t->first_non_note) {
@@ -399,7 +399,7 @@ static void add_non_note(struct notes_tree *t, char *path,
 	if (non_note_cmp(p, n) == 0) { /* n ~= p; overwrite p with n */
 		assert(strcmp(p->path, n->path) == 0);
 		p->mode = n->mode;
-		hashcpy(p->sha1, n->sha1);
+		oidcpy(&p->oid, &n->oid);
 		free(n);
 		t->prev_non_note = p;
 		return;
@@ -422,14 +422,14 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
 	unsigned char type;
 	struct leaf_node *l;
 
-	buf = fill_tree_descriptor(&desc, subtree->val_sha1);
+	buf = fill_tree_descriptor(&desc, subtree->val_oid.hash);
 	if (!buf)
 		die("Could not read %s for notes-index",
-		     sha1_to_hex(subtree->val_sha1));
+		     oid_to_hex(&subtree->val_oid));
 
-	prefix_len = subtree->key_sha1[19];
+	prefix_len = subtree->key_oid.hash[19];
 	assert(prefix_len * 2 >= n);
-	memcpy(object_sha1, subtree->key_sha1, prefix_len);
+	memcpy(object_sha1, subtree->key_oid.hash, prefix_len);
 	while (tree_entry(&desc, &entry)) {
 		path_len = strlen(entry.path);
 		len = get_sha1_hex_segment(entry.path, path_len,
@@ -447,12 +447,12 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
 			type = PTR_TYPE_NOTE;
 			l = (struct leaf_node *)
 				xcalloc(1, sizeof(struct leaf_node));
-			hashcpy(l->key_sha1, object_sha1);
-			hashcpy(l->val_sha1, entry.oid->hash);
+			hashcpy(l->key_oid.hash, object_sha1);
+			oidcpy(&l->val_oid, entry.oid);
 			if (len < 20) {
 				if (!S_ISDIR(entry.mode) || path_len != 2)
 					goto handle_non_note; /* not subtree */
-				l->key_sha1[19] = (unsigned char) len;
+				l->key_oid.hash[19] = (unsigned char) len;
 				type = PTR_TYPE_SUBTREE;
 			}
 			if (note_tree_insert(t, node, n, l, type,
@@ -460,7 +460,7 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
 				die("Failed to load %s %s into notes tree "
 				    "from %s",
 				    type == PTR_TYPE_NOTE ? "note" : "subtree",
-				    sha1_to_hex(l->key_sha1), t->ref);
+				    oid_to_hex(&l->key_oid), t->ref);
 		}
 		continue;
 
@@ -486,7 +486,7 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
 		 */
 		{
 			struct strbuf non_note_path = STRBUF_INIT;
-			const char *q = sha1_to_hex(subtree->key_sha1);
+			const char *q = oid_to_hex(&subtree->key_oid);
 			int i;
 			for (i = 0; i < prefix_len; i++) {
 				strbuf_addch(&non_note_path, *q++);
@@ -599,15 +599,17 @@ static int for_each_note_helper(struct notes_tree *t, struct int_node *tree,
 			    flags & FOR_EACH_NOTE_YIELD_SUBTREES) {
 				/* invoke callback with subtree */
 				unsigned int path_len =
-					l->key_sha1[19] * 2 + fanout;
+					l->key_oid.hash[19] * 2 + fanout;
 				assert(path_len < FANOUT_PATH_MAX - 1);
-				construct_path_with_fanout(l->key_sha1, fanout,
+				construct_path_with_fanout(l->key_oid.hash,
+							   fanout,
 							   path);
 				/* Create trailing slash, if needed */
 				if (path[path_len - 1] != '/')
 					path[path_len++] = '/';
 				path[path_len] = '\0';
-				ret = fn(l->key_sha1, l->val_sha1, path,
+				ret = fn(l->key_oid.hash, l->val_oid.hash,
+					 path,
 					 cb_data);
 			}
 			if (n > fanout * 2 ||
@@ -621,8 +623,10 @@ static int for_each_note_helper(struct notes_tree *t, struct int_node *tree,
 			break;
 		case PTR_TYPE_NOTE:
 			l = (struct leaf_node *) CLR_PTR_TYPE(p);
-			construct_path_with_fanout(l->key_sha1, fanout, path);
-			ret = fn(l->key_sha1, l->val_sha1, path, cb_data);
+			construct_path_with_fanout(l->key_oid.hash, fanout,
+						   path);
+			ret = fn(l->key_oid.hash, l->val_oid.hash, path,
+				 cb_data);
 			break;
 		}
 		if (ret)
@@ -742,7 +746,7 @@ static int write_each_non_note_until(const char *note_path,
 			; /* do nothing, prefer note to non-note */
 		else {
 			ret = write_each_note_helper(d->root, n->path, n->mode,
-						     n->sha1);
+						     n->oid.hash);
 			if (ret)
 				return ret;
 		}
@@ -1027,8 +1031,8 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
 		die("Failed to read notes tree referenced by %s (%s)",
 		    notes_ref, oid_to_hex(&object_oid));
 
-	hashclr(root_tree.key_sha1);
-	hashcpy(root_tree.val_sha1, oid.hash);
+	oidclr(&root_tree.key_oid);
+	oidcpy(&root_tree.val_oid, &oid);
 	load_subtree(t, &root_tree, t->root, 0);
 }
 
@@ -1092,8 +1096,8 @@ int add_note(struct notes_tree *t, const unsigned char *object_sha1,
 	if (!combine_notes)
 		combine_notes = t->combine_notes;
 	l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
-	hashcpy(l->key_sha1, object_sha1);
-	hashcpy(l->val_sha1, note_sha1);
+	hashcpy(l->key_oid.hash, object_sha1);
+	hashcpy(l->val_oid.hash, note_sha1);
 	return note_tree_insert(t, t->root, 0, l, PTR_TYPE_NOTE, combine_notes);
 }
 
@@ -1104,10 +1108,10 @@ int remove_note(struct notes_tree *t, const unsigned char *object_sha1)
 	if (!t)
 		t = &default_notes_tree;
 	assert(t->initialized);
-	hashcpy(l.key_sha1, object_sha1);
-	hashclr(l.val_sha1);
+	hashcpy(l.key_oid.hash, object_sha1);
+	oidclr(&l.val_oid);
 	note_tree_remove(t, t->root, 0, &l);
-	if (is_null_sha1(l.val_sha1)) /* no note was removed */
+	if (is_null_oid(&l.val_oid)) /* no note was removed */
 		return 1;
 	t->dirty = 1;
 	return 0;
@@ -1122,7 +1126,7 @@ const unsigned char *get_note(struct notes_tree *t,
 		t = &default_notes_tree;
 	assert(t->initialized);
 	found = note_tree_find(t, t->root, 0, object_sha1);
-	return found ? found->val_sha1 : NULL;
+	return found ? found->val_oid.hash : NULL;
 }
 
 int for_each_note(struct notes_tree *t, int flags, each_note_fn fn,
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 02/33] notes: convert internal parts to struct object_id

[Brandon Williams]

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

Convert several portions of the internals of the code to struct
object_id.  Introduce two macros to denote the different constants in
the code: KEY_INDEX for the last byte of the object ID, and
FANOUT_PATH_SEPARATORS for the number of possible path separators (on
Unix, "/").  While these constants are both 19 (one less than the number
of bytes in the hash), distinguish them to make the code more
understandable, and define them logically based on their intended
purpose.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
 notes.c | 64 +++++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 33 insertions(+), 31 deletions(-)

diff --git a/notes.c b/notes.c
index 251cf11c9..babe0c0eb 100644
--- a/notes.c
+++ b/notes.c
@@ -65,8 +65,10 @@ struct non_note {
 
 #define GET_NIBBLE(n, sha1) (((sha1[(n) >> 1]) >> ((~(n) & 0x01) << 2)) & 0x0f)
 
+#define KEY_INDEX (GIT_SHA1_RAWSZ - 1)
+#define FANOUT_PATH_SEPARATORS ((GIT_SHA1_HEXSZ / 2) - 1)
 #define SUBTREE_SHA1_PREFIXCMP(key_sha1, subtree_sha1) \
-	(memcmp(key_sha1, subtree_sha1, subtree_sha1[19]))
+	(memcmp(key_sha1, subtree_sha1, subtree_sha1[KEY_INDEX]))
 
 struct notes_tree default_notes_tree;
 
@@ -194,7 +196,7 @@ static void note_tree_remove(struct notes_tree *t,
 		struct leaf_node *entry)
 {
 	struct leaf_node *l;
-	struct int_node *parent_stack[20];
+	struct int_node *parent_stack[GIT_SHA1_RAWSZ];
 	unsigned char i, j;
 	void **p = note_tree_search(t, &tree, &n, entry->key_oid.hash);
 
@@ -341,21 +343,21 @@ static void note_tree_free(struct int_node *tree)
  * Otherwise, returns number of bytes written to sha1 (i.e. hex_len / 2).
  * Pads sha1 with NULs up to sha1_len (not included in returned length).
  */
-static int get_sha1_hex_segment(const char *hex, unsigned int hex_len,
-		unsigned char *sha1, unsigned int sha1_len)
+static int get_oid_hex_segment(const char *hex, unsigned int hex_len,
+		unsigned char *oid, unsigned int oid_len)
 {
 	unsigned int i, len = hex_len >> 1;
-	if (hex_len % 2 != 0 || len > sha1_len)
+	if (hex_len % 2 != 0 || len > oid_len)
 		return -1;
 	for (i = 0; i < len; i++) {
 		unsigned int val = (hexval(hex[0]) << 4) | hexval(hex[1]);
 		if (val & ~0xff)
 			return -1;
-		*sha1++ = val;
+		*oid++ = val;
 		hex += 2;
 	}
-	for (; i < sha1_len; i++)
-		*sha1++ = 0;
+	for (; i < oid_len; i++)
+		*oid++ = 0;
 	return len;
 }
 
@@ -413,7 +415,7 @@ static void add_non_note(struct notes_tree *t, char *path,
 static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
 		struct int_node *node, unsigned int n)
 {
-	unsigned char object_sha1[20];
+	struct object_id object_oid;
 	unsigned int prefix_len;
 	void *buf;
 	struct tree_desc desc;
@@ -427,13 +429,13 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
 		die("Could not read %s for notes-index",
 		     oid_to_hex(&subtree->val_oid));
 
-	prefix_len = subtree->key_oid.hash[19];
+	prefix_len = subtree->key_oid.hash[KEY_INDEX];
 	assert(prefix_len * 2 >= n);
-	memcpy(object_sha1, subtree->key_oid.hash, prefix_len);
+	memcpy(object_oid.hash, subtree->key_oid.hash, prefix_len);
 	while (tree_entry(&desc, &entry)) {
 		path_len = strlen(entry.path);
-		len = get_sha1_hex_segment(entry.path, path_len,
-				object_sha1 + prefix_len, 20 - prefix_len);
+		len = get_oid_hex_segment(entry.path, path_len,
+				object_oid.hash + prefix_len, GIT_SHA1_RAWSZ - prefix_len);
 		if (len < 0)
 			goto handle_non_note; /* entry.path is not a SHA1 */
 		len += prefix_len;
@@ -443,16 +445,16 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
 		 * If object SHA1 is incomplete (len < 20), and current
 		 * component consists of 2 hex chars, assume note subtree
 		 */
-		if (len <= 20) {
+		if (len <= GIT_SHA1_RAWSZ) {
 			type = PTR_TYPE_NOTE;
 			l = (struct leaf_node *)
 				xcalloc(1, sizeof(struct leaf_node));
-			hashcpy(l->key_oid.hash, object_sha1);
+			oidcpy(&l->key_oid, &object_oid);
 			oidcpy(&l->val_oid, entry.oid);
-			if (len < 20) {
+			if (len < GIT_SHA1_RAWSZ) {
 				if (!S_ISDIR(entry.mode) || path_len != 2)
 					goto handle_non_note; /* not subtree */
-				l->key_oid.hash[19] = (unsigned char) len;
+				l->key_oid.hash[KEY_INDEX] = (unsigned char) len;
 				type = PTR_TYPE_SUBTREE;
 			}
 			if (note_tree_insert(t, node, n, l, type,
@@ -542,14 +544,14 @@ static unsigned char determine_fanout(struct int_node *tree, unsigned char n,
 }
 
 /* hex SHA1 + 19 * '/' + NUL */
-#define FANOUT_PATH_MAX 40 + 19 + 1
+#define FANOUT_PATH_MAX GIT_SHA1_HEXSZ + FANOUT_PATH_SEPARATORS + 1
 
 static void construct_path_with_fanout(const unsigned char *sha1,
 		unsigned char fanout, char *path)
 {
 	unsigned int i = 0, j = 0;
 	const char *hex_sha1 = sha1_to_hex(sha1);
-	assert(fanout < 20);
+	assert(fanout < GIT_SHA1_RAWSZ);
 	while (fanout) {
 		path[i++] = hex_sha1[j++];
 		path[i++] = hex_sha1[j++];
@@ -599,7 +601,7 @@ static int for_each_note_helper(struct notes_tree *t, struct int_node *tree,
 			    flags & FOR_EACH_NOTE_YIELD_SUBTREES) {
 				/* invoke callback with subtree */
 				unsigned int path_len =
-					l->key_oid.hash[19] * 2 + fanout;
+					l->key_oid.hash[KEY_INDEX] * 2 + fanout;
 				assert(path_len < FANOUT_PATH_MAX - 1);
 				construct_path_with_fanout(l->key_oid.hash,
 							   fanout,
@@ -654,7 +656,7 @@ static void write_tree_entry(struct strbuf *buf, unsigned int mode,
 		unsigned char *sha1)
 {
 	strbuf_addf(buf, "%o %.*s%c", mode, path_len, path, '\0');
-	strbuf_add(buf, sha1, 20);
+	strbuf_add(buf, sha1, GIT_SHA1_RAWSZ);
 }
 
 static void tree_write_stack_init_subtree(struct tree_write_stack *tws,
@@ -666,7 +668,7 @@ static void tree_write_stack_init_subtree(struct tree_write_stack *tws,
 	n = (struct tree_write_stack *)
 		xmalloc(sizeof(struct tree_write_stack));
 	n->next = NULL;
-	strbuf_init(&n->buf, 256 * (32 + 40)); /* assume 256 entries per tree */
+	strbuf_init(&n->buf, 256 * (32 + GIT_SHA1_HEXSZ)); /* assume 256 entries per tree */
 	n->path[0] = n->path[1] = '\0';
 	tws->next = n;
 	tws->path[0] = path[0];
@@ -677,18 +679,18 @@ static int tree_write_stack_finish_subtree(struct tree_write_stack *tws)
 {
 	int ret;
 	struct tree_write_stack *n = tws->next;
-	unsigned char s[20];
+	struct object_id s;
 	if (n) {
 		ret = tree_write_stack_finish_subtree(n);
 		if (ret)
 			return ret;
-		ret = write_sha1_file(n->buf.buf, n->buf.len, tree_type, s);
+		ret = write_sha1_file(n->buf.buf, n->buf.len, tree_type, s.hash);
 		if (ret)
 			return ret;
 		strbuf_release(&n->buf);
 		free(n);
 		tws->next = NULL;
-		write_tree_entry(&tws->buf, 040000, tws->path, 2, s);
+		write_tree_entry(&tws->buf, 040000, tws->path, 2, s.hash);
 		tws->path[0] = tws->path[1] = '\0';
 	}
 	return 0;
@@ -771,7 +773,7 @@ static int write_each_note(const unsigned char *object_sha1,
 		note_path[note_path_len] = '\0';
 		mode = 040000;
 	}
-	assert(note_path_len <= 40 + 19);
+	assert(note_path_len <= GIT_SHA1_HEXSZ + FANOUT_PATH_SEPARATORS);
 
 	/* Weave non-note entries into note entries */
 	return  write_each_non_note_until(note_path, d) ||
@@ -946,8 +948,8 @@ void string_list_add_refs_by_glob(struct string_list *list, const char *glob)
 	if (has_glob_specials(glob)) {
 		for_each_glob_ref(string_list_add_one_ref, glob, list);
 	} else {
-		unsigned char sha1[20];
-		if (get_sha1(glob, sha1))
+		struct object_id oid;
+		if (get_oid(glob, &oid))
 			warning("notes ref %s is invalid", glob);
 		if (!unsorted_string_list_has_string(list, glob))
 			string_list_append(list, glob);
@@ -1150,7 +1152,7 @@ int write_notes_tree(struct notes_tree *t, unsigned char *result)
 
 	/* Prepare for traversal of current notes tree */
 	root.next = NULL; /* last forward entry in list is grounded */
-	strbuf_init(&root.buf, 256 * (32 + 40)); /* assume 256 entries */
+	strbuf_init(&root.buf, 256 * (32 + GIT_SHA1_HEXSZ)); /* assume 256 entries */
 	root.path[0] = root.path[1] = '\0';
 	cb_data.root = &root;
 	cb_data.next_non_note = t->first_non_note;
@@ -1315,9 +1317,9 @@ void expand_notes_ref(struct strbuf *sb)
 
 void expand_loose_notes_ref(struct strbuf *sb)
 {
-	unsigned char object[20];
+	struct object_id object;
 
-	if (get_sha1(sb->buf, object)) {
+	if (get_oid(sb->buf, &object)) {
 		/* fallback to expand_notes_ref */
 		expand_notes_ref(sb);
 	}
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 03/33] notes: convert for_each_note to struct object_id

[Brandon Williams]

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

Convert for_each_note and each of the callbacks to use struct object_id.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/notes.c  |  6 +++---
 notes.c          | 24 ++++++++++++------------
 notes.h          |  4 ++--
 remote-testsvn.c |  8 ++++----
 4 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index f2847c41e..53fe6d34d 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -109,11 +109,11 @@ static void free_note_data(struct note_data *d)
 	strbuf_release(&d->buf);
 }
 
-static int list_each_note(const unsigned char *object_sha1,
-		const unsigned char *note_sha1, char *note_path,
+static int list_each_note(const struct object_id *object_oid,
+		const struct object_id *note_oid, char *note_path,
 		void *cb_data)
 {
-	printf("%s %s\n", sha1_to_hex(note_sha1), sha1_to_hex(object_sha1));
+	printf("%s %s\n", oid_to_hex(note_oid), oid_to_hex(object_oid));
 	return 0;
 }
 
diff --git a/notes.c b/notes.c
index babe0c0eb..e881c10ee 100644
--- a/notes.c
+++ b/notes.c
@@ -610,7 +610,7 @@ static int for_each_note_helper(struct notes_tree *t, struct int_node *tree,
 				if (path[path_len - 1] != '/')
 					path[path_len++] = '/';
 				path[path_len] = '\0';
-				ret = fn(l->key_oid.hash, l->val_oid.hash,
+				ret = fn(&l->key_oid, &l->val_oid,
 					 path,
 					 cb_data);
 			}
@@ -627,7 +627,7 @@ static int for_each_note_helper(struct notes_tree *t, struct int_node *tree,
 			l = (struct leaf_node *) CLR_PTR_TYPE(p);
 			construct_path_with_fanout(l->key_oid.hash, fanout,
 						   path);
-			ret = fn(l->key_oid.hash, l->val_oid.hash, path,
+			ret = fn(&l->key_oid, &l->val_oid, path,
 				 cb_data);
 			break;
 		}
@@ -698,7 +698,7 @@ static int tree_write_stack_finish_subtree(struct tree_write_stack *tws)
 
 static int write_each_note_helper(struct tree_write_stack *tws,
 		const char *path, unsigned int mode,
-		const unsigned char *sha1)
+		const struct object_id *oid)
 {
 	size_t path_len = strlen(path);
 	unsigned int n = 0;
@@ -728,7 +728,7 @@ static int write_each_note_helper(struct tree_write_stack *tws,
 
 	/* Finally add given entry to the current tree object */
 	write_tree_entry(&tws->buf, mode, path + 3 * n, path_len - (3 * n),
-			 sha1);
+			 oid->hash);
 
 	return 0;
 }
@@ -748,7 +748,7 @@ static int write_each_non_note_until(const char *note_path,
 			; /* do nothing, prefer note to non-note */
 		else {
 			ret = write_each_note_helper(d->root, n->path, n->mode,
-						     n->oid.hash);
+						     &n->oid);
 			if (ret)
 				return ret;
 		}
@@ -758,8 +758,8 @@ static int write_each_non_note_until(const char *note_path,
 	return 0;
 }
 
-static int write_each_note(const unsigned char *object_sha1,
-		const unsigned char *note_sha1, char *note_path,
+static int write_each_note(const struct object_id *object_oid,
+		const struct object_id *note_oid, char *note_path,
 		void *cb_data)
 {
 	struct write_each_note_data *d =
@@ -777,7 +777,7 @@ static int write_each_note(const unsigned char *object_sha1,
 
 	/* Weave non-note entries into note entries */
 	return  write_each_non_note_until(note_path, d) ||
-		write_each_note_helper(d->root, note_path, mode, note_sha1);
+		write_each_note_helper(d->root, note_path, mode, note_oid);
 }
 
 struct note_delete_list {
@@ -785,20 +785,20 @@ struct note_delete_list {
 	const unsigned char *sha1;
 };
 
-static int prune_notes_helper(const unsigned char *object_sha1,
-		const unsigned char *note_sha1, char *note_path,
+static int prune_notes_helper(const struct object_id *object_oid,
+		const struct object_id *note_oid, char *note_path,
 		void *cb_data)
 {
 	struct note_delete_list **l = (struct note_delete_list **) cb_data;
 	struct note_delete_list *n;
 
-	if (has_sha1_file(object_sha1))
+	if (has_object_file(object_oid))
 		return 0; /* nothing to do for this note */
 
 	/* failed to find object => prune this note */
 	n = (struct note_delete_list *) xmalloc(sizeof(*n));
 	n->next = *l;
-	n->sha1 = object_sha1;
+	n->sha1 = object_oid->hash;
 	*l = n;
 	return 0;
 }
diff --git a/notes.h b/notes.h
index 5345642cf..6651673ae 100644
--- a/notes.h
+++ b/notes.h
@@ -202,8 +202,8 @@ int copy_note(struct notes_tree *t,
  * - copy_note()
  * - free_notes()
  */
-typedef int each_note_fn(const unsigned char *object_sha1,
-		const unsigned char *note_sha1, char *note_path,
+typedef int each_note_fn(const struct object_id *object_oid,
+		const struct object_id *note_oid, char *note_path,
 		void *cb_data);
 int for_each_note(struct notes_tree *t, int flags, each_note_fn fn,
 		void *cb_data);
diff --git a/remote-testsvn.c b/remote-testsvn.c
index f87bf851b..793c4ad1d 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -99,8 +99,8 @@ static int parse_rev_note(const char *msg, struct rev_note *res)
 	return -1;
 }
 
-static int note2mark_cb(const unsigned char *object_sha1,
-		const unsigned char *note_sha1, char *note_path,
+static int note2mark_cb(const struct object_id *object_oid,
+		const struct object_id *note_oid, char *note_path,
 		void *cb_data)
 {
 	FILE *file = (FILE *)cb_data;
@@ -109,14 +109,14 @@ static int note2mark_cb(const unsigned char *object_sha1,
 	enum object_type type;
 	struct rev_note note;
 
-	if (!(msg = read_sha1_file(note_sha1, &type, &msglen)) ||
+	if (!(msg = read_sha1_file(note_oid->hash, &type, &msglen)) ||
 			!msglen || type != OBJ_BLOB) {
 		free(msg);
 		return 1;
 	}
 	if (parse_rev_note(msg, &note))
 		return 2;
-	if (fprintf(file, ":%d %s\n", note.rev_nr, sha1_to_hex(object_sha1)) < 1)
+	if (fprintf(file, ":%d %s\n", note.rev_nr, oid_to_hex(object_oid)) < 1)
 		return 3;
 	return 0;
 }
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 04/33] notes: make get_note return pointer to struct object_id

[Brandon Williams]

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

Make get_note return a pointer to a const struct object_id.  Add a
defensive check to ensure we don't accidentally dereference a NULL
pointer.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/notes.c  | 22 +++++++++++-----------
 notes-cache.c    |  8 ++++----
 notes.c          | 18 +++++++++---------
 notes.h          |  2 +-
 remote-testsvn.c |  6 +++---
 5 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 53fe6d34d..3d9005b8f 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -351,7 +351,7 @@ static int list(int argc, const char **argv, const char *prefix)
 {
 	struct notes_tree *t;
 	unsigned char object[20];
-	const unsigned char *note;
+	const struct object_id *note;
 	int retval = -1;
 	struct option options[] = {
 		OPT_END()
@@ -372,7 +372,7 @@ static int list(int argc, const char **argv, const char *prefix)
 			die(_("failed to resolve '%s' as a valid ref."), argv[0]);
 		note = get_note(t, object);
 		if (note) {
-			puts(sha1_to_hex(note));
+			puts(oid_to_hex(note));
 			retval = 0;
 		} else
 			retval = error(_("no note found for object %s."),
@@ -392,7 +392,7 @@ static int add(int argc, const char **argv, const char *prefix)
 	const char *object_ref;
 	struct notes_tree *t;
 	unsigned char object[20], new_note[20];
-	const unsigned char *note;
+	const struct object_id *note;
 	struct note_data d = { 0, 0, NULL, STRBUF_INIT };
 	struct option options[] = {
 		{ OPTION_CALLBACK, 'm', "message", &d, N_("message"),
@@ -453,7 +453,7 @@ static int add(int argc, const char **argv, const char *prefix)
 			sha1_to_hex(object));
 	}
 
-	prepare_note_data(object, &d, note);
+	prepare_note_data(object, &d, note->hash);
 	if (d.buf.len || allow_empty) {
 		write_note_data(&d, new_note);
 		if (add_note(t, object, new_note, combine_notes_overwrite))
@@ -474,7 +474,7 @@ static int add(int argc, const char **argv, const char *prefix)
 static int copy(int argc, const char **argv, const char *prefix)
 {
 	int retval = 0, force = 0, from_stdin = 0;
-	const unsigned char *from_note, *note;
+	const struct object_id *from_note, *note;
 	const char *object_ref;
 	unsigned char object[20], from_obj[20];
 	struct notes_tree *t;
@@ -539,7 +539,7 @@ static int copy(int argc, const char **argv, const char *prefix)
 		goto out;
 	}
 
-	if (add_note(t, object, from_note, combine_notes_overwrite))
+	if (add_note(t, object, from_note->hash, combine_notes_overwrite))
 		die("BUG: combine_notes_overwrite failed");
 	commit_notes(t, "Notes added by 'git notes copy'");
 out:
@@ -553,7 +553,7 @@ static int append_edit(int argc, const char **argv, const char *prefix)
 	const char *object_ref;
 	struct notes_tree *t;
 	unsigned char object[20], new_note[20];
-	const unsigned char *note;
+	const struct object_id *note;
 	char *logmsg;
 	const char * const *usage;
 	struct note_data d = { 0, 0, NULL, STRBUF_INIT };
@@ -598,13 +598,13 @@ static int append_edit(int argc, const char **argv, const char *prefix)
 	t = init_notes_check(argv[0], NOTES_INIT_WRITABLE);
 	note = get_note(t, object);
 
-	prepare_note_data(object, &d, edit ? note : NULL);
+	prepare_note_data(object, &d, edit && note ? note->hash : NULL);
 
 	if (note && !edit) {
 		/* Append buf to previous note contents */
 		unsigned long size;
 		enum object_type type;
-		char *prev_buf = read_sha1_file(note, &type, &size);
+		char *prev_buf = read_sha1_file(note->hash, &type, &size);
 
 		strbuf_grow(&d.buf, size + 1);
 		if (d.buf.len && prev_buf && size)
@@ -638,7 +638,7 @@ static int show(int argc, const char **argv, const char *prefix)
 	const char *object_ref;
 	struct notes_tree *t;
 	unsigned char object[20];
-	const unsigned char *note;
+	const struct object_id *note;
 	int retval;
 	struct option options[] = {
 		OPT_END()
@@ -664,7 +664,7 @@ static int show(int argc, const char **argv, const char *prefix)
 		retval = error(_("no note found for object %s."),
 			       sha1_to_hex(object));
 	else {
-		const char *show_args[3] = {"show", sha1_to_hex(note), NULL};
+		const char *show_args[3] = {"show", oid_to_hex(note), NULL};
 		retval = execv_git_cmd(show_args);
 	}
 	free_notes(t);
diff --git a/notes-cache.c b/notes-cache.c
index 2843e9857..6e84a748f 100644
--- a/notes-cache.c
+++ b/notes-cache.c
@@ -69,15 +69,15 @@ int notes_cache_write(struct notes_cache *c)
 char *notes_cache_get(struct notes_cache *c, struct object_id *key_oid,
 		      size_t *outsize)
 {
-	const unsigned char *value_sha1;
+	const struct object_id *value_oid;
 	enum object_type type;
 	char *value;
 	unsigned long size;
 
-	value_sha1 = get_note(&c->tree, key_oid->hash);
-	if (!value_sha1)
+	value_oid = get_note(&c->tree, key_oid->hash);
+	if (!value_oid)
 		return NULL;
-	value = read_sha1_file(value_sha1, &type, &size);
+	value = read_sha1_file(value_oid->hash, &type, &size);
 
 	*outsize = size;
 	return value;
diff --git a/notes.c b/notes.c
index e881c10ee..fe4db2c1e 100644
--- a/notes.c
+++ b/notes.c
@@ -1119,7 +1119,7 @@ int remove_note(struct notes_tree *t, const unsigned char *object_sha1)
 	return 0;
 }
 
-const unsigned char *get_note(struct notes_tree *t,
+const struct object_id *get_note(struct notes_tree *t,
 		const unsigned char *object_sha1)
 {
 	struct leaf_node *found;
@@ -1128,7 +1128,7 @@ const unsigned char *get_note(struct notes_tree *t,
 		t = &default_notes_tree;
 	assert(t->initialized);
 	found = note_tree_find(t, t->root, 0, object_sha1);
-	return found ? found->val_oid.hash : NULL;
+	return found ? &found->val_oid : NULL;
 }
 
 int for_each_note(struct notes_tree *t, int flags, each_note_fn fn,
@@ -1219,7 +1219,7 @@ static void format_note(struct notes_tree *t, const unsigned char *object_sha1,
 			struct strbuf *sb, const char *output_encoding, int raw)
 {
 	static const char utf8[] = "utf-8";
-	const unsigned char *sha1;
+	const struct object_id *oid;
 	char *msg, *msg_p;
 	unsigned long linelen, msglen;
 	enum object_type type;
@@ -1229,11 +1229,11 @@ static void format_note(struct notes_tree *t, const unsigned char *object_sha1,
 	if (!t->initialized)
 		init_notes(t, NULL, NULL, 0);
 
-	sha1 = get_note(t, object_sha1);
-	if (!sha1)
+	oid = get_note(t, object_sha1);
+	if (!oid)
 		return;
 
-	if (!(msg = read_sha1_file(sha1, &type, &msglen)) || type != OBJ_BLOB) {
+	if (!(msg = read_sha1_file(oid->hash, &type, &msglen)) || type != OBJ_BLOB) {
 		free(msg);
 		return;
 	}
@@ -1291,14 +1291,14 @@ int copy_note(struct notes_tree *t,
 	      const unsigned char *from_obj, const unsigned char *to_obj,
 	      int force, combine_notes_fn combine_notes)
 {
-	const unsigned char *note = get_note(t, from_obj);
-	const unsigned char *existing_note = get_note(t, to_obj);
+	const struct object_id *note = get_note(t, from_obj);
+	const struct object_id *existing_note = get_note(t, to_obj);
 
 	if (!force && existing_note)
 		return 1;
 
 	if (note)
-		return add_note(t, to_obj, note, combine_notes);
+		return add_note(t, to_obj, note->hash, combine_notes);
 	else if (existing_note)
 		return add_note(t, to_obj, null_sha1, combine_notes);
 
diff --git a/notes.h b/notes.h
index 6651673ae..c72bb9710 100644
--- a/notes.h
+++ b/notes.h
@@ -140,7 +140,7 @@ int remove_note(struct notes_tree *t, const unsigned char *object_sha1);
  *
  * Return NULL if the given object has no notes.
  */
-const unsigned char *get_note(struct notes_tree *t,
+const struct object_id *get_note(struct notes_tree *t,
 		const unsigned char *object_sha1);
 
 /*
diff --git a/remote-testsvn.c b/remote-testsvn.c
index 793c4ad1d..017af1bd5 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -53,15 +53,15 @@ static void terminate_batch(void)
 /* NOTE: 'ref' refers to a git reference, while 'rev' refers to a svn revision. */
 static char *read_ref_note(const unsigned char sha1[20])
 {
-	const unsigned char *note_sha1;
+	const struct object_id *note_oid;
 	char *msg = NULL;
 	unsigned long msglen;
 	enum object_type type;
 
 	init_notes(NULL, notes_ref, NULL, 0);
-	if (!(note_sha1 = get_note(NULL, sha1)))
+	if (!(note_oid = get_note(NULL, sha1)))
 		return NULL;	/* note tree not found */
-	if (!(msg = read_sha1_file(note_sha1, &type, &msglen)))
+	if (!(msg = read_sha1_file(note_oid->hash, &type, &msglen)))
 		error("Empty notes tree. %s", notes_ref);
 	else if (!msglen || type != OBJ_BLOB) {
 		error("Note contains unusable content. "
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 05/33] notes: convert format_display_notes to struct object_id

[Brandon Williams]

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

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
 log-tree.c | 2 +-
 notes.c    | 8 ++++----
 notes.h    | 2 +-
 revision.c | 2 +-
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/log-tree.c b/log-tree.c
index a4ec11c2b..9c0c64a2d 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -655,7 +655,7 @@ void show_log(struct rev_info *opt)
 		struct strbuf notebuf = STRBUF_INIT;
 
 		raw = (opt->commit_format == CMIT_FMT_USERFORMAT);
-		format_display_notes(commit->object.oid.hash, &notebuf,
+		format_display_notes(&commit->object.oid, &notebuf,
 				     get_log_output_encoding(), raw);
 		ctx.notes_message = notebuf.len
 			? strbuf_detach(&notebuf, NULL)
diff --git a/notes.c b/notes.c
index fe4db2c1e..b5cabafde 100644
--- a/notes.c
+++ b/notes.c
@@ -1215,7 +1215,7 @@ void free_notes(struct notes_tree *t)
  * (raw != 0) gives the %N userformat; otherwise, the note message is given
  * for human consumption.
  */
-static void format_note(struct notes_tree *t, const unsigned char *object_sha1,
+static void format_note(struct notes_tree *t, const struct object_id *object_oid,
 			struct strbuf *sb, const char *output_encoding, int raw)
 {
 	static const char utf8[] = "utf-8";
@@ -1229,7 +1229,7 @@ static void format_note(struct notes_tree *t, const unsigned char *object_sha1,
 	if (!t->initialized)
 		init_notes(t, NULL, NULL, 0);
 
-	oid = get_note(t, object_sha1);
+	oid = get_note(t, object_oid->hash);
 	if (!oid)
 		return;
 
@@ -1277,13 +1277,13 @@ static void format_note(struct notes_tree *t, const unsigned char *object_sha1,
 	free(msg);
 }
 
-void format_display_notes(const unsigned char *object_sha1,
+void format_display_notes(const struct object_id *object_oid,
 			  struct strbuf *sb, const char *output_encoding, int raw)
 {
 	int i;
 	assert(display_notes_trees);
 	for (i = 0; display_notes_trees[i]; i++)
-		format_note(display_notes_trees[i], object_sha1, sb,
+		format_note(display_notes_trees[i], object_oid, sb,
 			    output_encoding, raw);
 }
 
diff --git a/notes.h b/notes.h
index c72bb9710..a66532103 100644
--- a/notes.h
+++ b/notes.h
@@ -277,7 +277,7 @@ void init_display_notes(struct display_notes_opt *opt);
  *
  * You *must* call init_display_notes() before using this function.
  */
-void format_display_notes(const unsigned char *object_sha1,
+void format_display_notes(const struct object_id *object_oid,
 			  struct strbuf *sb, const char *output_encoding, int raw);
 
 /*
diff --git a/revision.c b/revision.c
index b02394530..475d5b2dc 100644
--- a/revision.c
+++ b/revision.c
@@ -2908,7 +2908,7 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
 	if (opt->show_notes) {
 		if (!buf.len)
 			strbuf_addstr(&buf, message);
-		format_display_notes(commit->object.oid.hash, &buf, encoding, 1);
+		format_display_notes(&commit->object.oid, &buf, encoding, 1);
 	}
 
 	/*
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 06/33] builtin/notes: convert to struct object_id

[Brandon Williams]

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

Convert most of the static functions to use struct object_id.  In
addition, convert copy_notes_for_rewrite and its callers.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/am.c     |   2 +-
 builtin/commit.c |   2 +-
 builtin/notes.c  | 110 +++++++++++++++++++++++++++----------------------------
 notes-utils.c    |   4 +-
 notes-utils.h    |   2 +-
 5 files changed, 60 insertions(+), 60 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index 0f63dcab1..d9fdddac4 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -563,7 +563,7 @@ static int copy_notes_for_rebase(const struct am_state *state)
 			goto finish;
 		}
 
-		if (copy_note_for_rewrite(c, from_obj.hash, to_obj.hash))
+		if (copy_note_for_rewrite(c, &from_obj, &to_obj))
 			ret = error(_("Failed to copy notes from '%s' to '%s'"),
 					oid_to_hex(&from_obj), oid_to_hex(&to_obj));
 	}
diff --git a/builtin/commit.c b/builtin/commit.c
index da1ba4c86..758781004 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1809,7 +1809,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		cfg = init_copy_notes_for_rewrite("amend");
 		if (cfg) {
 			/* we are amending, so current_head is not NULL */
-			copy_note_for_rewrite(cfg, current_head->object.oid.hash, oid.hash);
+			copy_note_for_rewrite(cfg, &current_head->object.oid, &oid);
 			finish_copy_notes_for_rewrite(cfg, "Notes added by 'git commit --amend'");
 		}
 		run_rewrite_hook(&current_head->object.oid, &oid);
diff --git a/builtin/notes.c b/builtin/notes.c
index 3d9005b8f..7947a16ed 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -129,10 +129,10 @@ static void copy_obj_to_fd(int fd, const unsigned char *sha1)
 	}
 }
 
-static void write_commented_object(int fd, const unsigned char *object)
+static void write_commented_object(int fd, const struct object_id *object)
 {
 	const char *show_args[5] =
-		{"show", "--stat", "--no-notes", sha1_to_hex(object), NULL};
+		{"show", "--stat", "--no-notes", oid_to_hex(object), NULL};
 	struct child_process show = CHILD_PROCESS_INIT;
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf cbuf = STRBUF_INIT;
@@ -145,7 +145,7 @@ static void write_commented_object(int fd, const unsigned char *object)
 	show.git_cmd = 1;
 	if (start_command(&show))
 		die(_("unable to start 'show' for object '%s'"),
-		    sha1_to_hex(object));
+		    oid_to_hex(object));
 
 	if (strbuf_read(&buf, show.out, 0) < 0)
 		die_errno(_("could not read 'show' output"));
@@ -157,10 +157,10 @@ static void write_commented_object(int fd, const unsigned char *object)
 
 	if (finish_command(&show))
 		die(_("failed to finish 'show' for object '%s'"),
-		    sha1_to_hex(object));
+		    oid_to_hex(object));
 }
 
-static void prepare_note_data(const unsigned char *object, struct note_data *d,
+static void prepare_note_data(const struct object_id *object, struct note_data *d,
 		const unsigned char *old_note)
 {
 	if (d->use_editor || !d->given) {
@@ -243,16 +243,16 @@ static int parse_reuse_arg(const struct option *opt, const char *arg, int unset)
 {
 	struct note_data *d = opt->value;
 	char *buf;
-	unsigned char object[20];
+	struct object_id object;
 	enum object_type type;
 	unsigned long len;
 
 	if (d->buf.len)
 		strbuf_addch(&d->buf, '\n');
 
-	if (get_sha1(arg, object))
+	if (get_oid(arg, &object))
 		die(_("failed to resolve '%s' as a valid ref."), arg);
-	if (!(buf = read_sha1_file(object, &type, &len))) {
+	if (!(buf = read_sha1_file(object.hash, &type, &len))) {
 		free(buf);
 		die(_("failed to read object '%s'."), arg);
 	}
@@ -292,7 +292,7 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd)
 	}
 
 	while (strbuf_getline_lf(&buf, stdin) != EOF) {
-		unsigned char from_obj[20], to_obj[20];
+		struct object_id from_obj, to_obj;
 		struct strbuf **split;
 		int err;
 
@@ -301,15 +301,15 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd)
 			die(_("malformed input line: '%s'."), buf.buf);
 		strbuf_rtrim(split[0]);
 		strbuf_rtrim(split[1]);
-		if (get_sha1(split[0]->buf, from_obj))
+		if (get_oid(split[0]->buf, &from_obj))
 			die(_("failed to resolve '%s' as a valid ref."), split[0]->buf);
-		if (get_sha1(split[1]->buf, to_obj))
+		if (get_oid(split[1]->buf, &to_obj))
 			die(_("failed to resolve '%s' as a valid ref."), split[1]->buf);
 
 		if (rewrite_cmd)
-			err = copy_note_for_rewrite(c, from_obj, to_obj);
+			err = copy_note_for_rewrite(c, &from_obj, &to_obj);
 		else
-			err = copy_note(t, from_obj, to_obj, force,
+			err = copy_note(t, from_obj.hash, to_obj.hash, force,
 					combine_notes_overwrite);
 
 		if (err) {
@@ -350,7 +350,7 @@ static struct notes_tree *init_notes_check(const char *subcommand,
 static int list(int argc, const char **argv, const char *prefix)
 {
 	struct notes_tree *t;
-	unsigned char object[20];
+	struct object_id object;
 	const struct object_id *note;
 	int retval = -1;
 	struct option options[] = {
@@ -368,15 +368,15 @@ static int list(int argc, const char **argv, const char *prefix)
 
 	t = init_notes_check("list", 0);
 	if (argc) {
-		if (get_sha1(argv[0], object))
+		if (get_oid(argv[0], &object))
 			die(_("failed to resolve '%s' as a valid ref."), argv[0]);
-		note = get_note(t, object);
+		note = get_note(t, object.hash);
 		if (note) {
 			puts(oid_to_hex(note));
 			retval = 0;
 		} else
 			retval = error(_("no note found for object %s."),
-				       sha1_to_hex(object));
+				       oid_to_hex(&object));
 	} else
 		retval = for_each_note(t, 0, list_each_note, NULL);
 
@@ -391,7 +391,7 @@ static int add(int argc, const char **argv, const char *prefix)
 	int force = 0, allow_empty = 0;
 	const char *object_ref;
 	struct notes_tree *t;
-	unsigned char object[20], new_note[20];
+	struct object_id object, new_note;
 	const struct object_id *note;
 	struct note_data d = { 0, 0, NULL, STRBUF_INIT };
 	struct option options[] = {
@@ -423,11 +423,11 @@ static int add(int argc, const char **argv, const char *prefix)
 
 	object_ref = argc > 1 ? argv[1] : "HEAD";
 
-	if (get_sha1(object_ref, object))
+	if (get_oid(object_ref, &object))
 		die(_("failed to resolve '%s' as a valid ref."), object_ref);
 
 	t = init_notes_check("add", NOTES_INIT_WRITABLE);
-	note = get_note(t, object);
+	note = get_note(t, object.hash);
 
 	if (note) {
 		if (!force) {
@@ -437,7 +437,7 @@ static int add(int argc, const char **argv, const char *prefix)
 				return error(_("Cannot add notes. "
 					"Found existing notes for object %s. "
 					"Use '-f' to overwrite existing notes"),
-					sha1_to_hex(object));
+					oid_to_hex(&object));
 			}
 			/*
 			 * Redirect to "edit" subcommand.
@@ -450,19 +450,19 @@ static int add(int argc, const char **argv, const char *prefix)
 			return append_edit(argc, argv, prefix);
 		}
 		fprintf(stderr, _("Overwriting existing notes for object %s\n"),
-			sha1_to_hex(object));
+			oid_to_hex(&object));
 	}
 
-	prepare_note_data(object, &d, note->hash);
+	prepare_note_data(&object, &d, note->hash);
 	if (d.buf.len || allow_empty) {
-		write_note_data(&d, new_note);
-		if (add_note(t, object, new_note, combine_notes_overwrite))
+		write_note_data(&d, new_note.hash);
+		if (add_note(t, object.hash, new_note.hash, combine_notes_overwrite))
 			die("BUG: combine_notes_overwrite failed");
 		commit_notes(t, "Notes added by 'git notes add'");
 	} else {
 		fprintf(stderr, _("Removing note for object %s\n"),
-			sha1_to_hex(object));
-		remove_note(t, object);
+			oid_to_hex(&object));
+		remove_note(t, object.hash);
 		commit_notes(t, "Notes removed by 'git notes add'");
 	}
 
@@ -476,7 +476,7 @@ static int copy(int argc, const char **argv, const char *prefix)
 	int retval = 0, force = 0, from_stdin = 0;
 	const struct object_id *from_note, *note;
 	const char *object_ref;
-	unsigned char object[20], from_obj[20];
+	struct object_id object, from_obj;
 	struct notes_tree *t;
 	const char *rewrite_cmd = NULL;
 	struct option options[] = {
@@ -509,37 +509,37 @@ static int copy(int argc, const char **argv, const char *prefix)
 		usage_with_options(git_notes_copy_usage, options);
 	}
 
-	if (get_sha1(argv[0], from_obj))
+	if (get_oid(argv[0], &from_obj))
 		die(_("failed to resolve '%s' as a valid ref."), argv[0]);
 
 	object_ref = 1 < argc ? argv[1] : "HEAD";
 
-	if (get_sha1(object_ref, object))
+	if (get_oid(object_ref, &object))
 		die(_("failed to resolve '%s' as a valid ref."), object_ref);
 
 	t = init_notes_check("copy", NOTES_INIT_WRITABLE);
-	note = get_note(t, object);
+	note = get_note(t, object.hash);
 
 	if (note) {
 		if (!force) {
 			retval = error(_("Cannot copy notes. Found existing "
 				       "notes for object %s. Use '-f' to "
 				       "overwrite existing notes"),
-				       sha1_to_hex(object));
+				       oid_to_hex(&object));
 			goto out;
 		}
 		fprintf(stderr, _("Overwriting existing notes for object %s\n"),
-			sha1_to_hex(object));
+			oid_to_hex(&object));
 	}
 
-	from_note = get_note(t, from_obj);
+	from_note = get_note(t, from_obj.hash);
 	if (!from_note) {
 		retval = error(_("missing notes on source object %s. Cannot "
-			       "copy."), sha1_to_hex(from_obj));
+			       "copy."), oid_to_hex(&from_obj));
 		goto out;
 	}
 
-	if (add_note(t, object, from_note->hash, combine_notes_overwrite))
+	if (add_note(t, object.hash, from_note->hash, combine_notes_overwrite))
 		die("BUG: combine_notes_overwrite failed");
 	commit_notes(t, "Notes added by 'git notes copy'");
 out:
@@ -552,7 +552,7 @@ static int append_edit(int argc, const char **argv, const char *prefix)
 	int allow_empty = 0;
 	const char *object_ref;
 	struct notes_tree *t;
-	unsigned char object[20], new_note[20];
+	struct object_id object, new_note;
 	const struct object_id *note;
 	char *logmsg;
 	const char * const *usage;
@@ -592,13 +592,13 @@ static int append_edit(int argc, const char **argv, const char *prefix)
 
 	object_ref = 1 < argc ? argv[1] : "HEAD";
 
-	if (get_sha1(object_ref, object))
+	if (get_oid(object_ref, &object))
 		die(_("failed to resolve '%s' as a valid ref."), object_ref);
 
 	t = init_notes_check(argv[0], NOTES_INIT_WRITABLE);
-	note = get_note(t, object);
+	note = get_note(t, object.hash);
 
-	prepare_note_data(object, &d, edit && note ? note->hash : NULL);
+	prepare_note_data(&object, &d, edit && note ? note->hash : NULL);
 
 	if (note && !edit) {
 		/* Append buf to previous note contents */
@@ -615,14 +615,14 @@ static int append_edit(int argc, const char **argv, const char *prefix)
 	}
 
 	if (d.buf.len || allow_empty) {
-		write_note_data(&d, new_note);
-		if (add_note(t, object, new_note, combine_notes_overwrite))
+		write_note_data(&d, new_note.hash);
+		if (add_note(t, object.hash, new_note.hash, combine_notes_overwrite))
 			die("BUG: combine_notes_overwrite failed");
 		logmsg = xstrfmt("Notes added by 'git notes %s'", argv[0]);
 	} else {
 		fprintf(stderr, _("Removing note for object %s\n"),
-			sha1_to_hex(object));
-		remove_note(t, object);
+			oid_to_hex(&object));
+		remove_note(t, object.hash);
 		logmsg = xstrfmt("Notes removed by 'git notes %s'", argv[0]);
 	}
 	commit_notes(t, logmsg);
@@ -637,7 +637,7 @@ static int show(int argc, const char **argv, const char *prefix)
 {
 	const char *object_ref;
 	struct notes_tree *t;
-	unsigned char object[20];
+	struct object_id object;
 	const struct object_id *note;
 	int retval;
 	struct option options[] = {
@@ -654,15 +654,15 @@ static int show(int argc, const char **argv, const char *prefix)
 
 	object_ref = argc ? argv[0] : "HEAD";
 
-	if (get_sha1(object_ref, object))
+	if (get_oid(object_ref, &object))
 		die(_("failed to resolve '%s' as a valid ref."), object_ref);
 
 	t = init_notes_check("show", 0);
-	note = get_note(t, object);
+	note = get_note(t, object.hash);
 
 	if (!note)
 		retval = error(_("no note found for object %s."),
-			       sha1_to_hex(object));
+			       oid_to_hex(&object));
 	else {
 		const char *show_args[3] = {"show", oid_to_hex(note), NULL};
 		retval = execv_git_cmd(show_args);
@@ -760,7 +760,7 @@ static int git_config_get_notes_strategy(const char *key,
 static int merge(int argc, const char **argv, const char *prefix)
 {
 	struct strbuf remote_ref = STRBUF_INIT, msg = STRBUF_INIT;
-	unsigned char result_sha1[20];
+	struct object_id result_oid;
 	struct notes_tree *t;
 	struct notes_merge_options o;
 	int do_merge = 0, do_commit = 0, do_abort = 0;
@@ -842,16 +842,16 @@ static int merge(int argc, const char **argv, const char *prefix)
 		    remote_ref.buf, default_notes_ref());
 	strbuf_add(&(o.commit_msg), msg.buf + 7, msg.len - 7); /* skip "notes: " */
 
-	result = notes_merge(&o, t, result_sha1);
+	result = notes_merge(&o, t, result_oid.hash);
 
 	if (result >= 0) /* Merge resulted (trivially) in result_sha1 */
 		/* Update default notes ref with new commit */
-		update_ref(msg.buf, default_notes_ref(), result_sha1, NULL,
+		update_ref(msg.buf, default_notes_ref(), result_oid.hash, NULL,
 			   0, UPDATE_REFS_DIE_ON_ERR);
 	else { /* Merge has unresolved conflicts */
 		const struct worktree *wt;
 		/* Update .git/NOTES_MERGE_PARTIAL with partial merge result */
-		update_ref(msg.buf, "NOTES_MERGE_PARTIAL", result_sha1, NULL,
+		update_ref(msg.buf, "NOTES_MERGE_PARTIAL", result_oid.hash, NULL,
 			   0, UPDATE_REFS_DIE_ON_ERR);
 		/* Store ref-to-be-updated into .git/NOTES_MERGE_REF */
 		wt = find_shared_symref("NOTES_MERGE_REF", default_notes_ref());
@@ -878,10 +878,10 @@ static int merge(int argc, const char **argv, const char *prefix)
 static int remove_one_note(struct notes_tree *t, const char *name, unsigned flag)
 {
 	int status;
-	unsigned char sha1[20];
-	if (get_sha1(name, sha1))
+	struct object_id oid;
+	if (get_oid(name, &oid))
 		return error(_("Failed to resolve '%s' as a valid ref."), name);
-	status = remove_note(t, sha1);
+	status = remove_note(t, oid.hash);
 	if (status)
 		fprintf(stderr, _("Object %s has no note\n"), name);
 	else
diff --git a/notes-utils.c b/notes-utils.c
index 325ff3daa..7d7c22b43 100644
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -155,12 +155,12 @@ struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd)
 }
 
 int copy_note_for_rewrite(struct notes_rewrite_cfg *c,
-			  const unsigned char *from_obj, const unsigned char *to_obj)
+			  const struct object_id *from_obj, const struct object_id *to_obj)
 {
 	int ret = 0;
 	int i;
 	for (i = 0; c->trees[i]; i++)
-		ret = copy_note(c->trees[i], from_obj, to_obj, 1, c->combine) || ret;
+		ret = copy_note(c->trees[i], from_obj->hash, to_obj->hash, 1, c->combine) || ret;
 	return ret;
 }
 
diff --git a/notes-utils.h b/notes-utils.h
index fa538e1d9..119057839 100644
--- a/notes-utils.h
+++ b/notes-utils.h
@@ -40,7 +40,7 @@ struct notes_rewrite_cfg {
 int parse_notes_merge_strategy(const char *v, enum notes_merge_strategy *s);
 struct notes_rewrite_cfg *init_copy_notes_for_rewrite(const char *cmd);
 int copy_note_for_rewrite(struct notes_rewrite_cfg *c,
-			  const unsigned char *from_obj, const unsigned char *to_obj);
+			  const struct object_id *from_obj, const struct object_id *to_obj);
 void finish_copy_notes_for_rewrite(struct notes_rewrite_cfg *c, const char *msg);
 
 #endif
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 07/33] notes: convert some accessor functions to struct object_id

[Brandon Williams]

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

Convert add_note, get_note, and copy_note to take struct object_id.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/notes.c  | 20 ++++++++++----------
 notes-cache.c    |  4 ++--
 notes-merge.c    | 18 +++++++++---------
 notes-utils.c    |  2 +-
 notes.c          | 20 ++++++++++----------
 notes.h          |  8 ++++----
 remote-testsvn.c | 10 +++++-----
 7 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 7947a16ed..b13fc8789 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -309,7 +309,7 @@ static int notes_copy_from_stdin(int force, const char *rewrite_cmd)
 		if (rewrite_cmd)
 			err = copy_note_for_rewrite(c, &from_obj, &to_obj);
 		else
-			err = copy_note(t, from_obj.hash, to_obj.hash, force,
+			err = copy_note(t, &from_obj, &to_obj, force,
 					combine_notes_overwrite);
 
 		if (err) {
@@ -370,7 +370,7 @@ static int list(int argc, const char **argv, const char *prefix)
 	if (argc) {
 		if (get_oid(argv[0], &object))
 			die(_("failed to resolve '%s' as a valid ref."), argv[0]);
-		note = get_note(t, object.hash);
+		note = get_note(t, &object);
 		if (note) {
 			puts(oid_to_hex(note));
 			retval = 0;
@@ -427,7 +427,7 @@ static int add(int argc, const char **argv, const char *prefix)
 		die(_("failed to resolve '%s' as a valid ref."), object_ref);
 
 	t = init_notes_check("add", NOTES_INIT_WRITABLE);
-	note = get_note(t, object.hash);
+	note = get_note(t, &object);
 
 	if (note) {
 		if (!force) {
@@ -456,7 +456,7 @@ static int add(int argc, const char **argv, const char *prefix)
 	prepare_note_data(&object, &d, note->hash);
 	if (d.buf.len || allow_empty) {
 		write_note_data(&d, new_note.hash);
-		if (add_note(t, object.hash, new_note.hash, combine_notes_overwrite))
+		if (add_note(t, &object, &new_note, combine_notes_overwrite))
 			die("BUG: combine_notes_overwrite failed");
 		commit_notes(t, "Notes added by 'git notes add'");
 	} else {
@@ -518,7 +518,7 @@ static int copy(int argc, const char **argv, const char *prefix)
 		die(_("failed to resolve '%s' as a valid ref."), object_ref);
 
 	t = init_notes_check("copy", NOTES_INIT_WRITABLE);
-	note = get_note(t, object.hash);
+	note = get_note(t, &object);
 
 	if (note) {
 		if (!force) {
@@ -532,14 +532,14 @@ static int copy(int argc, const char **argv, const char *prefix)
 			oid_to_hex(&object));
 	}
 
-	from_note = get_note(t, from_obj.hash);
+	from_note = get_note(t, &from_obj);
 	if (!from_note) {
 		retval = error(_("missing notes on source object %s. Cannot "
 			       "copy."), oid_to_hex(&from_obj));
 		goto out;
 	}
 
-	if (add_note(t, object.hash, from_note->hash, combine_notes_overwrite))
+	if (add_note(t, &object, from_note, combine_notes_overwrite))
 		die("BUG: combine_notes_overwrite failed");
 	commit_notes(t, "Notes added by 'git notes copy'");
 out:
@@ -596,7 +596,7 @@ static int append_edit(int argc, const char **argv, const char *prefix)
 		die(_("failed to resolve '%s' as a valid ref."), object_ref);
 
 	t = init_notes_check(argv[0], NOTES_INIT_WRITABLE);
-	note = get_note(t, object.hash);
+	note = get_note(t, &object);
 
 	prepare_note_data(&object, &d, edit && note ? note->hash : NULL);
 
@@ -616,7 +616,7 @@ static int append_edit(int argc, const char **argv, const char *prefix)
 
 	if (d.buf.len || allow_empty) {
 		write_note_data(&d, new_note.hash);
-		if (add_note(t, object.hash, new_note.hash, combine_notes_overwrite))
+		if (add_note(t, &object, &new_note, combine_notes_overwrite))
 			die("BUG: combine_notes_overwrite failed");
 		logmsg = xstrfmt("Notes added by 'git notes %s'", argv[0]);
 	} else {
@@ -658,7 +658,7 @@ static int show(int argc, const char **argv, const char *prefix)
 		die(_("failed to resolve '%s' as a valid ref."), object_ref);
 
 	t = init_notes_check("show", 0);
-	note = get_note(t, object.hash);
+	note = get_note(t, &object);
 
 	if (!note)
 		retval = error(_("no note found for object %s."),
diff --git a/notes-cache.c b/notes-cache.c
index 6e84a748f..29b4cede5 100644
--- a/notes-cache.c
+++ b/notes-cache.c
@@ -74,7 +74,7 @@ char *notes_cache_get(struct notes_cache *c, struct object_id *key_oid,
 	char *value;
 	unsigned long size;
 
-	value_oid = get_note(&c->tree, key_oid->hash);
+	value_oid = get_note(&c->tree, key_oid);
 	if (!value_oid)
 		return NULL;
 	value = read_sha1_file(value_oid->hash, &type, &size);
@@ -90,5 +90,5 @@ int notes_cache_put(struct notes_cache *c, struct object_id *key_oid,
 
 	if (write_sha1_file(data, size, "blob", value_oid.hash) < 0)
 		return -1;
-	return add_note(&c->tree, key_oid->hash, value_oid.hash, NULL);
+	return add_note(&c->tree, key_oid, &value_oid, NULL);
 }
diff --git a/notes-merge.c b/notes-merge.c
index 6244f6af9..9a1a49506 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -444,14 +444,14 @@ static int merge_one_change(struct notes_merge_options *o,
 		if (o->verbosity >= 2)
 			printf("Using remote notes for %s\n",
 						oid_to_hex(&p->obj));
-		if (add_note(t, p->obj.hash, p->remote.hash, combine_notes_overwrite))
+		if (add_note(t, &p->obj, &p->remote, combine_notes_overwrite))
 			die("BUG: combine_notes_overwrite failed");
 		return 0;
 	case NOTES_MERGE_RESOLVE_UNION:
 		if (o->verbosity >= 2)
 			printf("Concatenating local and remote notes for %s\n",
 							oid_to_hex(&p->obj));
-		if (add_note(t, p->obj.hash, p->remote.hash, combine_notes_concatenate))
+		if (add_note(t, &p->obj, &p->remote, combine_notes_concatenate))
 			die("failed to concatenate notes "
 			    "(combine_notes_concatenate)");
 		return 0;
@@ -459,7 +459,7 @@ static int merge_one_change(struct notes_merge_options *o,
 		if (o->verbosity >= 2)
 			printf("Concatenating unique lines in local and remote "
 				"notes for %s\n", oid_to_hex(&p->obj));
-		if (add_note(t, p->obj.hash, p->remote.hash, combine_notes_cat_sort_uniq))
+		if (add_note(t, &p->obj, &p->remote, combine_notes_cat_sort_uniq))
 			die("failed to concatenate notes "
 			    "(combine_notes_cat_sort_uniq)");
 		return 0;
@@ -491,7 +491,7 @@ static int merge_changes(struct notes_merge_options *o,
 			   !oidcmp(&p->local, &p->base)) {
 			/* no local change; adopt remote change */
 			trace_printf("\t\t\tno local change, adopted remote\n");
-			if (add_note(t, p->obj.hash, p->remote.hash,
+			if (add_note(t, &p->obj, &p->remote,
 				     combine_notes_overwrite))
 				die("BUG: combine_notes_overwrite failed");
 		} else {
@@ -693,12 +693,12 @@ int notes_merge_commit(struct notes_merge_options *o,
 	baselen = path.len;
 	while ((e = readdir(dir)) != NULL) {
 		struct stat st;
-		unsigned char obj_sha1[20], blob_sha1[20];
+		struct object_id obj_oid, blob_oid;
 
 		if (is_dot_or_dotdot(e->d_name))
 			continue;
 
-		if (strlen(e->d_name) != 40 || get_sha1_hex(e->d_name, obj_sha1)) {
+		if (get_oid_hex(e->d_name, &obj_oid)) {
 			if (o->verbosity >= 3)
 				printf("Skipping non-SHA1 entry '%s%s'\n",
 					path.buf, e->d_name);
@@ -709,14 +709,14 @@ int notes_merge_commit(struct notes_merge_options *o,
 		/* write file as blob, and add to partial_tree */
 		if (stat(path.buf, &st))
 			die_errno("Failed to stat '%s'", path.buf);
-		if (index_path(blob_sha1, path.buf, &st, HASH_WRITE_OBJECT))
+		if (index_path(blob_oid.hash, path.buf, &st, HASH_WRITE_OBJECT))
 			die("Failed to write blob object from '%s'", path.buf);
-		if (add_note(partial_tree, obj_sha1, blob_sha1, NULL))
+		if (add_note(partial_tree, &obj_oid, &blob_oid, NULL))
 			die("Failed to add resolved note '%s' to notes tree",
 			    path.buf);
 		if (o->verbosity >= 4)
 			printf("Added resolved note for object %s: %s\n",
-				sha1_to_hex(obj_sha1), sha1_to_hex(blob_sha1));
+				oid_to_hex(&obj_oid), oid_to_hex(&blob_oid));
 		strbuf_setlen(&path, baselen);
 	}
 
diff --git a/notes-utils.c b/notes-utils.c
index 7d7c22b43..b2aada90a 100644
--- a/notes-utils.c
+++ b/notes-utils.c
@@ -160,7 +160,7 @@ int copy_note_for_rewrite(struct notes_rewrite_cfg *c,
 	int ret = 0;
 	int i;
 	for (i = 0; c->trees[i]; i++)
-		ret = copy_note(c->trees[i], from_obj->hash, to_obj->hash, 1, c->combine) || ret;
+		ret = copy_note(c->trees[i], from_obj, to_obj, 1, c->combine) || ret;
 	return ret;
 }
 
diff --git a/notes.c b/notes.c
index b5cabafde..4b3a1adda 100644
--- a/notes.c
+++ b/notes.c
@@ -1086,8 +1086,8 @@ void init_display_notes(struct display_notes_opt *opt)
 	string_list_clear(&display_notes_refs, 0);
 }
 
-int add_note(struct notes_tree *t, const unsigned char *object_sha1,
-		const unsigned char *note_sha1, combine_notes_fn combine_notes)
+int add_note(struct notes_tree *t, const struct object_id *object_oid,
+		const struct object_id *note_oid, combine_notes_fn combine_notes)
 {
 	struct leaf_node *l;
 
@@ -1098,8 +1098,8 @@ int add_note(struct notes_tree *t, const unsigned char *object_sha1,
 	if (!combine_notes)
 		combine_notes = t->combine_notes;
 	l = (struct leaf_node *) xmalloc(sizeof(struct leaf_node));
-	hashcpy(l->key_oid.hash, object_sha1);
-	hashcpy(l->val_oid.hash, note_sha1);
+	oidcpy(&l->key_oid, object_oid);
+	oidcpy(&l->val_oid, note_oid);
 	return note_tree_insert(t, t->root, 0, l, PTR_TYPE_NOTE, combine_notes);
 }
 
@@ -1120,14 +1120,14 @@ int remove_note(struct notes_tree *t, const unsigned char *object_sha1)
 }
 
 const struct object_id *get_note(struct notes_tree *t,
-		const unsigned char *object_sha1)
+		const struct object_id *oid)
 {
 	struct leaf_node *found;
 
 	if (!t)
 		t = &default_notes_tree;
 	assert(t->initialized);
-	found = note_tree_find(t, t->root, 0, object_sha1);
+	found = note_tree_find(t, t->root, 0, oid->hash);
 	return found ? &found->val_oid : NULL;
 }
 
@@ -1229,7 +1229,7 @@ static void format_note(struct notes_tree *t, const struct object_id *object_oid
 	if (!t->initialized)
 		init_notes(t, NULL, NULL, 0);
 
-	oid = get_note(t, object_oid->hash);
+	oid = get_note(t, object_oid);
 	if (!oid)
 		return;
 
@@ -1288,7 +1288,7 @@ void format_display_notes(const struct object_id *object_oid,
 }
 
 int copy_note(struct notes_tree *t,
-	      const unsigned char *from_obj, const unsigned char *to_obj,
+	      const struct object_id *from_obj, const struct object_id *to_obj,
 	      int force, combine_notes_fn combine_notes)
 {
 	const struct object_id *note = get_note(t, from_obj);
@@ -1298,9 +1298,9 @@ int copy_note(struct notes_tree *t,
 		return 1;
 
 	if (note)
-		return add_note(t, to_obj, note->hash, combine_notes);
+		return add_note(t, to_obj, note, combine_notes);
 	else if (existing_note)
-		return add_note(t, to_obj, null_sha1, combine_notes);
+		return add_note(t, to_obj, &null_oid, combine_notes);
 
 	return 0;
 }
diff --git a/notes.h b/notes.h
index a66532103..3848c2fb3 100644
--- a/notes.h
+++ b/notes.h
@@ -121,8 +121,8 @@ void init_notes(struct notes_tree *t, const char *notes_ref,
  * are not persistent until a subsequent call to write_notes_tree() returns
  * zero.
  */
-int add_note(struct notes_tree *t, const unsigned char *object_sha1,
-		const unsigned char *note_sha1, combine_notes_fn combine_notes);
+int add_note(struct notes_tree *t, const struct object_id *object_oid,
+		const struct object_id *note_oid, combine_notes_fn combine_notes);
 
 /*
  * Remove the given note object from the given notes_tree structure
@@ -141,7 +141,7 @@ int remove_note(struct notes_tree *t, const unsigned char *object_sha1);
  * Return NULL if the given object has no notes.
  */
 const struct object_id *get_note(struct notes_tree *t,
-		const unsigned char *object_sha1);
+		const struct object_id *object_oid);
 
 /*
  * Copy a note from one object to another in the given notes_tree.
@@ -156,7 +156,7 @@ const struct object_id *get_note(struct notes_tree *t,
  * zero.
  */
 int copy_note(struct notes_tree *t,
-	      const unsigned char *from_obj, const unsigned char *to_obj,
+	      const struct object_id *from_obj, const struct object_id *to_obj,
 	      int force, combine_notes_fn combine_notes);
 
 /*
diff --git a/remote-testsvn.c b/remote-testsvn.c
index 017af1bd5..8e8d5c794 100644
--- a/remote-testsvn.c
+++ b/remote-testsvn.c
@@ -51,7 +51,7 @@ static void terminate_batch(void)
 }
 
 /* NOTE: 'ref' refers to a git reference, while 'rev' refers to a svn revision. */
-static char *read_ref_note(const unsigned char sha1[20])
+static char *read_ref_note(const struct object_id *oid)
 {
 	const struct object_id *note_oid;
 	char *msg = NULL;
@@ -59,7 +59,7 @@ static char *read_ref_note(const unsigned char sha1[20])
 	enum object_type type;
 
 	init_notes(NULL, notes_ref, NULL, 0);
-	if (!(note_oid = get_note(NULL, sha1)))
+	if (!(note_oid = get_note(NULL, oid)))
 		return NULL;	/* note tree not found */
 	if (!(msg = read_sha1_file(note_oid->hash, &type, &msglen)))
 		error("Empty notes tree. %s", notes_ref);
@@ -174,15 +174,15 @@ static int cmd_import(const char *line)
 	int code;
 	int dumpin_fd;
 	char *note_msg;
-	unsigned char head_sha1[20];
+	struct object_id head_oid;
 	unsigned int startrev;
 	struct child_process svndump_proc = CHILD_PROCESS_INIT;
 	const char *command = "svnrdump";
 
-	if (read_ref(private_ref, head_sha1))
+	if (read_ref(private_ref, head_oid.hash))
 		startrev = 0;
 	else {
-		note_msg = read_ref_note(head_sha1);
+		note_msg = read_ref_note(&head_oid);
 		if(note_msg == NULL) {
 			warning("No note found for %s.", private_ref);
 			startrev = 0;
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 08/33] grep: convert to struct object_id

[Brandon Williams]

Convert the remaining parts of grep to use struct object_id.

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/grep.c | 22 +++++++++++-----------
 cache.h        |  7 +++++++
 grep.c         | 17 ++++++++---------
 grep.h         |  2 +-
 4 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index c6c26e9b9..623c13a93 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -327,7 +327,7 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid,
 
 #ifndef NO_PTHREADS
 	if (num_threads) {
-		add_work(opt, GREP_SOURCE_SHA1, pathbuf.buf, path, oid);
+		add_work(opt, GREP_SOURCE_OID, pathbuf.buf, path, oid);
 		strbuf_release(&pathbuf);
 		return 0;
 	} else
@@ -336,7 +336,7 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid,
 		struct grep_source gs;
 		int hit;
 
-		grep_source_init(&gs, GREP_SOURCE_SHA1, pathbuf.buf, path, oid);
+		grep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid);
 		strbuf_release(&pathbuf);
 		hit = grep_source(opt, &gs);
 
@@ -570,7 +570,7 @@ static int grep_submodule_launch(struct grep_opt *opt,
 	 * with the object's name: 'tree-name:filename'.  In order to
 	 * provide uniformity of output we want to pass the name of the
 	 * parent project's object name to the submodule so the submodule can
-	 * prefix its output with the parent's name and not its own SHA1.
+	 * prefix its output with the parent's name and not its own OID.
 	 */
 	if (gs->identifier && end_of_base)
 		argv_array_pushf(&cp.args, "--parent-basename=%.*s",
@@ -583,12 +583,12 @@ static int grep_submodule_launch(struct grep_opt *opt,
 		 * If there is a tree identifier for the submodule, add the
 		 * rev after adding the submodule options but before the
 		 * pathspecs.  To do this we listen for the '--' and insert the
-		 * sha1 before pushing the '--' onto the child process argv
+		 * oid before pushing the '--' onto the child process argv
 		 * array.
 		 */
 		if (gs->identifier &&
 		    !strcmp("--", submodule_options.argv[i])) {
-			argv_array_push(&cp.args, sha1_to_hex(gs->identifier));
+			argv_array_push(&cp.args, oid_to_hex(gs->identifier));
 		}
 
 		argv_array_push(&cp.args, submodule_options.argv[i]);
@@ -618,11 +618,11 @@ static int grep_submodule_launch(struct grep_opt *opt,
 
 /*
  * Prep grep structures for a submodule grep
- * sha1: the sha1 of the submodule or NULL if using the working tree
+ * oid: the oid of the submodule or NULL if using the working tree
  * filename: name of the submodule including tree name of parent
  * path: location of the submodule
  */
-static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
+static int grep_submodule(struct grep_opt *opt, const struct object_id *oid,
 			  const char *filename, const char *path)
 {
 	if (!is_submodule_initialized(path))
@@ -632,7 +632,7 @@ static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
 		 * If searching history, check for the presense of the
 		 * submodule's gitdir before skipping the submodule.
 		 */
-		if (sha1) {
+		if (oid) {
 			const struct submodule *sub =
 					submodule_from_path(null_sha1, path);
 			if (sub)
@@ -647,7 +647,7 @@ static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
 
 #ifndef NO_PTHREADS
 	if (num_threads) {
-		add_work(opt, GREP_SOURCE_SUBMODULE, filename, path, sha1);
+		add_work(opt, GREP_SOURCE_SUBMODULE, filename, path, oid);
 		return 0;
 	} else
 #endif
@@ -656,7 +656,7 @@ static int grep_submodule(struct grep_opt *opt, const unsigned char *sha1,
 		int hit;
 
 		grep_source_init(&gs, GREP_SOURCE_SUBMODULE,
-				 filename, path, sha1);
+				 filename, path, oid);
 		hit = grep_submodule_launch(opt, &gs);
 
 		grep_source_clear(&gs);
@@ -775,7 +775,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 					 check_attr);
 			free(data);
 		} else if (recurse_submodules && S_ISGITLINK(entry.mode)) {
-			hit |= grep_submodule(opt, entry.oid->hash, base->buf,
+			hit |= grep_submodule(opt, entry.oid, base->buf,
 					      base->buf + tn_len);
 		}
 
diff --git a/cache.h b/cache.h
index ae4c45d37..50fd2b3cc 100644
--- a/cache.h
+++ b/cache.h
@@ -1026,6 +1026,13 @@ static inline void oidcpy(struct object_id *dst, const struct object_id *src)
 	hashcpy(dst->hash, src->hash);
 }
 
+static inline struct object_id *oiddup(const struct object_id *src)
+{
+	struct object_id *dst = xmalloc(sizeof(struct object_id));
+	oidcpy(dst, src);
+	return dst;
+}
+
 static inline void hashclr(unsigned char *hash)
 {
 	memset(hash, 0, GIT_SHA1_RAWSZ);
diff --git a/grep.c b/grep.c
index 47cee4506..a240b4cdb 100644
--- a/grep.c
+++ b/grep.c
@@ -1403,7 +1403,7 @@ static int fill_textconv_grep(struct userdiff_driver *driver,
 	 */
 	df = alloc_filespec(gs->path);
 	switch (gs->type) {
-	case GREP_SOURCE_SHA1:
+	case GREP_SOURCE_OID:
 		fill_filespec(df, gs->identifier, 1, 0100644);
 		break;
 	case GREP_SOURCE_FILE:
@@ -1747,9 +1747,8 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type,
 		 * If the identifier is non-NULL (in the submodule case) it
 		 * will be a SHA1 that needs to be copied.
 		 */
-	case GREP_SOURCE_SHA1:
-		gs->identifier = xmalloc(20);
-		hashcpy(gs->identifier, identifier);
+	case GREP_SOURCE_OID:
+		gs->identifier = oiddup(identifier);
 		break;
 	case GREP_SOURCE_BUF:
 		gs->identifier = NULL;
@@ -1772,7 +1771,7 @@ void grep_source_clear_data(struct grep_source *gs)
 {
 	switch (gs->type) {
 	case GREP_SOURCE_FILE:
-	case GREP_SOURCE_SHA1:
+	case GREP_SOURCE_OID:
 	case GREP_SOURCE_SUBMODULE:
 		free(gs->buf);
 		gs->buf = NULL;
@@ -1784,7 +1783,7 @@ void grep_source_clear_data(struct grep_source *gs)
 	}
 }
 
-static int grep_source_load_sha1(struct grep_source *gs)
+static int grep_source_load_oid(struct grep_source *gs)
 {
 	enum object_type type;
 
@@ -1795,7 +1794,7 @@ static int grep_source_load_sha1(struct grep_source *gs)
 	if (!gs->buf)
 		return error(_("'%s': unable to read %s"),
 			     gs->name,
-			     sha1_to_hex(gs->identifier));
+			     oid_to_hex(gs->identifier));
 	return 0;
 }
 
@@ -1841,8 +1840,8 @@ static int grep_source_load(struct grep_source *gs)
 	switch (gs->type) {
 	case GREP_SOURCE_FILE:
 		return grep_source_load_file(gs);
-	case GREP_SOURCE_SHA1:
-		return grep_source_load_sha1(gs);
+	case GREP_SOURCE_OID:
+		return grep_source_load_oid(gs);
 	case GREP_SOURCE_BUF:
 		return gs->buf ? 0 : -1;
 	case GREP_SOURCE_SUBMODULE:
diff --git a/grep.h b/grep.h
index 267534ca2..c88b40bdc 100644
--- a/grep.h
+++ b/grep.h
@@ -158,7 +158,7 @@ struct grep_source {
 	char *name;
 
 	enum grep_source_type {
-		GREP_SOURCE_SHA1,
+		GREP_SOURCE_OID,
 		GREP_SOURCE_FILE,
 		GREP_SOURCE_BUF,
 		GREP_SOURCE_SUBMODULE,
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 09/33] diff: convert get_stat_data to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 diff-lib.c | 26 +++++++++++++-------------
 1 file changed, 13 insertions(+), 13 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 2982bf055..a3bc78162 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -264,12 +264,12 @@ static void diff_index_show_file(struct rev_info *revs,
 }
 
 static int get_stat_data(const struct cache_entry *ce,
-			 const unsigned char **sha1p,
+			 const struct object_id **oidp,
 			 unsigned int *modep,
 			 int cached, int match_missing,
 			 unsigned *dirty_submodule, struct diff_options *diffopt)
 {
-	const unsigned char *sha1 = ce->oid.hash;
+	const struct object_id *oid = &ce->oid;
 	unsigned int mode = ce->ce_mode;
 
 	if (!cached && !ce_uptodate(ce)) {
@@ -280,7 +280,7 @@ static int get_stat_data(const struct cache_entry *ce,
 			return -1;
 		else if (changed) {
 			if (match_missing) {
-				*sha1p = sha1;
+				*oidp = oid;
 				*modep = mode;
 				return 0;
 			}
@@ -290,11 +290,11 @@ static int get_stat_data(const struct cache_entry *ce,
 						    0, dirty_submodule);
 		if (changed) {
 			mode = ce_mode_from_stat(ce, st.st_mode);
-			sha1 = null_sha1;
+			oid = &null_oid;
 		}
 	}
 
-	*sha1p = sha1;
+	*oidp = oid;
 	*modep = mode;
 	return 0;
 }
@@ -303,7 +303,7 @@ static void show_new_file(struct rev_info *revs,
 			  const struct cache_entry *new,
 			  int cached, int match_missing)
 {
-	const unsigned char *sha1;
+	const struct object_id *oid;
 	unsigned int mode;
 	unsigned dirty_submodule = 0;
 
@@ -311,11 +311,11 @@ static void show_new_file(struct rev_info *revs,
 	 * New file in the index: it might actually be different in
 	 * the working tree.
 	 */
-	if (get_stat_data(new, &sha1, &mode, cached, match_missing,
+	if (get_stat_data(new, &oid, &mode, cached, match_missing,
 	    &dirty_submodule, &revs->diffopt) < 0)
 		return;
 
-	diff_index_show_file(revs, "+", new, sha1, !is_null_sha1(sha1), mode, dirty_submodule);
+	diff_index_show_file(revs, "+", new, oid->hash, !is_null_oid(oid), mode, dirty_submodule);
 }
 
 static int show_modified(struct rev_info *revs,
@@ -325,10 +325,10 @@ static int show_modified(struct rev_info *revs,
 			 int cached, int match_missing)
 {
 	unsigned int mode, oldmode;
-	const unsigned char *sha1;
+	const struct object_id *oid;
 	unsigned dirty_submodule = 0;
 
-	if (get_stat_data(new, &sha1, &mode, cached, match_missing,
+	if (get_stat_data(new, &oid, &mode, cached, match_missing,
 			  &dirty_submodule, &revs->diffopt) < 0) {
 		if (report_missing)
 			diff_index_show_file(revs, "-", old,
@@ -338,7 +338,7 @@ static int show_modified(struct rev_info *revs,
 	}
 
 	if (revs->combine_merges && !cached &&
-	    (hashcmp(sha1, old->oid.hash) || oidcmp(&old->oid, &new->oid))) {
+	    (oidcmp(oid, &old->oid) || oidcmp(&old->oid, &new->oid))) {
 		struct combine_diff_path *p;
 		int pathlen = ce_namelen(new);
 
@@ -362,12 +362,12 @@ static int show_modified(struct rev_info *revs,
 	}
 
 	oldmode = old->ce_mode;
-	if (mode == oldmode && !hashcmp(sha1, old->oid.hash) && !dirty_submodule &&
+	if (mode == oldmode && !oidcmp(oid, &old->oid) && !dirty_submodule &&
 	    !DIFF_OPT_TST(&revs->diffopt, FIND_COPIES_HARDER))
 		return 0;
 
 	diff_change(&revs->diffopt, oldmode, mode,
-		    old->oid.hash, sha1, 1, !is_null_sha1(sha1),
+		    old->oid.hash, oid->hash, 1, !is_null_oid(oid),
 		    old->name, 0, dirty_submodule);
 	return 0;
 }
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 10/33] diff: convert diff_index_show_file to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 diff-lib.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index a3bc78162..2c838aaf4 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -255,12 +255,12 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 static void diff_index_show_file(struct rev_info *revs,
 				 const char *prefix,
 				 const struct cache_entry *ce,
-				 const unsigned char *sha1, int sha1_valid,
+				 const struct object_id *oid, int oid_valid,
 				 unsigned int mode,
 				 unsigned dirty_submodule)
 {
 	diff_addremove(&revs->diffopt, prefix[0], mode,
-		       sha1, sha1_valid, ce->name, dirty_submodule);
+		       oid->hash, oid_valid, ce->name, dirty_submodule);
 }
 
 static int get_stat_data(const struct cache_entry *ce,
@@ -315,7 +315,7 @@ static void show_new_file(struct rev_info *revs,
 	    &dirty_submodule, &revs->diffopt) < 0)
 		return;
 
-	diff_index_show_file(revs, "+", new, oid->hash, !is_null_oid(oid), mode, dirty_submodule);
+	diff_index_show_file(revs, "+", new, oid, !is_null_oid(oid), mode, dirty_submodule);
 }
 
 static int show_modified(struct rev_info *revs,
@@ -332,7 +332,7 @@ static int show_modified(struct rev_info *revs,
 			  &dirty_submodule, &revs->diffopt) < 0) {
 		if (report_missing)
 			diff_index_show_file(revs, "-", old,
-					     old->oid.hash, 1, old->ce_mode,
+					     &old->oid, 1, old->ce_mode,
 					     0);
 		return -1;
 	}
@@ -426,7 +426,7 @@ static void do_oneway_diff(struct unpack_trees_options *o,
 	 * Something removed from the tree?
 	 */
 	if (!idx) {
-		diff_index_show_file(revs, "-", tree, tree->oid.hash, 1,
+		diff_index_show_file(revs, "-", tree, &tree->oid, 1,
 				     tree->ce_mode, 0);
 		return;
 	}
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 11/33] diff: convert diff_addremove to struct object_id

[Brandon Williams]

Convert diff_addremove to take a struct object_id.  In addtion convert
the function pointer type 'add_remove_fn_t' to also take a struct
object_id.

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 diff-lib.c  | 6 +++---
 diff.c      | 8 ++++----
 diff.h      | 8 ++++----
 revision.c  | 4 ++--
 tree-diff.c | 8 ++++----
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 2c838aaf4..7984ff962 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -210,14 +210,14 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 					continue;
 				}
 				diff_addremove(&revs->diffopt, '-', ce->ce_mode,
-					       ce->oid.hash,
+					       &ce->oid,
 					       !is_null_oid(&ce->oid),
 					       ce->name, 0);
 				continue;
 			} else if (revs->diffopt.ita_invisible_in_index &&
 				   ce_intent_to_add(ce)) {
 				diff_addremove(&revs->diffopt, '+', ce->ce_mode,
-					       EMPTY_BLOB_SHA1_BIN, 0,
+					       &empty_tree_oid, 0,
 					       ce->name, 0);
 				continue;
 			}
@@ -260,7 +260,7 @@ static void diff_index_show_file(struct rev_info *revs,
 				 unsigned dirty_submodule)
 {
 	diff_addremove(&revs->diffopt, prefix[0], mode,
-		       oid->hash, oid_valid, ce->name, dirty_submodule);
+		       oid, oid_valid, ce->name, dirty_submodule);
 }
 
 static int get_stat_data(const struct cache_entry *ce,
diff --git a/diff.c b/diff.c
index f3546536b..3fa335f44 100644
--- a/diff.c
+++ b/diff.c
@@ -5081,8 +5081,8 @@ static int is_submodule_ignored(const char *path, struct diff_options *options)
 
 void diff_addremove(struct diff_options *options,
 		    int addremove, unsigned mode,
-		    const unsigned char *sha1,
-		    int sha1_valid,
+		    const struct object_id *oid,
+		    int oid_valid,
 		    const char *concatpath, unsigned dirty_submodule)
 {
 	struct diff_filespec *one, *two;
@@ -5114,9 +5114,9 @@ void diff_addremove(struct diff_options *options,
 	two = alloc_filespec(concatpath);
 
 	if (addremove != '+')
-		fill_filespec(one, sha1, sha1_valid, mode);
+		fill_filespec(one, oid->hash, oid_valid, mode);
 	if (addremove != '-') {
-		fill_filespec(two, sha1, sha1_valid, mode);
+		fill_filespec(two, oid->hash, oid_valid, mode);
 		two->dirty_submodule = dirty_submodule;
 	}
 
diff --git a/diff.h b/diff.h
index d75e6d15e..1086975a5 100644
--- a/diff.h
+++ b/diff.h
@@ -31,8 +31,8 @@ typedef void (*change_fn_t)(struct diff_options *options,
 
 typedef void (*add_remove_fn_t)(struct diff_options *options,
 		    int addremove, unsigned mode,
-		    const unsigned char *sha1,
-		    int sha1_valid,
+		    const struct object_id *oid,
+		    int oid_valid,
 		    const char *fullpath, unsigned dirty_submodule);
 
 typedef void (*diff_format_fn_t)(struct diff_queue_struct *q,
@@ -247,8 +247,8 @@ extern int diff_can_quit_early(struct diff_options *);
 extern void diff_addremove(struct diff_options *,
 			   int addremove,
 			   unsigned mode,
-			   const unsigned char *sha1,
-			   int sha1_valid,
+			   const struct object_id *oid,
+			   int oid_valid,
 			   const char *fullpath, unsigned dirty_submodule);
 
 extern void diff_change(struct diff_options *,
diff --git a/revision.c b/revision.c
index 475d5b2dc..71519193c 100644
--- a/revision.c
+++ b/revision.c
@@ -401,8 +401,8 @@ static int tree_difference = REV_TREE_SAME;
 
 static void file_add_remove(struct diff_options *options,
 		    int addremove, unsigned mode,
-		    const unsigned char *sha1,
-		    int sha1_valid,
+		    const struct object_id *oid,
+		    int oid_valid,
 		    const char *fullpath, unsigned dirty_submodule)
 {
 	int diff = addremove == '+' ? REV_TREE_NEW : REV_TREE_OLD;
diff --git a/tree-diff.c b/tree-diff.c
index e164e532b..f2c747ea5 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -78,21 +78,21 @@ static int emit_diff_first_parent_only(struct diff_options *opt, struct combine_
 			1, 1, p->path, 0, 0);
 	}
 	else {
-		const unsigned char *sha1;
+		const struct object_id *oid;
 		unsigned int mode;
 		int addremove;
 
 		if (p->mode) {
 			addremove = '+';
-			sha1 = p->oid.hash;
+			oid = &p->oid;
 			mode = p->mode;
 		} else {
 			addremove = '-';
-			sha1 = p0->oid.hash;
+			oid = &p0->oid;
 			mode = p0->mode;
 		}
 
-		opt->add_remove(opt, addremove, mode, sha1, 1, p->path, 0);
+		opt->add_remove(opt, addremove, mode, oid, 1, p->path, 0);
 	}
 
 	return 0;	/* we are done with p */
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 12/33] diff: convert run_diff_files to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 diff-lib.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index 7984ff962..c82b07dc1 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -101,7 +101,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 		struct cache_entry *ce = active_cache[i];
 		int changed;
 		unsigned dirty_submodule = 0;
-		const unsigned char *old_sha1, *new_sha1;
+		const struct object_id *old_oid, *new_oid;
 
 		if (diff_can_quit_early(&revs->diffopt))
 			break;
@@ -233,12 +233,12 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 				continue;
 		}
 		oldmode = ce->ce_mode;
-		old_sha1 = ce->oid.hash;
-		new_sha1 = changed ? null_sha1 : ce->oid.hash;
+		old_oid = &ce->oid;
+		new_oid = changed ? &null_oid : &ce->oid;
 		diff_change(&revs->diffopt, oldmode, newmode,
-			    old_sha1, new_sha1,
-			    !is_null_sha1(old_sha1),
-			    !is_null_sha1(new_sha1),
+			    old_oid->hash, new_oid->hash,
+			    !is_null_oid(old_oid),
+			    !is_null_oid(new_oid),
 			    ce->name, 0, dirty_submodule);
 
 	}
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 13/33] diff: convert diff_change to struct object_id

[Brandon Williams]

Convert diff_change to take a struct object_id.  In addition convert the
function pointer type 'change_fn_t' to also take a struct object_id.

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 diff-lib.c  |  4 ++--
 diff.c      | 14 +++++++-------
 diff.h      | 13 ++++++-------
 revision.c  |  6 +++---
 tree-diff.c |  2 +-
 5 files changed, 19 insertions(+), 20 deletions(-)

diff --git a/diff-lib.c b/diff-lib.c
index c82b07dc1..1e8215df5 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -236,7 +236,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
 		old_oid = &ce->oid;
 		new_oid = changed ? &null_oid : &ce->oid;
 		diff_change(&revs->diffopt, oldmode, newmode,
-			    old_oid->hash, new_oid->hash,
+			    old_oid, new_oid,
 			    !is_null_oid(old_oid),
 			    !is_null_oid(new_oid),
 			    ce->name, 0, dirty_submodule);
@@ -367,7 +367,7 @@ static int show_modified(struct rev_info *revs,
 		return 0;
 
 	diff_change(&revs->diffopt, oldmode, mode,
-		    old->oid.hash, oid->hash, 1, !is_null_oid(oid),
+		    &old->oid, oid, 1, !is_null_oid(oid),
 		    old->name, 0, dirty_submodule);
 	return 0;
 }
diff --git a/diff.c b/diff.c
index 3fa335f44..b9bb3f6ca 100644
--- a/diff.c
+++ b/diff.c
@@ -5127,9 +5127,9 @@ void diff_addremove(struct diff_options *options,
 
 void diff_change(struct diff_options *options,
 		 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 *concatpath,
 		 unsigned old_dirty_submodule, unsigned new_dirty_submodule)
 {
@@ -5142,8 +5142,8 @@ void diff_change(struct diff_options *options,
 
 	if (DIFF_OPT_TST(options, REVERSE_DIFF)) {
 		SWAP(old_mode, new_mode);
-		SWAP(old_sha1, new_sha1);
-		SWAP(old_sha1_valid, new_sha1_valid);
+		SWAP(old_oid, new_oid);
+		SWAP(old_oid_valid, new_oid_valid);
 		SWAP(old_dirty_submodule, new_dirty_submodule);
 	}
 
@@ -5153,8 +5153,8 @@ void diff_change(struct diff_options *options,
 
 	one = alloc_filespec(concatpath);
 	two = alloc_filespec(concatpath);
-	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);
 	one->dirty_submodule = old_dirty_submodule;
 	two->dirty_submodule = new_dirty_submodule;
 	p = diff_queue(&diff_queued_diff, one, two);
diff --git a/diff.h b/diff.h
index 1086975a5..fcf334bb6 100644
--- a/diff.h
+++ b/diff.h
@@ -23,9 +23,9 @@ typedef int (*pathchange_fn_t)(struct diff_options *options,
 
 typedef void (*change_fn_t)(struct diff_options *options,
 		 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 *fullpath,
 		 unsigned old_dirty_submodule, unsigned new_dirty_submodule);
 
@@ -253,10 +253,9 @@ extern void diff_addremove(struct diff_options *,
 
 extern void diff_change(struct diff_options *,
 			unsigned mode1, unsigned mode2,
-			const unsigned char *sha1,
-			const unsigned char *sha2,
-			int sha1_valid,
-			int sha2_valid,
+			const struct object_id *old_oid,
+			const struct object_id *new_oid,
+			int old_oid_valid, int new_oid_valid,
 			const char *fullpath,
 			unsigned dirty_submodule1, unsigned dirty_submodule2);
 
diff --git a/revision.c b/revision.c
index 71519193c..7637e7556 100644
--- a/revision.c
+++ b/revision.c
@@ -414,9 +414,9 @@ static void file_add_remove(struct diff_options *options,
 
 static void file_change(struct diff_options *options,
 		 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 *fullpath,
 		 unsigned old_dirty_submodule, unsigned new_dirty_submodule)
 {
diff --git a/tree-diff.c b/tree-diff.c
index f2c747ea5..7ae1f10b2 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -74,7 +74,7 @@ static int emit_diff_first_parent_only(struct diff_options *opt, struct combine_
 {
 	struct combine_diff_parent *p0 = &p->parent[0];
 	if (p->mode && p0->mode) {
-		opt->change(opt, p0->mode, p->mode, p0->oid.hash, p->oid.hash,
+		opt->change(opt, p0->mode, p->mode, &p0->oid, &p->oid,
 			1, 1, p->path, 0, 0);
 	}
 	else {
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 14/33] diff: convert fill_filespec to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/blame.c   |  2 +-
 builtin/diff.c    |  4 ++--
 combine-diff.c    |  4 ++--
 diff-lib.c        |  2 +-
 diff-no-index.c   |  2 +-
 diff.c            | 16 ++++++++--------
 diffcore-rename.c |  2 +-
 diffcore.h        |  2 +-
 grep.c            |  2 +-
 line-log.c        |  6 +++---
 10 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 1043e5376..5ad435380 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -163,7 +163,7 @@ int textconv_object(const char *path,
 	struct userdiff_driver *textconv;
 
 	df = alloc_filespec(path);
-	fill_filespec(df, oid->hash, oid_valid, mode);
+	fill_filespec(df, oid, oid_valid, mode);
 	textconv = get_textconv(df);
 	if (!textconv) {
 		free_filespec(df);
diff --git a/builtin/diff.c b/builtin/diff.c
index 8c03ddaf5..b2d7c32cd 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -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_oid->hash, old_oid_valid, old_mode);
-	fill_filespec(two, new_oid->hash, new_oid_valid, new_mode);
+	fill_filespec(one, old_oid, old_oid_valid, old_mode);
+	fill_filespec(two, new_oid, new_oid_valid, new_mode);
 
 	diff_queue(&diff_queued_diff, one, two);
 }
diff --git a/combine-diff.c b/combine-diff.c
index 2848034fe..ad063ecb1 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -302,7 +302,7 @@ static char *grab_blob(const struct object_id *oid, unsigned int mode,
 		return xcalloc(1, 1);
 	} else if (textconv) {
 		struct diff_filespec *df = alloc_filespec(path);
-		fill_filespec(df, oid->hash, 1, mode);
+		fill_filespec(df, oid, 1, mode);
 		*size = fill_textconv(textconv, df, &blob);
 		free_filespec(df);
 	} else {
@@ -1022,7 +1022,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
 						   &result_size, NULL, NULL);
 		} else if (textconv) {
 			struct diff_filespec *df = alloc_filespec(elem->path);
-			fill_filespec(df, null_sha1, 0, st.st_mode);
+			fill_filespec(df, &null_oid, 0, st.st_mode);
 			result_size = fill_textconv(textconv, df, &result);
 			free_filespec(df);
 		} else if (0 <= (fd = open(elem->path, O_RDONLY))) {
diff --git a/diff-lib.c b/diff-lib.c
index 1e8215df5..9e076a488 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -409,7 +409,7 @@ static void do_oneway_diff(struct unpack_trees_options *o,
 		struct diff_filepair *pair;
 		pair = diff_unmerge(&revs->diffopt, idx->name);
 		if (tree)
-			fill_filespec(pair->one, tree->oid.hash, 1,
+			fill_filespec(pair->one, &tree->oid, 1,
 				      tree->ce_mode);
 		return;
 	}
diff --git a/diff-no-index.c b/diff-no-index.c
index 79229382b..80ff17d46 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -82,7 +82,7 @@ static struct diff_filespec *noindex_filespec(const char *name, int mode)
 	if (!name)
 		name = "/dev/null";
 	s = alloc_filespec(name);
-	fill_filespec(s, null_sha1, 0, mode);
+	fill_filespec(s, &null_oid, 0, mode);
 	if (name == file_from_standard_input)
 		populate_from_stdin(s);
 	return s;
diff --git a/diff.c b/diff.c
index b9bb3f6ca..e0c179f5f 100644
--- a/diff.c
+++ b/diff.c
@@ -2702,13 +2702,13 @@ void free_filespec(struct diff_filespec *spec)
 	}
 }
 
-void fill_filespec(struct diff_filespec *spec, const unsigned char *sha1,
-		   int sha1_valid, unsigned short mode)
+void fill_filespec(struct diff_filespec *spec, const struct object_id *oid,
+		   int oid_valid, unsigned short mode)
 {
 	if (mode) {
 		spec->mode = canon_mode(mode);
-		hashcpy(spec->oid.hash, sha1);
-		spec->oid_valid = sha1_valid;
+		oidcpy(&spec->oid, oid);
+		spec->oid_valid = oid_valid;
 	}
 }
 
@@ -5114,9 +5114,9 @@ void diff_addremove(struct diff_options *options,
 	two = alloc_filespec(concatpath);
 
 	if (addremove != '+')
-		fill_filespec(one, oid->hash, oid_valid, mode);
+		fill_filespec(one, oid, oid_valid, mode);
 	if (addremove != '-') {
-		fill_filespec(two, oid->hash, oid_valid, mode);
+		fill_filespec(two, oid, oid_valid, mode);
 		two->dirty_submodule = dirty_submodule;
 	}
 
@@ -5153,8 +5153,8 @@ void diff_change(struct diff_options *options,
 
 	one = alloc_filespec(concatpath);
 	two = alloc_filespec(concatpath);
-	fill_filespec(one, old_oid->hash, old_oid_valid, old_mode);
-	fill_filespec(two, new_oid->hash, new_oid_valid, new_mode);
+	fill_filespec(one, old_oid, old_oid_valid, old_mode);
+	fill_filespec(two, new_oid, new_oid_valid, new_mode);
 	one->dirty_submodule = old_dirty_submodule;
 	two->dirty_submodule = new_dirty_submodule;
 	p = diff_queue(&diff_queued_diff, one, two);
diff --git a/diffcore-rename.c b/diffcore-rename.c
index f7444c86b..3d9719dad 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -60,7 +60,7 @@ static int add_rename_dst(struct diff_filespec *two)
 		memmove(rename_dst + first + 1, rename_dst + first,
 			(rename_dst_nr - first - 1) * sizeof(*rename_dst));
 	rename_dst[first].two = alloc_filespec(two->path);
-	fill_filespec(rename_dst[first].two, two->oid.hash, two->oid_valid,
+	fill_filespec(rename_dst[first].two, &two->oid, two->oid_valid,
 		      two->mode);
 	rename_dst[first].pair = NULL;
 	return 0;
diff --git a/diffcore.h b/diffcore.h
index 623024135..a30da161d 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -52,7 +52,7 @@ struct diff_filespec {
 
 extern struct diff_filespec *alloc_filespec(const char *);
 extern void free_filespec(struct diff_filespec *);
-extern void fill_filespec(struct diff_filespec *, const unsigned char *,
+extern void fill_filespec(struct diff_filespec *, const struct object_id *,
 			  int, unsigned short);
 
 #define CHECK_SIZE_ONLY 1
diff --git a/grep.c b/grep.c
index a240b4cdb..3e21c92b9 100644
--- a/grep.c
+++ b/grep.c
@@ -1407,7 +1407,7 @@ static int fill_textconv_grep(struct userdiff_driver *driver,
 		fill_filespec(df, gs->identifier, 1, 0100644);
 		break;
 	case GREP_SOURCE_FILE:
-		fill_filespec(df, null_sha1, 0, 0100644);
+		fill_filespec(df, &null_oid, 0, 0100644);
 		break;
 	default:
 		die("BUG: attempt to textconv something without a path?");
diff --git a/line-log.c b/line-log.c
index b9087814b..a3bd2f2d5 100644
--- a/line-log.c
+++ b/line-log.c
@@ -500,12 +500,12 @@ static struct commit *check_single_commit(struct rev_info *revs)
 static void fill_blob_sha1(struct commit *commit, struct diff_filespec *spec)
 {
 	unsigned mode;
-	unsigned char sha1[20];
+	struct object_id oid;
 
 	if (get_tree_entry(commit->object.oid.hash, spec->path,
-			   sha1, &mode))
+			   oid.hash, &mode))
 		die("There is no path %s in the commit", spec->path);
-	fill_filespec(spec, sha1, 1, mode);
+	fill_filespec(spec, &oid, 1, mode);
 
 	return;
 }
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 15/33] diff: convert reuse_worktree_file to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 diff.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/diff.c b/diff.c
index e0c179f5f..084c8b2d0 100644
--- a/diff.c
+++ b/diff.c
@@ -2717,7 +2717,7 @@ void fill_filespec(struct diff_filespec *spec, const struct object_id *oid,
  * the work tree has that object contents, return true, so that
  * prepare_temp_file() does not have to inflate and extract.
  */
-static int reuse_worktree_file(const char *name, const unsigned char *sha1, int want_file)
+static int reuse_worktree_file(const char *name, const struct object_id *oid, int want_file)
 {
 	const struct cache_entry *ce;
 	struct stat st;
@@ -2748,7 +2748,7 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int
 	 * objects however would tend to be slower as they need
 	 * to be individually opened and inflated.
 	 */
-	if (!FAST_WORKING_DIRECTORY && !want_file && has_sha1_pack(sha1))
+	if (!FAST_WORKING_DIRECTORY && !want_file && has_sha1_pack(oid->hash))
 		return 0;
 
 	/*
@@ -2768,7 +2768,7 @@ static int reuse_worktree_file(const char *name, const unsigned char *sha1, int
 	 * This is not the sha1 we are looking for, or
 	 * unreusable because it is not a regular file.
 	 */
-	if (hashcmp(sha1, ce->oid.hash) || !S_ISREG(ce->ce_mode))
+	if (oidcmp(oid, &ce->oid) || !S_ISREG(ce->ce_mode))
 		return 0;
 
 	/*
@@ -2842,7 +2842,7 @@ int diff_populate_filespec(struct diff_filespec *s, unsigned int flags)
 		return diff_populate_gitlink(s, size_only);
 
 	if (!s->oid_valid ||
-	    reuse_worktree_file(s->path, s->oid.hash, 0)) {
+	    reuse_worktree_file(s->path, &s->oid, 0)) {
 		struct strbuf buf = STRBUF_INIT;
 		struct stat st;
 		int fd;
@@ -3008,7 +3008,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
 
 	if (!S_ISGITLINK(one->mode) &&
 	    (!one->oid_valid ||
-	     reuse_worktree_file(name, one->oid.hash, 1))) {
+	     reuse_worktree_file(name, &one->oid, 1))) {
 		struct stat st;
 		if (lstat(name, &st) < 0) {
 			if (errno == ENOENT)
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 16/33] diff: finish conversion for prepare_temp_file to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 diff.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/diff.c b/diff.c
index 084c8b2d0..a8ceeb024 100644
--- a/diff.c
+++ b/diff.c
@@ -3030,13 +3030,13 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
 			/* we can borrow from the file in the work tree */
 			temp->name = name;
 			if (!one->oid_valid)
-				sha1_to_hex_r(temp->hex, null_sha1);
+				oid_to_hex_r(temp->hex, &null_oid);
 			else
 				oid_to_hex_r(temp->hex, &one->oid);
 			/* Even though we may sometimes borrow the
 			 * contents from the work tree, we always want
 			 * one->mode.  mode is trustworthy even when
-			 * !(one->sha1_valid), as long as
+			 * !(one->oid_valid), as long as
 			 * DIFF_FILE_VALID(one).
 			 */
 			xsnprintf(temp->mode, sizeof(temp->mode), "%06o", one->mode);
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 17/33] patch-ids: convert to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/log.c |  2 +-
 patch-ids.c   | 20 ++++++++++----------
 patch-ids.h   |  4 ++--
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/builtin/log.c b/builtin/log.c
index a440601ef..6bdba3444 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1354,7 +1354,7 @@ static void prepare_bases(struct base_tree_info *bases,
 		struct object_id *patch_id;
 		if (commit->util)
 			continue;
-		if (commit_patch_id(commit, &diffopt, oid.hash, 0))
+		if (commit_patch_id(commit, &diffopt, &oid, 0))
 			die(_("cannot get patch id"));
 		ALLOC_GROW(bases->patch_id, bases->nr_patch_id + 1, bases->alloc_patch_id);
 		patch_id = bases->patch_id + bases->nr_patch_id;
diff --git a/patch-ids.c b/patch-ids.c
index 92eba7a05..aab26cbbb 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -11,7 +11,7 @@ static int patch_id_defined(struct commit *commit)
 }
 
 int commit_patch_id(struct commit *commit, struct diff_options *options,
-		    unsigned char *sha1, int diff_header_only)
+		    struct object_id *oid, int diff_header_only)
 {
 	if (!patch_id_defined(commit))
 		return -1;
@@ -22,7 +22,7 @@ int commit_patch_id(struct commit *commit, struct diff_options *options,
 	else
 		diff_root_tree_sha1(commit->object.oid.hash, "", options);
 	diffcore_std(options);
-	return diff_flush_patch_id(options, sha1, diff_header_only);
+	return diff_flush_patch_id(options, oid->hash, diff_header_only);
 }
 
 /*
@@ -39,15 +39,15 @@ static int patch_id_cmp(struct patch_id *a,
 			struct patch_id *b,
 			struct diff_options *opt)
 {
-	if (is_null_sha1(a->patch_id) &&
-	    commit_patch_id(a->commit, opt, a->patch_id, 0))
+	if (is_null_oid(&a->patch_id) &&
+	    commit_patch_id(a->commit, opt, &a->patch_id, 0))
 		return error("Could not get patch ID for %s",
 			oid_to_hex(&a->commit->object.oid));
-	if (is_null_sha1(b->patch_id) &&
-	    commit_patch_id(b->commit, opt, b->patch_id, 0))
+	if (is_null_oid(&b->patch_id) &&
+	    commit_patch_id(b->commit, opt, &b->patch_id, 0))
 		return error("Could not get patch ID for %s",
 			oid_to_hex(&b->commit->object.oid));
-	return hashcmp(a->patch_id, b->patch_id);
+	return oidcmp(&a->patch_id, &b->patch_id);
 }
 
 int init_patch_ids(struct patch_ids *ids)
@@ -71,13 +71,13 @@ static int init_patch_id_entry(struct patch_id *patch,
 			       struct commit *commit,
 			       struct patch_ids *ids)
 {
-	unsigned char header_only_patch_id[GIT_MAX_RAWSZ];
+	struct object_id header_only_patch_id;
 
 	patch->commit = commit;
-	if (commit_patch_id(commit, &ids->diffopts, header_only_patch_id, 1))
+	if (commit_patch_id(commit, &ids->diffopts, &header_only_patch_id, 1))
 		return -1;
 
-	hashmap_entry_init(patch, sha1hash(header_only_patch_id));
+	hashmap_entry_init(patch, sha1hash(header_only_patch_id.hash));
 	return 0;
 }
 
diff --git a/patch-ids.h b/patch-ids.h
index b9e5751f8..bec0f727a 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_MAX_RAWSZ];
+	struct object_id patch_id;
 	struct commit *commit;
 };
 
@@ -13,7 +13,7 @@ struct patch_ids {
 };
 
 int commit_patch_id(struct commit *commit, struct diff_options *options,
-		    unsigned char *sha1, int);
+		    struct object_id *oid, int);
 int init_patch_ids(struct patch_ids *);
 int free_patch_ids(struct patch_ids *);
 struct patch_id *add_commit_patch_id(struct commit *, struct patch_ids *);
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 18/33] diff: convert diff_flush_patch_id to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 diff.c      | 12 ++++++------
 diff.h      |  2 +-
 patch-ids.c |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/diff.c b/diff.c
index a8ceeb024..dd325e616 100644
--- a/diff.c
+++ b/diff.c
@@ -4584,7 +4584,7 @@ static void patch_id_add_mode(git_SHA_CTX *ctx, unsigned mode)
 }
 
 /* returns 0 upon success, and writes result into sha1 */
-static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1, int diff_header_only)
+static int diff_get_patch_id(struct diff_options *options, struct object_id *oid, int diff_header_only)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
 	int i;
@@ -4656,9 +4656,9 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1,
 		if (diff_filespec_is_binary(p->one) ||
 		    diff_filespec_is_binary(p->two)) {
 			git_SHA1_Update(&ctx, oid_to_hex(&p->one->oid),
-					40);
+					GIT_SHA1_HEXSZ);
 			git_SHA1_Update(&ctx, oid_to_hex(&p->two->oid),
-					40);
+					GIT_SHA1_HEXSZ);
 			continue;
 		}
 
@@ -4671,15 +4671,15 @@ static int diff_get_patch_id(struct diff_options *options, unsigned char *sha1,
 				     p->one->path);
 	}
 
-	git_SHA1_Final(sha1, &ctx);
+	git_SHA1_Final(oid->hash, &ctx);
 	return 0;
 }
 
-int diff_flush_patch_id(struct diff_options *options, unsigned char *sha1, int diff_header_only)
+int diff_flush_patch_id(struct diff_options *options, struct object_id *oid, int diff_header_only)
 {
 	struct diff_queue_struct *q = &diff_queued_diff;
 	int i;
-	int result = diff_get_patch_id(options, sha1, diff_header_only);
+	int result = diff_get_patch_id(options, oid, diff_header_only);
 
 	for (i = 0; i < q->nr; i++)
 		diff_free_filepair(q->queue[i]);
diff --git a/diff.h b/diff.h
index fcf334bb6..150f1c5a1 100644
--- a/diff.h
+++ b/diff.h
@@ -354,7 +354,7 @@ extern int run_diff_files(struct rev_info *revs, unsigned int option);
 extern int run_diff_index(struct rev_info *revs, int cached);
 
 extern int do_diff_cache(const struct object_id *, struct diff_options *);
-extern int diff_flush_patch_id(struct diff_options *, unsigned char *, int);
+extern int diff_flush_patch_id(struct diff_options *, struct object_id *, int);
 
 extern int diff_result_code(struct diff_options *, int);
 
diff --git a/patch-ids.c b/patch-ids.c
index aab26cbbb..a70a291d8 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -22,7 +22,7 @@ int commit_patch_id(struct commit *commit, struct diff_options *options,
 	else
 		diff_root_tree_sha1(commit->object.oid.hash, "", options);
 	diffcore_std(options);
-	return diff_flush_patch_id(options, oid->hash, diff_header_only);
+	return diff_flush_patch_id(options, oid, diff_header_only);
 }
 
 /*
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 19/33] combine-diff: convert diff_tree_combined to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/diff.c |  2 +-
 combine-diff.c | 10 +++++-----
 diff.h         |  2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/builtin/diff.c b/builtin/diff.c
index b2d7c32cd..73b4ff3db 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -194,7 +194,7 @@ static int builtin_diff_combined(struct rev_info *revs,
 		revs->dense_combined_merges = revs->combine_merges = 1;
 	for (i = 1; i < ents; i++)
 		oid_array_append(&parents, &ent[i].item->oid);
-	diff_tree_combined(ent[0].item->oid.hash, &parents,
+	diff_tree_combined(&ent[0].item->oid, &parents,
 			   revs->dense_combined_merges, revs);
 	oid_array_clear(&parents);
 	return 0;
diff --git a/combine-diff.c b/combine-diff.c
index ad063ecb1..84981df75 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1384,7 +1384,7 @@ static struct combine_diff_path *find_paths_multitree(
 }
 
 
-void diff_tree_combined(const unsigned char *sha1,
+void diff_tree_combined(const struct object_id *oid,
 			const struct oid_array *parents,
 			int dense,
 			struct rev_info *rev)
@@ -1448,11 +1448,11 @@ void diff_tree_combined(const unsigned char *sha1,
 		 * diff(sha1,parent_i) for all i to do the job, specifically
 		 * for parent0.
 		 */
-		paths = find_paths_generic(sha1, parents, &diffopts);
+		paths = find_paths_generic(oid->hash, parents, &diffopts);
 	}
 	else {
 		int stat_opt;
-		paths = find_paths_multitree(sha1, parents, &diffopts);
+		paths = find_paths_multitree(oid->hash, parents, &diffopts);
 
 		/*
 		 * show stat against the first parent even
@@ -1463,7 +1463,7 @@ void diff_tree_combined(const unsigned char *sha1,
 		if (stat_opt) {
 			diffopts.output_format = stat_opt;
 
-			diff_tree_sha1(parents->oid[0].hash, sha1, "", &diffopts);
+			diff_tree_sha1(parents->oid[0].hash, oid->hash, "", &diffopts);
 			diffcore_std(&diffopts);
 			if (opt->orderfile)
 				diffcore_order(opt->orderfile);
@@ -1539,6 +1539,6 @@ void diff_tree_combined_merge(const struct commit *commit, int dense,
 		oid_array_append(&parents, &parent->item->object.oid);
 		parent = parent->next;
 	}
-	diff_tree_combined(commit->object.oid.hash, &parents, dense, rev);
+	diff_tree_combined(&commit->object.oid, &parents, dense, rev);
 	oid_array_clear(&parents);
 }
diff --git a/diff.h b/diff.h
index 150f1c5a1..6aeeda035 100644
--- a/diff.h
+++ b/diff.h
@@ -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 oid_array *parents, int dense, struct rev_info *rev);
+extern void diff_tree_combined(const struct object_id *oid, 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);
 
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 20/33] combine-diff: convert find_paths_* to struct object_id

[Brandon Williams]

Convert find_paths_generic and find_paths_multitree to use struct
object_id.


Signed-off-by: Brandon Williams <bmwill@google.com>
---
 combine-diff.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index 84981df75..c82364510 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,
+static struct combine_diff_path *find_paths_generic(const struct object_id *oid,
 	const struct oid_array *parents, struct diff_options *opt)
 {
 	struct combine_diff_path *paths = NULL;
@@ -1336,7 +1336,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->oid[i].hash, sha1, "", opt);
+		diff_tree_sha1(parents->oid[i].hash, oid->hash, "", opt);
 		diffcore_std(opt);
 		paths = intersect_paths(paths, i, num_parent);
 
@@ -1360,7 +1360,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 oid_array *parents,
+	const struct object_id *oid, const struct oid_array *parents,
 	struct diff_options *opt)
 {
 	int i, nparent = parents->nr;
@@ -1376,7 +1376,7 @@ static struct combine_diff_path *find_paths_multitree(
 	paths_head.next = NULL;
 
 	strbuf_init(&base, PATH_MAX);
-	diff_tree_paths(&paths_head, sha1, parents_sha1, nparent, &base, opt);
+	diff_tree_paths(&paths_head, oid->hash, parents_sha1, nparent, &base, opt);
 
 	strbuf_release(&base);
 	free(parents_sha1);
@@ -1448,11 +1448,11 @@ void diff_tree_combined(const struct object_id *oid,
 		 * diff(sha1,parent_i) for all i to do the job, specifically
 		 * for parent0.
 		 */
-		paths = find_paths_generic(oid->hash, parents, &diffopts);
+		paths = find_paths_generic(oid, parents, &diffopts);
 	}
 	else {
 		int stat_opt;
-		paths = find_paths_multitree(oid->hash, parents, &diffopts);
+		paths = find_paths_multitree(oid, parents, &diffopts);
 
 		/*
 		 * show stat against the first parent even
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 21/33] tree-diff: convert diff_root_tree_sha1 to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/fast-export.c | 4 ++--
 diff.h                | 4 ++--
 log-tree.c            | 2 +-
 patch-ids.c           | 2 +-
 tree-diff.c           | 4 ++--
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 24e29ad7e..e242726f0 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -566,8 +566,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
 			       commit->tree->object.oid.hash, "", &rev->diffopt);
 	}
 	else
-		diff_root_tree_sha1(commit->tree->object.oid.hash,
-				    "", &rev->diffopt);
+		diff_root_tree_oid(&commit->tree->object.oid,
+				   "", &rev->diffopt);
 
 	/* Export the referenced blobs, and remember the marks. */
 	for (i = 0; i < diff_queued_diff.nr; i++)
diff --git a/diff.h b/diff.h
index 6aeeda035..8d46a6709 100644
--- a/diff.h
+++ b/diff.h
@@ -215,8 +215,8 @@ extern struct combine_diff_path *diff_tree_paths(
 	struct strbuf *base, struct diff_options *opt);
 extern int diff_tree_sha1(const unsigned char *old, const unsigned char *new,
 			  const char *base, struct diff_options *opt);
-extern int diff_root_tree_sha1(const unsigned char *new, const char *base,
-                               struct diff_options *opt);
+extern int diff_root_tree_oid(const struct object_id *new_oid, const char *base,
+			      struct diff_options *opt);
 
 struct combine_diff_path {
 	struct combine_diff_path *next;
diff --git a/log-tree.c b/log-tree.c
index 9c0c64a2d..b40242534 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -803,7 +803,7 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 	parents = get_saved_parents(opt, commit);
 	if (!parents) {
 		if (opt->show_root_diff) {
-			diff_root_tree_sha1(oid->hash, "", &opt->diffopt);
+			diff_root_tree_oid(oid, "", &opt->diffopt);
 			log_tree_diff_flush(opt);
 		}
 		return !opt->loginfo;
diff --git a/patch-ids.c b/patch-ids.c
index a70a291d8..aaf462c03 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -20,7 +20,7 @@ int commit_patch_id(struct commit *commit, struct diff_options *options,
 		diff_tree_sha1(commit->parents->item->object.oid.hash,
 			       commit->object.oid.hash, "", options);
 	else
-		diff_root_tree_sha1(commit->object.oid.hash, "", options);
+		diff_root_tree_oid(&commit->object.oid, "", options);
 	diffcore_std(options);
 	return diff_flush_patch_id(options, oid, diff_header_only);
 }
diff --git a/tree-diff.c b/tree-diff.c
index 7ae1f10b2..f9bbaf3c4 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -711,7 +711,7 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha
 	return retval;
 }
 
-int diff_root_tree_sha1(const unsigned char *new, const char *base, struct diff_options *opt)
+int diff_root_tree_oid(const struct object_id *new_oid, const char *base, struct diff_options *opt)
 {
-	return diff_tree_sha1(NULL, new, base, opt);
+	return diff_tree_sha1(NULL, new_oid->hash, base, opt);
 }
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 22/33] notes-merge: convert notes_merge* to struct object_id

[Brandon Williams]

Convert notes_merge and notes_merge_commit to use struct object_id.


Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/notes.c |  6 +++---
 notes-merge.c   | 58 ++++++++++++++++++++++++++++-----------------------------
 notes-merge.h   | 12 ++++++------
 3 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index b13fc8789..2ebc2b7c4 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -724,7 +724,7 @@ static int merge_commit(struct notes_merge_options *o)
 	if (!o->local_ref)
 		die(_("failed to resolve NOTES_MERGE_REF"));
 
-	if (notes_merge_commit(o, t, partial, oid.hash))
+	if (notes_merge_commit(o, t, partial, &oid))
 		die(_("failed to finalize notes merge"));
 
 	/* Reuse existing commit message in reflog message */
@@ -842,9 +842,9 @@ static int merge(int argc, const char **argv, const char *prefix)
 		    remote_ref.buf, default_notes_ref());
 	strbuf_add(&(o.commit_msg), msg.buf + 7, msg.len - 7); /* skip "notes: " */
 
-	result = notes_merge(&o, t, result_oid.hash);
+	result = notes_merge(&o, t, &result_oid);
 
-	if (result >= 0) /* Merge resulted (trivially) in result_sha1 */
+	if (result >= 0) /* Merge resulted (trivially) in result_oid */
 		/* Update default notes ref with new commit */
 		update_ref(msg.buf, default_notes_ref(), result_oid.hash, NULL,
 			   0, UPDATE_REFS_DIE_ON_ERR);
diff --git a/notes-merge.c b/notes-merge.c
index 9a1a49506..9dbf7f6a3 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -533,17 +533,17 @@ static int merge_from_diffs(struct notes_merge_options *o,
 
 int notes_merge(struct notes_merge_options *o,
 		struct notes_tree *local_tree,
-		unsigned char *result_sha1)
+		struct object_id *result_oid)
 {
 	struct object_id local_oid, remote_oid;
 	struct commit *local, *remote;
 	struct commit_list *bases = NULL;
-	const unsigned char *base_sha1, *base_tree_sha1;
+	const struct object_id *base_oid, *base_tree_oid;
 	int result = 0;
 
 	assert(o->local_ref && o->remote_ref);
 	assert(!strcmp(o->local_ref, local_tree->ref));
-	hashclr(result_sha1);
+	oidclr(result_oid);
 
 	trace_printf("notes_merge(o->local_ref = %s, o->remote_ref = %s)\n",
 	       o->local_ref, o->remote_ref);
@@ -553,16 +553,16 @@ int notes_merge(struct notes_merge_options *o,
 		die("Failed to resolve local notes ref '%s'", o->local_ref);
 	else if (!check_refname_format(o->local_ref, 0) &&
 		is_null_oid(&local_oid))
-		local = NULL; /* local_sha1 == null_sha1 indicates unborn ref */
+		local = NULL; /* local_oid == null_oid indicates unborn ref */
 	else if (!(local = lookup_commit_reference(&local_oid)))
 		die("Could not parse local commit %s (%s)",
 		    oid_to_hex(&local_oid), o->local_ref);
 	trace_printf("\tlocal commit: %.7s\n", oid_to_hex(&local_oid));
 
-	/* Dereference o->remote_ref into remote_sha1 */
+	/* Dereference o->remote_ref into remote_oid */
 	if (get_oid(o->remote_ref, &remote_oid)) {
 		/*
-		 * Failed to get remote_sha1. If o->remote_ref looks like an
+		 * Failed to get remote_oid. If o->remote_ref looks like an
 		 * unborn ref, perform the merge using an empty notes tree.
 		 */
 		if (!check_refname_format(o->remote_ref, 0)) {
@@ -583,12 +583,12 @@ int notes_merge(struct notes_merge_options *o,
 		    "(%s)", o->remote_ref, o->local_ref);
 	if (!local) {
 		/* result == remote commit */
-		hashcpy(result_sha1, remote_oid.hash);
+		oidcpy(result_oid, &remote_oid);
 		goto found_result;
 	}
 	if (!remote) {
 		/* result == local commit */
-		hashcpy(result_sha1, local_oid.hash);
+		oidcpy(result_oid, &local_oid);
 		goto found_result;
 	}
 	assert(local && remote);
@@ -596,47 +596,47 @@ int notes_merge(struct notes_merge_options *o,
 	/* Find merge bases */
 	bases = get_merge_bases(local, remote);
 	if (!bases) {
-		base_sha1 = null_sha1;
-		base_tree_sha1 = EMPTY_TREE_SHA1_BIN;
+		base_oid = &null_oid;
+		base_tree_oid = &empty_tree_oid;
 		if (o->verbosity >= 4)
 			printf("No merge base found; doing history-less merge\n");
 	} else if (!bases->next) {
-		base_sha1 = bases->item->object.oid.hash;
-		base_tree_sha1 = bases->item->tree->object.oid.hash;
+		base_oid = &bases->item->object.oid;
+		base_tree_oid = &bases->item->tree->object.oid;
 		if (o->verbosity >= 4)
 			printf("One merge base found (%.7s)\n",
-				sha1_to_hex(base_sha1));
+			       oid_to_hex(base_oid));
 	} else {
 		/* TODO: How to handle multiple merge-bases? */
-		base_sha1 = bases->item->object.oid.hash;
-		base_tree_sha1 = bases->item->tree->object.oid.hash;
+		base_oid = &bases->item->object.oid;
+		base_tree_oid = &bases->item->tree->object.oid;
 		if (o->verbosity >= 3)
 			printf("Multiple merge bases found. Using the first "
-				"(%.7s)\n", sha1_to_hex(base_sha1));
+				"(%.7s)\n", oid_to_hex(base_oid));
 	}
 
 	if (o->verbosity >= 4)
 		printf("Merging remote commit %.7s into local commit %.7s with "
 			"merge-base %.7s\n", oid_to_hex(&remote->object.oid),
 			oid_to_hex(&local->object.oid),
-			sha1_to_hex(base_sha1));
+			oid_to_hex(base_oid));
 
-	if (!hashcmp(remote->object.oid.hash, base_sha1)) {
+	if (!oidcmp(&remote->object.oid, base_oid)) {
 		/* Already merged; result == local commit */
 		if (o->verbosity >= 2)
 			printf("Already up-to-date!\n");
-		hashcpy(result_sha1, local->object.oid.hash);
+		oidcpy(result_oid, &local->object.oid);
 		goto found_result;
 	}
-	if (!hashcmp(local->object.oid.hash, base_sha1)) {
+	if (!oidcmp(&local->object.oid, base_oid)) {
 		/* Fast-forward; result == remote commit */
 		if (o->verbosity >= 2)
 			printf("Fast-forward\n");
-		hashcpy(result_sha1, remote->object.oid.hash);
+		oidcpy(result_oid, &remote->object.oid);
 		goto found_result;
 	}
 
-	result = merge_from_diffs(o, base_tree_sha1, local->tree->object.oid.hash,
+	result = merge_from_diffs(o, base_tree_oid->hash, local->tree->object.oid.hash,
 				  remote->tree->object.oid.hash, local_tree);
 
 	if (result != 0) { /* non-trivial merge (with or without conflicts) */
@@ -646,28 +646,28 @@ int notes_merge(struct notes_merge_options *o,
 		commit_list_insert(local, &parents);
 		create_notes_commit(local_tree, parents,
 				    o->commit_msg.buf, o->commit_msg.len,
-				    result_sha1);
+				    result_oid->hash);
 	}
 
 found_result:
 	free_commit_list(bases);
 	strbuf_release(&(o->commit_msg));
-	trace_printf("notes_merge(): result = %i, result_sha1 = %.7s\n",
-	       result, sha1_to_hex(result_sha1));
+	trace_printf("notes_merge(): result = %i, result_oid = %.7s\n",
+	       result, oid_to_hex(result_oid));
 	return result;
 }
 
 int notes_merge_commit(struct notes_merge_options *o,
 		       struct notes_tree *partial_tree,
 		       struct commit *partial_commit,
-		       unsigned char *result_sha1)
+		       struct object_id *result_oid)
 {
 	/*
 	 * Iterate through files in .git/NOTES_MERGE_WORKTREE and add all
 	 * found notes to 'partial_tree'. Write the updated notes tree to
 	 * the DB, and commit the resulting tree object while reusing the
 	 * commit message and parents from 'partial_commit'.
-	 * Finally store the new commit object SHA1 into 'result_sha1'.
+	 * Finally store the new commit object OID into 'result_oid'.
 	 */
 	DIR *dir;
 	struct dirent *e;
@@ -721,11 +721,11 @@ int notes_merge_commit(struct notes_merge_options *o,
 	}
 
 	create_notes_commit(partial_tree, partial_commit->parents,
-			    msg, strlen(msg), result_sha1);
+			    msg, strlen(msg), result_oid->hash);
 	unuse_commit_buffer(partial_commit, buffer);
 	if (o->verbosity >= 4)
 		printf("Finalized notes merge commit: %s\n",
-			sha1_to_hex(result_sha1));
+			oid_to_hex(result_oid));
 	strbuf_release(&path);
 	closedir(dir);
 	return 0;
diff --git a/notes-merge.h b/notes-merge.h
index 0d890563b..eaa8e3b86 100644
--- a/notes-merge.h
+++ b/notes-merge.h
@@ -33,15 +33,15 @@ void init_notes_merge_options(struct notes_merge_options *o);
  *
  * 1. The merge trivially results in an existing commit (e.g. fast-forward or
  *    already-up-to-date). 'local_tree' is untouched, the SHA1 of the result
- *    is written into 'result_sha1' and 0 is returned.
+ *    is written into 'result_oid' and 0 is returned.
  * 2. The merge successfully completes, producing a merge commit. local_tree
  *    contains the updated notes tree, the SHA1 of the resulting commit is
- *    written into 'result_sha1', and 1 is returned.
+ *    written into 'result_oid', and 1 is returned.
  * 3. The merge results in conflicts. This is similar to #2 in that the
  *    partial merge result (i.e. merge result minus the unmerged entries)
  *    are stored in 'local_tree', and the SHA1 or the resulting commit
  *    (to be amended when the conflicts have been resolved) is written into
- *    'result_sha1'. The unmerged entries are written into the
+ *    'result_oid'. The unmerged entries are written into the
  *    .git/NOTES_MERGE_WORKTREE directory with conflict markers.
  *    -1 is returned.
  *
@@ -52,7 +52,7 @@ void init_notes_merge_options(struct notes_merge_options *o);
  */
 int notes_merge(struct notes_merge_options *o,
 		struct notes_tree *local_tree,
-		unsigned char *result_sha1);
+		struct object_id *result_oid);
 
 /*
  * Finalize conflict resolution from an earlier notes_merge()
@@ -63,12 +63,12 @@ int notes_merge(struct notes_merge_options *o,
  *
  * This function will add the (now resolved) notes in .git/NOTES_MERGE_WORKTREE
  * to 'partial_tree', and create a final notes merge commit, the SHA1 of which
- * will be stored in 'result_sha1'.
+ * will be stored in 'result_oid'.
  */
 int notes_merge_commit(struct notes_merge_options *o,
 		       struct notes_tree *partial_tree,
 		       struct commit *partial_commit,
-		       unsigned char *result_sha1);
+		       struct object_id *result_oid);
 
 /*
  * Abort conflict resolution from an earlier notes_merge()
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 23/33] notes-merge: convert merge_from_diffs to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 notes-merge.c | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/notes-merge.c b/notes-merge.c
index 9dbf7f6a3..be78f1954 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -114,8 +114,8 @@ static struct object_id uninitialized = {
 };
 
 static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
-						 const unsigned char *base,
-						 const unsigned char *remote,
+						 const struct object_id *base,
+						 const struct object_id *remote,
 						 int *num_changes)
 {
 	struct diff_options opt;
@@ -123,13 +123,13 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
 	int i, len = 0;
 
 	trace_printf("\tdiff_tree_remote(base = %.7s, remote = %.7s)\n",
-	       sha1_to_hex(base), sha1_to_hex(remote));
+	       oid_to_hex(base), oid_to_hex(remote));
 
 	diff_setup(&opt);
 	DIFF_OPT_SET(&opt, RECURSIVE);
 	opt.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_setup_done(&opt);
-	diff_tree_sha1(base, remote, "", &opt);
+	diff_tree_sha1(base->hash, remote->hash, "", &opt);
 	diffcore_std(&opt);
 
 	changes = xcalloc(diff_queued_diff.nr, sizeof(struct notes_merge_pair));
@@ -179,20 +179,20 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
 
 static void diff_tree_local(struct notes_merge_options *o,
 			    struct notes_merge_pair *changes, int len,
-			    const unsigned char *base,
-			    const unsigned char *local)
+			    const struct object_id *base,
+			    const struct object_id *local)
 {
 	struct diff_options opt;
 	int i;
 
 	trace_printf("\tdiff_tree_local(len = %i, base = %.7s, local = %.7s)\n",
-	       len, sha1_to_hex(base), sha1_to_hex(local));
+	       len, oid_to_hex(base), oid_to_hex(local));
 
 	diff_setup(&opt);
 	DIFF_OPT_SET(&opt, RECURSIVE);
 	opt.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_setup_done(&opt);
-	diff_tree_sha1(base, local, "", &opt);
+	diff_tree_sha1(base->hash, local->hash, "", &opt);
 	diffcore_std(&opt);
 
 	for (i = 0; i < diff_queued_diff.nr; i++) {
@@ -505,16 +505,17 @@ static int merge_changes(struct notes_merge_options *o,
 }
 
 static int merge_from_diffs(struct notes_merge_options *o,
-			    const unsigned char *base,
-			    const unsigned char *local,
-			    const unsigned char *remote, struct notes_tree *t)
+			    const struct object_id *base,
+			    const struct object_id *local,
+			    const struct object_id *remote,
+			    struct notes_tree *t)
 {
 	struct notes_merge_pair *changes;
 	int num_changes, conflicts;
 
 	trace_printf("\tmerge_from_diffs(base = %.7s, local = %.7s, "
-	       "remote = %.7s)\n", sha1_to_hex(base), sha1_to_hex(local),
-	       sha1_to_hex(remote));
+	       "remote = %.7s)\n", oid_to_hex(base), oid_to_hex(local),
+	       oid_to_hex(remote));
 
 	changes = diff_tree_remote(o, base, remote, &num_changes);
 	diff_tree_local(o, changes, num_changes, base, local);
@@ -636,8 +637,8 @@ int notes_merge(struct notes_merge_options *o,
 		goto found_result;
 	}
 
-	result = merge_from_diffs(o, base_tree_oid->hash, local->tree->object.oid.hash,
-				  remote->tree->object.oid.hash, local_tree);
+	result = merge_from_diffs(o, base_tree_oid, &local->tree->object.oid,
+				  &remote->tree->object.oid, local_tree);
 
 	if (result != 0) { /* non-trivial merge (with or without conflicts) */
 		/* Commit (partial) result */
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 24/33] notes-merge: convert find_notes_merge_pair_ps to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 notes-merge.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/notes-merge.c b/notes-merge.c
index be78f1954..55dbb3659 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -58,7 +58,7 @@ static int verify_notes_filepair(struct diff_filepair *p, unsigned char *sha1)
 }
 
 static struct notes_merge_pair *find_notes_merge_pair_pos(
-		struct notes_merge_pair *list, int len, unsigned char *obj,
+		struct notes_merge_pair *list, int len, struct object_id *obj,
 		int insert_new, int *occupied)
 {
 	/*
@@ -75,7 +75,7 @@ static struct notes_merge_pair *find_notes_merge_pair_pos(
 	int i = last_index < len ? last_index : len - 1;
 	int prev_cmp = 0, cmp = -1;
 	while (i >= 0 && i < len) {
-		cmp = hashcmp(obj, list[i].obj.hash);
+		cmp = oidcmp(obj, &list[i].obj);
 		if (!cmp) /* obj belongs @ i */
 			break;
 		else if (cmp < 0 && prev_cmp <= 0) /* obj belongs < i */
@@ -138,19 +138,19 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
 		struct diff_filepair *p = diff_queued_diff.queue[i];
 		struct notes_merge_pair *mp;
 		int occupied;
-		unsigned char obj[20];
+		struct object_id obj;
 
-		if (verify_notes_filepair(p, obj)) {
+		if (verify_notes_filepair(p, obj.hash)) {
 			trace_printf("\t\tCannot merge entry '%s' (%c): "
 			       "%.7s -> %.7s. Skipping!\n", p->one->path,
 			       p->status, oid_to_hex(&p->one->oid),
 			       oid_to_hex(&p->two->oid));
 			continue;
 		}
-		mp = find_notes_merge_pair_pos(changes, len, obj, 1, &occupied);
+		mp = find_notes_merge_pair_pos(changes, len, &obj, 1, &occupied);
 		if (occupied) {
 			/* We've found an addition/deletion pair */
-			assert(!hashcmp(mp->obj.hash, obj));
+			assert(!oidcmp(&mp->obj, &obj));
 			if (is_null_oid(&p->one->oid)) { /* addition */
 				assert(is_null_oid(&mp->remote));
 				oidcpy(&mp->remote, &p->two->oid);
@@ -160,7 +160,7 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
 			} else
 				assert(!"Invalid existing change recorded");
 		} else {
-			hashcpy(mp->obj.hash, obj);
+			oidcpy(&mp->obj, &obj);
 			oidcpy(&mp->base, &p->one->oid);
 			oidcpy(&mp->local, &uninitialized);
 			oidcpy(&mp->remote, &p->two->oid);
@@ -199,25 +199,25 @@ static void diff_tree_local(struct notes_merge_options *o,
 		struct diff_filepair *p = diff_queued_diff.queue[i];
 		struct notes_merge_pair *mp;
 		int match;
-		unsigned char obj[20];
+		struct object_id obj;
 
-		if (verify_notes_filepair(p, obj)) {
+		if (verify_notes_filepair(p, obj.hash)) {
 			trace_printf("\t\tCannot merge entry '%s' (%c): "
 			       "%.7s -> %.7s. Skipping!\n", p->one->path,
 			       p->status, oid_to_hex(&p->one->oid),
 			       oid_to_hex(&p->two->oid));
 			continue;
 		}
-		mp = find_notes_merge_pair_pos(changes, len, obj, 0, &match);
+		mp = find_notes_merge_pair_pos(changes, len, &obj, 0, &match);
 		if (!match) {
 			trace_printf("\t\tIgnoring local-only change for %s: "
-			       "%.7s -> %.7s\n", sha1_to_hex(obj),
+			       "%.7s -> %.7s\n", oid_to_hex(&obj),
 			       oid_to_hex(&p->one->oid),
 			       oid_to_hex(&p->two->oid));
 			continue;
 		}
 
-		assert(!hashcmp(mp->obj.hash, obj));
+		assert(!oidcmp(&mp->obj, &obj));
 		if (is_null_oid(&p->two->oid)) { /* deletion */
 			/*
 			 * Either this is a true deletion (1), or it is part
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 25/33] notes-merge: convert verify_notes_filepair to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 notes-merge.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/notes-merge.c b/notes-merge.c
index 55dbb3659..962e9b1bc 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -22,21 +22,21 @@ void init_notes_merge_options(struct notes_merge_options *o)
 	o->verbosity = NOTES_MERGE_VERBOSITY_DEFAULT;
 }
 
-static int path_to_sha1(const char *path, unsigned char *sha1)
+static int path_to_oid(const char *path, struct object_id *oid)
 {
-	char hex_sha1[40];
+	char hex_oid[GIT_SHA1_HEXSZ];
 	int i = 0;
-	while (*path && i < 40) {
+	while (*path && i < GIT_SHA1_HEXSZ) {
 		if (*path != '/')
-			hex_sha1[i++] = *path;
+			hex_oid[i++] = *path;
 		path++;
 	}
-	if (*path || i != 40)
+	if (*path || i != GIT_SHA1_HEXSZ)
 		return -1;
-	return get_sha1_hex(hex_sha1, sha1);
+	return get_oid_hex(hex_oid, oid);
 }
 
-static int verify_notes_filepair(struct diff_filepair *p, unsigned char *sha1)
+static int verify_notes_filepair(struct diff_filepair *p, struct object_id *oid)
 {
 	switch (p->status) {
 	case DIFF_STATUS_MODIFIED:
@@ -54,7 +54,7 @@ static int verify_notes_filepair(struct diff_filepair *p, unsigned char *sha1)
 		return -1;
 	}
 	assert(!strcmp(p->one->path, p->two->path));
-	return path_to_sha1(p->one->path, sha1);
+	return path_to_oid(p->one->path, oid);
 }
 
 static struct notes_merge_pair *find_notes_merge_pair_pos(
@@ -140,7 +140,7 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
 		int occupied;
 		struct object_id obj;
 
-		if (verify_notes_filepair(p, obj.hash)) {
+		if (verify_notes_filepair(p, &obj)) {
 			trace_printf("\t\tCannot merge entry '%s' (%c): "
 			       "%.7s -> %.7s. Skipping!\n", p->one->path,
 			       p->status, oid_to_hex(&p->one->oid),
@@ -201,7 +201,7 @@ static void diff_tree_local(struct notes_merge_options *o,
 		int match;
 		struct object_id obj;
 
-		if (verify_notes_filepair(p, obj.hash)) {
+		if (verify_notes_filepair(p, &obj)) {
 			trace_printf("\t\tCannot merge entry '%s' (%c): "
 			       "%.7s -> %.7s. Skipping!\n", p->one->path,
 			       p->status, oid_to_hex(&p->one->oid),
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 26/33] notes-merge: convert write_note_to_worktree to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 notes-merge.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/notes-merge.c b/notes-merge.c
index 962e9b1bc..7d88857a8 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -292,11 +292,11 @@ static void check_notes_merge_worktree(struct notes_merge_options *o)
 		    git_path(NOTES_MERGE_WORKTREE));
 }
 
-static void write_buf_to_worktree(const unsigned char *obj,
+static void write_buf_to_worktree(const struct object_id *obj,
 				  const char *buf, unsigned long size)
 {
 	int fd;
-	char *path = git_pathdup(NOTES_MERGE_WORKTREE "/%s", sha1_to_hex(obj));
+	char *path = git_pathdup(NOTES_MERGE_WORKTREE "/%s", oid_to_hex(obj));
 	if (safe_create_leading_directories_const(path))
 		die_errno("unable to create directory for '%s'", path);
 
@@ -320,19 +320,19 @@ static void write_buf_to_worktree(const unsigned char *obj,
 	free(path);
 }
 
-static void write_note_to_worktree(const unsigned char *obj,
-				   const unsigned char *note)
+static void write_note_to_worktree(const struct object_id *obj,
+				   const struct object_id *note)
 {
 	enum object_type type;
 	unsigned long size;
-	void *buf = read_sha1_file(note, &type, &size);
+	void *buf = read_sha1_file(note->hash, &type, &size);
 
 	if (!buf)
 		die("cannot read note %s for object %s",
-		    sha1_to_hex(note), sha1_to_hex(obj));
+		    oid_to_hex(note), oid_to_hex(obj));
 	if (type != OBJ_BLOB)
 		die("blob expected in note %s for object %s",
-		    sha1_to_hex(note), sha1_to_hex(obj));
+		    oid_to_hex(note), oid_to_hex(obj));
 	write_buf_to_worktree(obj, buf, size);
 	free(buf);
 }
@@ -358,7 +358,7 @@ static int ll_merge_in_worktree(struct notes_merge_options *o,
 	if ((status < 0) || !result_buf.ptr)
 		die("Failed to execute internal merge");
 
-	write_buf_to_worktree(p->obj.hash, result_buf.ptr, result_buf.size);
+	write_buf_to_worktree(&p->obj, result_buf.ptr, result_buf.size);
 	free(result_buf.ptr);
 
 	return status;
@@ -393,7 +393,7 @@ static int merge_one_change_manual(struct notes_merge_options *o,
 				"deleted in %s and modified in %s. Version from %s "
 				"left in tree.\n",
 				oid_to_hex(&p->obj), lref, rref, rref);
-		write_note_to_worktree(p->obj.hash, p->remote.hash);
+		write_note_to_worktree(&p->obj, &p->remote);
 	} else if (is_null_oid(&p->remote)) {
 		/* D/F conflict, checkout p->local */
 		assert(!is_null_oid(&p->local));
@@ -402,7 +402,7 @@ static int merge_one_change_manual(struct notes_merge_options *o,
 				"deleted in %s and modified in %s. Version from %s "
 				"left in tree.\n",
 				oid_to_hex(&p->obj), rref, lref, lref);
-		write_note_to_worktree(p->obj.hash, p->local.hash);
+		write_note_to_worktree(&p->obj, &p->local);
 	} else {
 		/* "regular" conflict, checkout result of ll_merge() */
 		const char *reason = "content";
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 27/33] diff-tree: convert diff_tree_sha1 to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/blame.c       | 20 ++++++++++----------
 builtin/diff-tree.c   |  8 +++-----
 builtin/diff.c        |  2 +-
 builtin/fast-export.c |  4 ++--
 builtin/log.c         |  6 +++---
 builtin/merge.c       |  2 +-
 combine-diff.c        |  4 ++--
 diff.h                |  5 +++--
 line-log.c            |  4 ++--
 log-tree.c            |  8 ++++----
 merge-recursive.c     |  2 +-
 notes-merge.c         |  4 ++--
 patch-ids.c           |  4 ++--
 revision.c            |  4 ++--
 sequencer.c           |  4 ++--
 tree-diff.c           | 12 +++++++-----
 16 files changed, 47 insertions(+), 46 deletions(-)

diff --git a/builtin/blame.c b/builtin/blame.c
index 5ad435380..7645aa991 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -565,9 +565,9 @@ static struct origin *find_origin(struct scoreboard *sb,
 	if (is_null_oid(&origin->commit->object.oid))
 		do_diff_cache(&parent->tree->object.oid, &diff_opts);
 	else
-		diff_tree_sha1(parent->tree->object.oid.hash,
-			       origin->commit->tree->object.oid.hash,
-			       "", &diff_opts);
+		diff_tree_oid(&parent->tree->object.oid,
+			      &origin->commit->tree->object.oid,
+			      "", &diff_opts);
 	diffcore_std(&diff_opts);
 
 	if (!diff_queued_diff.nr) {
@@ -635,9 +635,9 @@ static struct origin *find_rename(struct scoreboard *sb,
 	if (is_null_oid(&origin->commit->object.oid))
 		do_diff_cache(&parent->tree->object.oid, &diff_opts);
 	else
-		diff_tree_sha1(parent->tree->object.oid.hash,
-			       origin->commit->tree->object.oid.hash,
-			       "", &diff_opts);
+		diff_tree_oid(&parent->tree->object.oid,
+			      &origin->commit->tree->object.oid,
+			      "", &diff_opts);
 	diffcore_std(&diff_opts);
 
 	for (i = 0; i < diff_queued_diff.nr; i++) {
@@ -1262,7 +1262,7 @@ static void find_copy_in_parent(struct scoreboard *sb,
 	/* Try "find copies harder" on new path if requested;
 	 * we do not want to use diffcore_rename() actually to
 	 * match things up; find_copies_harder is set only to
-	 * force diff_tree_sha1() to feed all filepairs to diff_queue,
+	 * force diff_tree_oid() to feed all filepairs to diff_queue,
 	 * and this code needs to be after diff_setup_done(), which
 	 * usually makes find-copies-harder imply copy detection.
 	 */
@@ -1274,9 +1274,9 @@ static void find_copy_in_parent(struct scoreboard *sb,
 	if (is_null_oid(&target->commit->object.oid))
 		do_diff_cache(&parent->tree->object.oid, &diff_opts);
 	else
-		diff_tree_sha1(parent->tree->object.oid.hash,
-			       target->commit->tree->object.oid.hash,
-			       "", &diff_opts);
+		diff_tree_oid(&parent->tree->object.oid,
+			      &target->commit->tree->object.oid,
+			      "", &diff_opts);
 
 	if (!DIFF_OPT_TST(&diff_opts, FIND_COPIES_HARDER))
 		diffcore_std(&diff_opts);
diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 5ea1c1431..aef167619 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -49,8 +49,8 @@ static int stdin_diff_trees(struct tree *tree1, const char *p)
 		return -1;
 	printf("%s %s\n", oid_to_hex(&tree1->object.oid),
 			  oid_to_hex(&tree2->object.oid));
-	diff_tree_sha1(tree1->object.oid.hash, tree2->object.oid.hash,
-		       "", &log_tree_opt.diffopt);
+	diff_tree_oid(&tree1->object.oid, &tree2->object.oid,
+		      "", &log_tree_opt.diffopt);
 	log_tree_diff_flush(&log_tree_opt);
 	return 0;
 }
@@ -148,9 +148,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 		if (tree2->flags & UNINTERESTING) {
 			SWAP(tree2, tree1);
 		}
-		diff_tree_sha1(tree1->oid.hash,
-			       tree2->oid.hash,
-			       "", &opt->diffopt);
+		diff_tree_oid(&tree1->oid, &tree2->oid, "", &opt->diffopt);
 		log_tree_diff_flush(opt);
 		break;
 	}
diff --git a/builtin/diff.c b/builtin/diff.c
index 73b4ff3db..4c6a1a962 100644
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -174,7 +174,7 @@ static int builtin_diff_tree(struct rev_info *revs,
 		swap = 1;
 	oid[swap] = &ent0->item->oid;
 	oid[1 - swap] = &ent1->item->oid;
-	diff_tree_sha1(oid[0]->hash, oid[1]->hash, "", &revs->diffopt);
+	diff_tree_oid(oid[0], oid[1], "", &revs->diffopt);
 	log_tree_diff_flush(revs);
 	return 0;
 }
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index e242726f0..d57f36c43 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -562,8 +562,8 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
 	    get_object_mark(&commit->parents->item->object) != 0 &&
 	    !full_tree) {
 		parse_commit_or_die(commit->parents->item);
-		diff_tree_sha1(commit->parents->item->tree->object.oid.hash,
-			       commit->tree->object.oid.hash, "", &rev->diffopt);
+		diff_tree_oid(&commit->parents->item->tree->object.oid,
+			      &commit->tree->object.oid, "", &rev->diffopt);
 	}
 	else
 		diff_root_tree_oid(&commit->tree->object.oid,
diff --git a/builtin/log.c b/builtin/log.c
index 6bdba3444..4ef522ee5 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1043,9 +1043,9 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout,
 
 	diff_setup_done(&opts);
 
-	diff_tree_sha1(origin->tree->object.oid.hash,
-		       head->tree->object.oid.hash,
-		       "", &opts);
+	diff_tree_oid(&origin->tree->object.oid,
+		      &head->tree->object.oid,
+		      "", &opts);
 	diffcore_std(&opts);
 	diff_flush(&opts);
 
diff --git a/builtin/merge.c b/builtin/merge.c
index a4a098f40..afaed6a2c 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -415,7 +415,7 @@ static void finish(struct commit *head_commit,
 			DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT;
 		opts.detect_rename = DIFF_DETECT_RENAME;
 		diff_setup_done(&opts);
-		diff_tree_sha1(head->hash, new_head->hash, "", &opts);
+		diff_tree_oid(head, new_head, "", &opts);
 		diffcore_std(&opts);
 		diff_flush(&opts);
 	}
diff --git a/combine-diff.c b/combine-diff.c
index c82364510..04c4ae856 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1336,7 +1336,7 @@ static struct combine_diff_path *find_paths_generic(const struct object_id *oid,
 			opt->output_format = stat_opt;
 		else
 			opt->output_format = DIFF_FORMAT_NO_OUTPUT;
-		diff_tree_sha1(parents->oid[i].hash, oid->hash, "", opt);
+		diff_tree_oid(&parents->oid[i], oid, "", opt);
 		diffcore_std(opt);
 		paths = intersect_paths(paths, i, num_parent);
 
@@ -1463,7 +1463,7 @@ void diff_tree_combined(const struct object_id *oid,
 		if (stat_opt) {
 			diffopts.output_format = stat_opt;
 
-			diff_tree_sha1(parents->oid[0].hash, oid->hash, "", &diffopts);
+			diff_tree_oid(&parents->oid[0], oid, "", &diffopts);
 			diffcore_std(&diffopts);
 			if (opt->orderfile)
 				diffcore_order(opt->orderfile);
diff --git a/diff.h b/diff.h
index 8d46a6709..e0b503460 100644
--- a/diff.h
+++ b/diff.h
@@ -213,8 +213,9 @@ extern struct combine_diff_path *diff_tree_paths(
 	struct combine_diff_path *p, const unsigned char *sha1,
 	const unsigned char **parent_sha1, int nparent,
 	struct strbuf *base, struct diff_options *opt);
-extern int diff_tree_sha1(const unsigned char *old, const unsigned char *new,
-			  const char *base, struct diff_options *opt);
+extern int diff_tree_oid(const struct object_id *old_oid,
+			 const struct object_id *new_oid,
+			 const char *base, struct diff_options *opt);
 extern int diff_root_tree_oid(const struct object_id *new_oid, const char *base,
 			      struct diff_options *opt);
 
diff --git a/line-log.c b/line-log.c
index a3bd2f2d5..2588ce076 100644
--- a/line-log.c
+++ b/line-log.c
@@ -819,8 +819,8 @@ static void queue_diffs(struct line_log_data *range,
 	assert(commit);
 
 	DIFF_QUEUE_CLEAR(&diff_queued_diff);
-	diff_tree_sha1(parent ? parent->tree->object.oid.hash : NULL,
-			commit->tree->object.oid.hash, "", opt);
+	diff_tree_oid(parent ? &parent->tree->object.oid : NULL,
+		      &commit->tree->object.oid, "", opt);
 	if (opt->detect_rename) {
 		filter_diffs_for_paths(range, 1);
 		if (diff_might_be_rename())
diff --git a/log-tree.c b/log-tree.c
index b40242534..2903874ec 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -822,8 +822,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 			 * we merged _in_.
 			 */
 			parse_commit_or_die(parents->item);
-			diff_tree_sha1(parents->item->tree->object.oid.hash,
-				       oid->hash, "", &opt->diffopt);
+			diff_tree_oid(&parents->item->tree->object.oid,
+				      oid, "", &opt->diffopt);
 			log_tree_diff_flush(opt);
 			return !opt->loginfo;
 		}
@@ -837,8 +837,8 @@ static int log_tree_diff(struct rev_info *opt, struct commit *commit, struct log
 		struct commit *parent = parents->item;
 
 		parse_commit_or_die(parent);
-		diff_tree_sha1(parent->tree->object.oid.hash,
-			       oid->hash, "", &opt->diffopt);
+		diff_tree_oid(&parent->tree->object.oid,
+			      oid, "", &opt->diffopt);
 		log_tree_diff_flush(opt);
 
 		showed_log |= !opt->loginfo;
diff --git a/merge-recursive.c b/merge-recursive.c
index ae5238d82..5cc86df2d 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -528,7 +528,7 @@ static struct string_list *get_renames(struct merge_options *o,
 	opts.show_rename_progress = o->show_rename_progress;
 	opts.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_setup_done(&opts);
-	diff_tree_sha1(o_tree->object.oid.hash, tree->object.oid.hash, "", &opts);
+	diff_tree_oid(&o_tree->object.oid, &tree->object.oid, "", &opts);
 	diffcore_std(&opts);
 	if (opts.needed_rename_limit > o->needed_rename_limit)
 		o->needed_rename_limit = opts.needed_rename_limit;
diff --git a/notes-merge.c b/notes-merge.c
index 7d88857a8..70e3fbeef 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -129,7 +129,7 @@ static struct notes_merge_pair *diff_tree_remote(struct notes_merge_options *o,
 	DIFF_OPT_SET(&opt, RECURSIVE);
 	opt.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_setup_done(&opt);
-	diff_tree_sha1(base->hash, remote->hash, "", &opt);
+	diff_tree_oid(base, remote, "", &opt);
 	diffcore_std(&opt);
 
 	changes = xcalloc(diff_queued_diff.nr, sizeof(struct notes_merge_pair));
@@ -192,7 +192,7 @@ static void diff_tree_local(struct notes_merge_options *o,
 	DIFF_OPT_SET(&opt, RECURSIVE);
 	opt.output_format = DIFF_FORMAT_NO_OUTPUT;
 	diff_setup_done(&opt);
-	diff_tree_sha1(base->hash, local->hash, "", &opt);
+	diff_tree_oid(base, local, "", &opt);
 	diffcore_std(&opt);
 
 	for (i = 0; i < diff_queued_diff.nr; i++) {
diff --git a/patch-ids.c b/patch-ids.c
index aaf462c03..9c0ab9e67 100644
--- a/patch-ids.c
+++ b/patch-ids.c
@@ -17,8 +17,8 @@ int commit_patch_id(struct commit *commit, struct diff_options *options,
 		return -1;
 
 	if (commit->parents)
-		diff_tree_sha1(commit->parents->item->object.oid.hash,
-			       commit->object.oid.hash, "", options);
+		diff_tree_oid(&commit->parents->item->object.oid,
+			      &commit->object.oid, "", options);
 	else
 		diff_root_tree_oid(&commit->object.oid, "", options);
 	diffcore_std(options);
diff --git a/revision.c b/revision.c
index 7637e7556..3030f33ee 100644
--- a/revision.c
+++ b/revision.c
@@ -455,7 +455,7 @@ static int rev_compare_tree(struct rev_info *revs,
 
 	tree_difference = REV_TREE_SAME;
 	DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
-	if (diff_tree_sha1(t1->object.oid.hash, t2->object.oid.hash, "",
+	if (diff_tree_oid(&t1->object.oid, &t2->object.oid, "",
 			   &revs->pruning) < 0)
 		return REV_TREE_DIFFERENT;
 	return tree_difference;
@@ -471,7 +471,7 @@ static int rev_same_tree_as_empty(struct rev_info *revs, struct commit *commit)
 
 	tree_difference = REV_TREE_SAME;
 	DIFF_OPT_CLR(&revs->pruning, HAS_CHANGES);
-	retval = diff_tree_sha1(NULL, t1->object.oid.hash, "", &revs->pruning);
+	retval = diff_tree_oid(NULL, &t1->object.oid, "", &revs->pruning);
 
 	return retval >= 0 && (tree_difference == REV_TREE_SAME);
 }
diff --git a/sequencer.c b/sequencer.c
index a23b948ac..7a114def8 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2129,8 +2129,8 @@ static int pick_commits(struct todo_list *todo_list, struct replay_opts *opts)
 			if (read_oneliner(&buf, rebase_path_orig_head(), 0) &&
 			    !get_sha1(buf.buf, orig.hash) &&
 			    !get_sha1("HEAD", head.hash)) {
-				diff_tree_sha1(orig.hash, head.hash,
-					       "", &log_tree_opt.diffopt);
+				diff_tree_oid(&orig, &head, "",
+					      &log_tree_opt.diffopt);
 				log_tree_diff_flush(&log_tree_opt);
 			}
 		}
diff --git a/tree-diff.c b/tree-diff.c
index f9bbaf3c4..fc020d76d 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -419,7 +419,7 @@ static struct combine_diff_path *ll_diff_tree_paths(
 	 * load parents first, as they are probably already cached.
 	 *
 	 * ( log_tree_diff() parses commit->parent before calling here via
-	 *   diff_tree_sha1(parent, commit) )
+	 *   diff_tree_oid(parent, commit) )
 	 */
 	for (i = 0; i < nparent; ++i)
 		tptree[i] = fill_tree_descriptor(&tp[i], parents_sha1[i]);
@@ -694,7 +694,9 @@ static int ll_diff_tree_sha1(const unsigned char *old, const unsigned char *new,
 	return 0;
 }
 
-int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const char *base_str, struct diff_options *opt)
+int diff_tree_oid(const struct object_id *old_oid,
+		  const struct object_id *new_oid,
+		  const char *base_str, struct diff_options *opt)
 {
 	struct strbuf base;
 	int retval;
@@ -702,9 +704,9 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha
 	strbuf_init(&base, PATH_MAX);
 	strbuf_addstr(&base, base_str);
 
-	retval = ll_diff_tree_sha1(old, new, &base, opt);
+	retval = ll_diff_tree_sha1(old_oid->hash, new_oid->hash, &base, opt);
 	if (!*base_str && DIFF_OPT_TST(opt, FOLLOW_RENAMES) && diff_might_be_rename())
-		try_to_follow_renames(old, new, &base, opt);
+		try_to_follow_renames(old_oid->hash, new_oid->hash, &base, opt);
 
 	strbuf_release(&base);
 
@@ -713,5 +715,5 @@ int diff_tree_sha1(const unsigned char *old, const unsigned char *new, const cha
 
 int diff_root_tree_oid(const struct object_id *new_oid, const char *base, struct diff_options *opt)
 {
-	return diff_tree_sha1(NULL, new_oid->hash, base, opt);
+	return diff_tree_oid(NULL, new_oid, base, opt);
 }
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 28/33] builtin/diff-tree: cleanup references to sha1

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/diff-tree.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index aef167619..8b26a66a9 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -7,7 +7,7 @@
 
 static struct rev_info log_tree_opt;
 
-static int diff_tree_commit_sha1(const struct object_id *oid)
+static int diff_tree_commit_oid(const struct object_id *oid)
 {
 	struct commit *commit = lookup_commit_reference(oid);
 	if (!commit)
@@ -98,7 +98,6 @@ static void diff_tree_tweak_rev(struct rev_info *rev, struct setup_revision_opt
 
 int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 {
-	int nr_sha1;
 	char line[1000];
 	struct object *tree1, *tree2;
 	static struct rev_info *opt = &log_tree_opt;
@@ -132,15 +131,14 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 	 * reverse the order of the objects if the second one
 	 * is marked UNINTERESTING.
 	 */
-	nr_sha1 = opt->pending.nr;
-	switch (nr_sha1) {
+	switch (opt->pending.nr) {
 	case 0:
 		if (!read_stdin)
 			usage(diff_tree_usage);
 		break;
 	case 1:
 		tree1 = opt->pending.objects[0].item;
-		diff_tree_commit_sha1(&tree1->oid);
+		diff_tree_commit_oid(&tree1->oid);
 		break;
 	case 2:
 		tree1 = opt->pending.objects[0].item;
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 29/33] tree-diff: convert try_to_follow_renames to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 tree-diff.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tree-diff.c b/tree-diff.c
index fc020d76d..29e3f6144 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -577,7 +577,9 @@ static inline int diff_might_be_rename(void)
 		!DIFF_FILE_VALID(diff_queued_diff.queue[0]->one);
 }
 
-static void try_to_follow_renames(const unsigned char *old, const unsigned char *new, struct strbuf *base, struct diff_options *opt)
+static void try_to_follow_renames(const struct object_id *old_oid,
+				  const struct object_id *new_oid,
+				  struct strbuf *base, struct diff_options *opt)
 {
 	struct diff_options diff_opts;
 	struct diff_queue_struct *q = &diff_queued_diff;
@@ -615,7 +617,7 @@ static void try_to_follow_renames(const unsigned char *old, const unsigned char
 	diff_opts.break_opt = opt->break_opt;
 	diff_opts.rename_score = opt->rename_score;
 	diff_setup_done(&diff_opts);
-	ll_diff_tree_sha1(old, new, base, &diff_opts);
+	ll_diff_tree_sha1(old_oid->hash, new_oid->hash, base, &diff_opts);
 	diffcore_std(&diff_opts);
 	clear_pathspec(&diff_opts.pathspec);
 
@@ -706,7 +708,7 @@ int diff_tree_oid(const struct object_id *old_oid,
 
 	retval = ll_diff_tree_sha1(old_oid->hash, new_oid->hash, &base, opt);
 	if (!*base_str && DIFF_OPT_TST(opt, FOLLOW_RENAMES) && diff_might_be_rename())
-		try_to_follow_renames(old_oid->hash, new_oid->hash, &base, opt);
+		try_to_follow_renames(old_oid, new_oid, &base, opt);
 
 	strbuf_release(&base);
 
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 30/33] tree-diff: convert diff_tree_paths to struct object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 combine-diff.c | 10 +++++-----
 diff.h         |  4 ++--
 tree-diff.c    | 63 +++++++++++++++++++++++++++++-----------------------------
 3 files changed, 39 insertions(+), 38 deletions(-)

diff --git a/combine-diff.c b/combine-diff.c
index 04c4ae856..ec9d93044 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -1364,22 +1364,22 @@ static struct combine_diff_path *find_paths_multitree(
 	struct diff_options *opt)
 {
 	int i, nparent = parents->nr;
-	const unsigned char **parents_sha1;
+	const struct object_id **parents_oid;
 	struct combine_diff_path paths_head;
 	struct strbuf base;
 
-	ALLOC_ARRAY(parents_sha1, nparent);
+	ALLOC_ARRAY(parents_oid, nparent);
 	for (i = 0; i < nparent; i++)
-		parents_sha1[i] = parents->oid[i].hash;
+		parents_oid[i] = &parents->oid[i];
 
 	/* fake list head, so worker can assume it is non-NULL */
 	paths_head.next = NULL;
 
 	strbuf_init(&base, PATH_MAX);
-	diff_tree_paths(&paths_head, oid->hash, parents_sha1, nparent, &base, opt);
+	diff_tree_paths(&paths_head, oid, parents_oid, nparent, &base, opt);
 
 	strbuf_release(&base);
-	free(parents_sha1);
+	free(parents_oid);
 	return paths_head.next;
 }
 
diff --git a/diff.h b/diff.h
index e0b503460..0d0c14f28 100644
--- a/diff.h
+++ b/diff.h
@@ -210,8 +210,8 @@ const char *diff_line_prefix(struct diff_options *);
 extern const char mime_boundary_leader[];
 
 extern struct combine_diff_path *diff_tree_paths(
-	struct combine_diff_path *p, const unsigned char *sha1,
-	const unsigned char **parent_sha1, int nparent,
+	struct combine_diff_path *p, const struct object_id *oid,
+	const struct object_id **parents_oid, int nparent,
 	struct strbuf *base, struct diff_options *opt);
 extern int diff_tree_oid(const struct object_id *old_oid,
 			 const struct object_id *new_oid,
diff --git a/tree-diff.c b/tree-diff.c
index 29e3f6144..6a960f569 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -26,11 +26,12 @@
 } while(0)
 
 static struct combine_diff_path *ll_diff_tree_paths(
-	struct combine_diff_path *p, const unsigned char *sha1,
-	const unsigned char **parents_sha1, int nparent,
+	struct combine_diff_path *p, const struct object_id *oid,
+	const struct object_id **parents_oid, int nparent,
 	struct strbuf *base, struct diff_options *opt);
-static int ll_diff_tree_sha1(const unsigned char *old, const unsigned char *new,
-			     struct strbuf *base, struct diff_options *opt);
+static int ll_diff_tree_oid(const struct object_id *old_oid,
+			    const struct object_id *new_oid,
+			    struct strbuf *base, struct diff_options *opt);
 
 /*
  * Compare two tree entries, taking into account only path/S_ISDIR(mode),
@@ -183,7 +184,7 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
 {
 	unsigned mode;
 	const char *path;
-	const unsigned char *sha1;
+	const struct object_id *oid;
 	int pathlen;
 	int old_baselen = base->len;
 	int i, isdir, recurse = 0, emitthis = 1;
@@ -193,7 +194,7 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
 
 	if (t) {
 		/* path present in resulting tree */
-		sha1 = tree_entry_extract(t, &path, &mode)->hash;
+		oid = tree_entry_extract(t, &path, &mode);
 		pathlen = tree_entry_len(&t->entry);
 		isdir = S_ISDIR(mode);
 	} else {
@@ -208,7 +209,7 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
 		pathlen = tree_entry_len(&tp[imin].entry);
 
 		isdir = S_ISDIR(mode);
-		sha1 = NULL;
+		oid = NULL;
 		mode = 0;
 	}
 
@@ -220,7 +221,7 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
 	if (emitthis) {
 		int keep;
 		struct combine_diff_path *pprev = p;
-		p = path_appendnew(p, nparent, base, path, pathlen, mode, sha1);
+		p = path_appendnew(p, nparent, base, path, pathlen, mode, oid ? oid->hash : NULL);
 
 		for (i = 0; i < nparent; ++i) {
 			/*
@@ -229,7 +230,7 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
 			 */
 			int tpi_valid = tp && !(tp[i].entry.mode & S_IFXMIN_NEQ);
 
-			const unsigned char *sha1_i;
+			const struct object_id *oid_i;
 			unsigned mode_i;
 
 			p->parent[i].status =
@@ -239,16 +240,16 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
 						DIFF_STATUS_ADDED;
 
 			if (tpi_valid) {
-				sha1_i = tp[i].entry.oid->hash;
+				oid_i = tp[i].entry.oid;
 				mode_i = tp[i].entry.mode;
 			}
 			else {
-				sha1_i = NULL;
+				oid_i = &null_oid;
 				mode_i = 0;
 			}
 
 			p->parent[i].mode = mode_i;
-			hashcpy(p->parent[i].oid.hash, sha1_i ? sha1_i : null_sha1);
+			oidcpy(&p->parent[i].oid, oid_i);
 		}
 
 		keep = 1;
@@ -273,21 +274,20 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
 	}
 
 	if (recurse) {
-		const unsigned char **parents_sha1;
+		const struct object_id **parents_oid;
 
-		FAST_ARRAY_ALLOC(parents_sha1, nparent);
+		FAST_ARRAY_ALLOC(parents_oid, nparent);
 		for (i = 0; i < nparent; ++i) {
 			/* same rule as in emitthis */
 			int tpi_valid = tp && !(tp[i].entry.mode & S_IFXMIN_NEQ);
 
-			parents_sha1[i] = tpi_valid ? tp[i].entry.oid->hash
-						    : NULL;
+			parents_oid[i] = tpi_valid ? tp[i].entry.oid : NULL;
 		}
 
 		strbuf_add(base, path, pathlen);
 		strbuf_addch(base, '/');
-		p = ll_diff_tree_paths(p, sha1, parents_sha1, nparent, base, opt);
-		FAST_ARRAY_FREE(parents_sha1, nparent);
+		p = ll_diff_tree_paths(p, oid, parents_oid, nparent, base, opt);
+		FAST_ARRAY_FREE(parents_oid, nparent);
 	}
 
 	strbuf_setlen(base, old_baselen);
@@ -312,7 +312,7 @@ static void skip_uninteresting(struct tree_desc *t, struct strbuf *base,
 
 
 /*
- * generate paths for combined diff D(sha1,parents_sha1[])
+ * generate paths for combined diff D(sha1,parents_oid[])
  *
  * Resulting paths are appended to combine_diff_path linked list, and also, are
  * emitted on the go via opt->pathchange() callback, so it is possible to
@@ -404,8 +404,8 @@ static inline void update_tp_entries(struct tree_desc *tp, int nparent)
 }
 
 static struct combine_diff_path *ll_diff_tree_paths(
-	struct combine_diff_path *p, const unsigned char *sha1,
-	const unsigned char **parents_sha1, int nparent,
+	struct combine_diff_path *p, const struct object_id *oid,
+	const struct object_id **parents_oid, int nparent,
 	struct strbuf *base, struct diff_options *opt)
 {
 	struct tree_desc t, *tp;
@@ -422,8 +422,8 @@ static struct combine_diff_path *ll_diff_tree_paths(
 	 *   diff_tree_oid(parent, commit) )
 	 */
 	for (i = 0; i < nparent; ++i)
-		tptree[i] = fill_tree_descriptor(&tp[i], parents_sha1[i]);
-	ttree = fill_tree_descriptor(&t, sha1);
+		tptree[i] = fill_tree_descriptor(&tp[i], parents_oid[i]->hash);
+	ttree = fill_tree_descriptor(&t, oid->hash);
 
 	/* Enable recursion indefinitely */
 	opt->pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);
@@ -548,11 +548,11 @@ static struct combine_diff_path *ll_diff_tree_paths(
 }
 
 struct combine_diff_path *diff_tree_paths(
-	struct combine_diff_path *p, const unsigned char *sha1,
-	const unsigned char **parents_sha1, int nparent,
+	struct combine_diff_path *p, const struct object_id *oid,
+	const struct object_id **parents_oid, int nparent,
 	struct strbuf *base, struct diff_options *opt)
 {
-	p = ll_diff_tree_paths(p, sha1, parents_sha1, nparent, base, opt);
+	p = ll_diff_tree_paths(p, oid, parents_oid, nparent, base, opt);
 
 	/*
 	 * free pre-allocated last element, if any
@@ -617,7 +617,7 @@ static void try_to_follow_renames(const struct object_id *old_oid,
 	diff_opts.break_opt = opt->break_opt;
 	diff_opts.rename_score = opt->rename_score;
 	diff_setup_done(&diff_opts);
-	ll_diff_tree_sha1(old_oid->hash, new_oid->hash, base, &diff_opts);
+	ll_diff_tree_oid(old_oid, new_oid, base, &diff_opts);
 	diffcore_std(&diff_opts);
 	clear_pathspec(&diff_opts.pathspec);
 
@@ -676,15 +676,16 @@ static void try_to_follow_renames(const struct object_id *old_oid,
 	q->nr = 1;
 }
 
-static int ll_diff_tree_sha1(const unsigned char *old, const unsigned char *new,
-			     struct strbuf *base, struct diff_options *opt)
+static int ll_diff_tree_oid(const struct object_id *old_oid,
+			    const struct object_id *new_oid,
+			    struct strbuf *base, struct diff_options *opt)
 {
 	struct combine_diff_path phead, *p;
 	pathchange_fn_t pathchange_old = opt->pathchange;
 
 	phead.next = NULL;
 	opt->pathchange = emit_diff_first_parent_only;
-	diff_tree_paths(&phead, new, &old, 1, base, opt);
+	diff_tree_paths(&phead, new_oid, &old_oid, 1, base, opt);
 
 	for (p = phead.next; p;) {
 		struct combine_diff_path *pprev = p;
@@ -706,7 +707,7 @@ int diff_tree_oid(const struct object_id *old_oid,
 	strbuf_init(&base, PATH_MAX);
 	strbuf_addstr(&base, base_str);
 
-	retval = ll_diff_tree_sha1(old_oid->hash, new_oid->hash, &base, opt);
+	retval = ll_diff_tree_oid(old_oid, new_oid, &base, opt);
 	if (!*base_str && DIFF_OPT_TST(opt, FOLLOW_RENAMES) && diff_might_be_rename())
 		try_to_follow_renames(old_oid, new_oid, &base, opt);
 
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 31/33] tree-diff: convert path_appendnew to object_id

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 tree-diff.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tree-diff.c b/tree-diff.c
index 6a960f569..467e38172 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -132,7 +132,7 @@ static int emit_diff_first_parent_only(struct diff_options *opt, struct combine_
  */
 static struct combine_diff_path *path_appendnew(struct combine_diff_path *last,
 	int nparent, const struct strbuf *base, const char *path, int pathlen,
-	unsigned mode, const unsigned char *sha1)
+	unsigned mode, const struct object_id *oid)
 {
 	struct combine_diff_path *p;
 	size_t len = st_add(base->len, pathlen);
@@ -162,7 +162,7 @@ static struct combine_diff_path *path_appendnew(struct combine_diff_path *last,
 	memcpy(p->path + base->len, path, pathlen);
 	p->path[len] = 0;
 	p->mode = mode;
-	hashcpy(p->oid.hash, sha1 ? sha1 : null_sha1);
+	oidcpy(&p->oid, oid ? oid : &null_oid);
 
 	return p;
 }
@@ -221,7 +221,7 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
 	if (emitthis) {
 		int keep;
 		struct combine_diff_path *pprev = p;
-		p = path_appendnew(p, nparent, base, path, pathlen, mode, oid ? oid->hash : NULL);
+		p = path_appendnew(p, nparent, base, path, pathlen, mode, oid);
 
 		for (i = 0; i < nparent; ++i) {
 			/*
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 32/33] diffcore-rename: use is_empty_blob_oid

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 diffcore-rename.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/diffcore-rename.c b/diffcore-rename.c
index 3d9719dad..03d1e8d40 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -464,7 +464,7 @@ void diffcore_rename(struct diff_options *options)
 				 strcmp(options->single_follow, p->two->path))
 				continue; /* not interested */
 			else if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&
-				 is_empty_blob_sha1(p->two->oid.hash))
+				 is_empty_blob_oid(&p->two->oid))
 				continue;
 			else if (add_rename_dst(p->two) < 0) {
 				warning("skipping rename detection, detected"
@@ -474,7 +474,7 @@ void diffcore_rename(struct diff_options *options)
 			}
 		}
 		else if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&
-			 is_empty_blob_sha1(p->one->oid.hash))
+			 is_empty_blob_oid(&p->one->oid))
 			continue;
 		else if (!DIFF_PAIR_UNMERGED(p) && !DIFF_FILE_VALID(p->two)) {
 			/*
-- 
2.13.0.219.gdb65acc882-goog

[PATCH 33/33] diff: rename diff_fill_sha1_info to diff_fill_oid_info

[Brandon Williams]

Signed-off-by: Brandon Williams <bmwill@google.com>
---
 diff.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/diff.c b/diff.c
index dd325e616..c758a0d73 100644
--- a/diff.c
+++ b/diff.c
@@ -3239,7 +3239,7 @@ static void run_diff_cmd(const char *pgm,
 		fprintf(o->file, "* Unmerged path %s\n", name);
 }
 
-static void diff_fill_sha1_info(struct diff_filespec *one)
+static void diff_fill_oid_info(struct diff_filespec *one)
 {
 	if (DIFF_FILE_VALID(one)) {
 		if (!one->oid_valid) {
@@ -3298,8 +3298,8 @@ static void run_diff(struct diff_filepair *p, struct diff_options *o)
 		return;
 	}
 
-	diff_fill_sha1_info(one);
-	diff_fill_sha1_info(two);
+	diff_fill_oid_info(one);
+	diff_fill_oid_info(two);
 
 	if (!pgm &&
 	    DIFF_FILE_VALID(one) && DIFF_FILE_VALID(two) &&
@@ -3344,8 +3344,8 @@ static void run_diffstat(struct diff_filepair *p, struct diff_options *o,
 	if (o->prefix_length)
 		strip_prefix(o->prefix_length, &name, &other);
 
-	diff_fill_sha1_info(p->one);
-	diff_fill_sha1_info(p->two);
+	diff_fill_oid_info(p->one);
+	diff_fill_oid_info(p->two);
 
 	builtin_diffstat(name, other, p->one, p->two, diffstat, o, p);
 }
@@ -3368,8 +3368,8 @@ static void run_checkdiff(struct diff_filepair *p, struct diff_options *o)
 	if (o->prefix_length)
 		strip_prefix(o->prefix_length, &name, &other);
 
-	diff_fill_sha1_info(p->one);
-	diff_fill_sha1_info(p->two);
+	diff_fill_oid_info(p->one);
+	diff_fill_oid_info(p->two);
 
 	builtin_checkdiff(name, other, attr_path, p->one, p->two, o);
 }
@@ -4616,8 +4616,8 @@ static int diff_get_patch_id(struct diff_options *options, struct object_id *oid
 		if (DIFF_PAIR_UNMERGED(p))
 			continue;
 
-		diff_fill_sha1_info(p->one);
-		diff_fill_sha1_info(p->two);
+		diff_fill_oid_info(p->one);
+		diff_fill_oid_info(p->two);
 
 		len1 = remove_space(p->one->path, strlen(p->one->path));
 		len2 = remove_space(p->two->path, strlen(p->two->path));
-- 
2.13.0.219.gdb65acc882-goog

Re: [PATCH 16/33] diff: finish conversion for prepare_temp_file to struct object_id

[Stefan Beller]

On Tue, May 30, 2017 at 10:30 AM, Brandon Williams <bmwill@google.com> wrote:
> Signed-off-by: Brandon Williams <bmwill@google.com>

Up to (and including) this commit, the series looks good to me.
Will continue to review this series later.

Thanks,
Stefan

> ---
>  diff.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 084c8b2d0..a8ceeb024 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -3030,13 +3030,13 @@ static struct diff_tempfile *prepare_temp_file(const char *name,
>                         /* we can borrow from the file in the work tree */
>                         temp->name = name;
>                         if (!one->oid_valid)
> -                               sha1_to_hex_r(temp->hex, null_sha1);
> +                               oid_to_hex_r(temp->hex, &null_oid);
>                         else
>                                 oid_to_hex_r(temp->hex, &one->oid);
>                         /* Even though we may sometimes borrow the
>                          * contents from the work tree, we always want
>                          * one->mode.  mode is trustworthy even when
> -                        * !(one->sha1_valid), as long as
> +                        * !(one->oid_valid), as long as
>                          * DIFF_FILE_VALID(one).
>                          */
>                         xsnprintf(temp->mode, sizeof(temp->mode), "%06o", one->mode);
> --
> 2.13.0.219.gdb65acc882-goog
>

Re: [PATCH 20/33] combine-diff: convert find_paths_* to struct object_id

[Stefan Beller]

On Tue, May 30, 2017 at 10:30 AM, Brandon Williams <bmwill@google.com> wrote:
> Convert find_paths_generic and find_paths_multitree to use struct
> object_id.
>
>

double empty line?

Unlike some other prior patches, it is nice to see a commit message here. :)

Code of the patches up to here looks good.

Re: [PATCH 22/33] notes-merge: convert notes_merge* to struct object_id

[Stefan Beller]

On Tue, May 30, 2017 at 10:30 AM, Brandon Williams <bmwill@google.com> wrote:
> Convert notes_merge and notes_merge_commit to use struct object_id.
>
>
> Signed-off-by: Brandon Williams <bmwill@google.com>

spurious new line before sign off.

Re: [PATCH 23/33] notes-merge: convert merge_from_diffs to struct object_id

[Stefan Beller]

> @@ -505,16 +505,17 @@ static int merge_changes(struct notes_merge_options *o,
>  }
>
>  static int merge_from_diffs(struct notes_merge_options *o,
> -                           const unsigned char *base,
> -                           const unsigned char *local,
> -                           const unsigned char *remote, struct notes_tree *t)
> +                           const struct object_id *base,
> +                           const struct object_id *local,
> +                           const struct object_id *remote,
> +                           struct notes_tree *t)
>  {
>         struct notes_merge_pair *changes;
>         int num_changes, conflicts;
>
>         trace_printf("\tmerge_from_diffs(base = %.7s, local = %.7s, "
> -              "remote = %.7s)\n", sha1_to_hex(base), sha1_to_hex(local),
> -              sha1_to_hex(remote));
> +              "remote = %.7s)\n", oid_to_hex(base), oid_to_hex(local),
> +              oid_to_hex(remote));

This part is interesting.

7 used to be the default abbreviation length for sha1s (see
e6c587c733 (abbrev: auto size the default abbreviation, 2016-09-30)
for details) and this may have been a left over from the conversion
to variable length abbreviated hashes. This could be a reminder
we want to fix these, too. (Though not in this series?).

Roughly like

    trace_printf("\tmerge_from_diffs(base = %s, local = %s, remote = %s)\n",
        find_unique_abbrev(base.hash, DEFAULT_ABBREV),
        find_unique_abbrev(local.hash, DEFAULT_ABBREV)
        find_unique_abbrev(remote.hash, DEFAULT_ABBREV));

Re: [PATCH 25/33] notes-merge: convert verify_notes_filepair to struct object_id

[Stefan Beller]

On Tue, May 30, 2017 at 10:31 AM, Brandon Williams <bmwill@google.com> wrote:
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  notes-merge.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/notes-merge.c b/notes-merge.c
> index 55dbb3659..962e9b1bc 100644
> --- a/notes-merge.c
> +++ b/notes-merge.c
> @@ -22,21 +22,21 @@ void init_notes_merge_options(struct notes_merge_options *o)
>         o->verbosity = NOTES_MERGE_VERBOSITY_DEFAULT;
>  }
>
> -static int path_to_sha1(const char *path, unsigned char *sha1)
> +static int path_to_oid(const char *path, struct object_id *oid)
>  {
> -       char hex_sha1[40];
> +       char hex_oid[GIT_SHA1_HEXSZ];

Here we better use GIT_MAX_HEXSZ ?

Re: [PATCH 30/33] tree-diff: convert diff_tree_paths to struct object_id

[Stefan Beller]

On Tue, May 30, 2017 at 10:31 AM, Brandon Williams <bmwill@google.com> wrote:
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  combine-diff.c | 10 +++++-----
>  diff.h         |  4 ++--
>  tree-diff.c    | 63 +++++++++++++++++++++++++++++-----------------------------
>  3 files changed, 39 insertions(+), 38 deletions(-)
>
> diff --git a/combine-diff.c b/combine-diff.c
> index 04c4ae856..ec9d93044 100644
> --- a/combine-diff.c
> +++ b/combine-diff.c
> @@ -1364,22 +1364,22 @@ static struct combine_diff_path *find_paths_multitree(
>         struct diff_options *opt)
>  {
>         int i, nparent = parents->nr;
> -       const unsigned char **parents_sha1;
> +       const struct object_id **parents_oid;
>         struct combine_diff_path paths_head;
>         struct strbuf base;
>
> -       ALLOC_ARRAY(parents_sha1, nparent);
> +       ALLOC_ARRAY(parents_oid, nparent);
>         for (i = 0; i < nparent; i++)
> -               parents_sha1[i] = parents->oid[i].hash;
> +               parents_oid[i] = &parents->oid[i];

I have the impression that we could get away with one layer less
of indirection. Previously we had a heap allocated array (*) of (char*),
now we'd have a an array (*) of pointers(*) of the oid struct, that
is a (char[]) essentially. Maybe I am just confused?

Re: [PATCH 30/33] tree-diff: convert diff_tree_paths to struct object_id

[Jeff King]

On Wed, May 31, 2017 at 11:24:33AM -0700, Stefan Beller wrote:

> On Tue, May 30, 2017 at 10:31 AM, Brandon Williams <bmwill@google.com> wrote:
> >
> > Signed-off-by: Brandon Williams <bmwill@google.com>
> > ---
> >  combine-diff.c | 10 +++++-----
> >  diff.h         |  4 ++--
> >  tree-diff.c    | 63 +++++++++++++++++++++++++++++-----------------------------
> >  3 files changed, 39 insertions(+), 38 deletions(-)
> >
> > diff --git a/combine-diff.c b/combine-diff.c
> > index 04c4ae856..ec9d93044 100644
> > --- a/combine-diff.c
> > +++ b/combine-diff.c
> > @@ -1364,22 +1364,22 @@ static struct combine_diff_path *find_paths_multitree(
> >         struct diff_options *opt)
> >  {
> >         int i, nparent = parents->nr;
> > -       const unsigned char **parents_sha1;
> > +       const struct object_id **parents_oid;
> >         struct combine_diff_path paths_head;
> >         struct strbuf base;
> >
> > -       ALLOC_ARRAY(parents_sha1, nparent);
> > +       ALLOC_ARRAY(parents_oid, nparent);
> >         for (i = 0; i < nparent; i++)
> > -               parents_sha1[i] = parents->oid[i].hash;
> > +               parents_oid[i] = &parents->oid[i];
> 
> I have the impression that we could get away with one layer less
> of indirection. Previously we had a heap allocated array (*) of (char*),
> now we'd have a an array (*) of pointers(*) of the oid struct, that
> is a (char[]) essentially. Maybe I am just confused?

I don't think so. We always could have allocated the original as an
array of 20-byte chunks. It's syntactically less awkward in C when that
20-byte chunk is wrapped in a struct like object_id. But fundamentally
I think we don't need to be making a copy of the oid. We're pointing to
the existing copy in the "parents" array.

Or did you mean that diff_tree_paths() could now take an actual
array-of-struct rather than an array-of-pointer-to-struct? That would
drop the "parents_oid" array entirely. I think that's actually
orthogonal to this change (the same could have been done with the
original sha1 array), but would be a nice cleanup on top.

-Peff

Re: [PATCH 30/33] tree-diff: convert diff_tree_paths to struct object_id

[Jeff King]

On Wed, May 31, 2017 at 05:29:20PM -0400, Jeff King wrote:

> Or did you mean that diff_tree_paths() could now take an actual
> array-of-struct rather than an array-of-pointer-to-struct? That would
> drop the "parents_oid" array entirely. I think that's actually
> orthogonal to this change (the same could have been done with the
> original sha1 array), but would be a nice cleanup on top.

I took a quick look at this, but it doesn't work. This caller
(find_paths_multitree) would be happy to just pass the existing
"parents" array.

But if we change the interface to diff_tree_paths(), we also have to
change ll_diff_tree_paths() to match. And that function is called from
emit_path(), which really does assemble a list of pointers to
non-adjacent bits of memory. So it would still have to allocate, and
would now have to copy whole oids rather than just pointers.

But even worse, it seems to leave some entries as NULL. We'd need some
sentinel value (like the null sha1) to replace that.

So I think the code is already in its simplest form.

-Peff

Re: [PATCH 22/33] notes-merge: convert notes_merge* to struct object_id

[brian m. carlson]

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

On Tue, May 30, 2017 at 10:30:58AM -0700, Brandon Williams wrote:
> @@ -596,47 +596,47 @@ int notes_merge(struct notes_merge_options *o,
>  	/* Find merge bases */
>  	bases = get_merge_bases(local, remote);
>  	if (!bases) {
> -		base_sha1 = null_sha1;
> -		base_tree_sha1 = EMPTY_TREE_SHA1_BIN;
> +		base_oid = &null_oid;
> +		base_tree_oid = &empty_tree_oid;
>  		if (o->verbosity >= 4)
>  			printf("No merge base found; doing history-less merge\n");
>  	} else if (!bases->next) {
> -		base_sha1 = bases->item->object.oid.hash;
> -		base_tree_sha1 = bases->item->tree->object.oid.hash;
> +		base_oid = &bases->item->object.oid;
> +		base_tree_oid = &bases->item->tree->object.oid;
>  		if (o->verbosity >= 4)
>  			printf("One merge base found (%.7s)\n",
> -				sha1_to_hex(base_sha1));
> +			       oid_to_hex(base_oid));

I noticed you fixed up the indentation.  Thanks.

> diff --git a/notes-merge.h b/notes-merge.h
> index 0d890563b..eaa8e3b86 100644
> --- a/notes-merge.h
> +++ b/notes-merge.h
> @@ -33,15 +33,15 @@ void init_notes_merge_options(struct notes_merge_options *o);
>   *
>   * 1. The merge trivially results in an existing commit (e.g. fast-forward or
>   *    already-up-to-date). 'local_tree' is untouched, the SHA1 of the result
> - *    is written into 'result_sha1' and 0 is returned.
> + *    is written into 'result_oid' and 0 is returned.
>   * 2. The merge successfully completes, producing a merge commit. local_tree
>   *    contains the updated notes tree, the SHA1 of the resulting commit is
> - *    written into 'result_sha1', and 1 is returned.
> + *    written into 'result_oid', and 1 is returned.
>   * 3. The merge results in conflicts. This is similar to #2 in that the
>   *    partial merge result (i.e. merge result minus the unmerged entries)
>   *    are stored in 'local_tree', and the SHA1 or the resulting commit
>   *    (to be amended when the conflicts have been resolved) is written into
> - *    'result_sha1'. The unmerged entries are written into the
> + *    'result_oid'. The unmerged entries are written into the
>   *    .git/NOTES_MERGE_WORKTREE directory with conflict markers.
>   *    -1 is returned.
>   *

Did you want to change the comment to say "object ID" or "OID" instead
of "SHA1" like you did in an earlier patch?
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
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 00/33] object id conversion (grep and diff)

[brian m. carlson]

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

On Tue, May 30, 2017 at 10:30:36AM -0700, Brandon Williams wrote:
> A month or so ago I thought I would lend a hand to Brian and do a round of
> conversions from sha1 -> struct object_id.  Now that Brian's latest series has
> hit master I can finally send these patches out.
> 
> The first couple patches are from Brian which convert some of the notes logic
> to using 'struct object_id'.  The remaining patches are to convert the grep and
> diff machinery to using 'struct object_id'.
> 
> Brandon Williams (26):
>   grep: convert to struct object_id
>   diff: convert get_stat_data to struct object_id
>   diff: convert diff_index_show_file to struct object_id
>   diff: convert diff_addremove to struct object_id
>   diff: convert run_diff_files to struct object_id
>   diff: convert diff_change to struct object_id
>   diff: convert fill_filespec to struct object_id
>   diff: convert reuse_worktree_file to struct object_id
>   diff: finish conversion for prepare_temp_file to struct object_id
>   patch-ids: convert to struct object_id
>   diff: convert diff_flush_patch_id to struct object_id
>   combine-diff: convert diff_tree_combined to struct object_id
>   combine-diff: convert find_paths_* to struct object_id
>   tree-diff: convert diff_root_tree_sha1 to struct object_id
>   notes-merge: convert notes_merge* to struct object_id
>   notes-merge: convert merge_from_diffs to struct object_id
>   notes-merge: convert find_notes_merge_pair_ps to struct object_id
>   notes-merge: convert verify_notes_filepair to struct object_id
>   notes-merge: convert write_note_to_worktree to struct object_id
>   diff-tree: convert diff_tree_sha1 to struct object_id

I've wanted to convert this function for some time.  I'm glad you got
around to it.

Other than the issue I pointed out, the fact that I'm obviously not
qualified to review my own patches, and the issue that Stefan pointed
out with GIT_MAX_HEXSZ, this looks good to me.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
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 23/33] notes-merge: convert merge_from_diffs to struct object_id

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

>>         trace_printf("\tmerge_from_diffs(base = %.7s, local = %.7s, "
>> -              "remote = %.7s)\n", sha1_to_hex(base), sha1_to_hex(local),
>> -              sha1_to_hex(remote));
>> +              "remote = %.7s)\n", oid_to_hex(base), oid_to_hex(local),
>> +              oid_to_hex(remote));
>
> This part is interesting.
>
> 7 used to be the default abbreviation length for sha1s (see
> e6c587c733 (abbrev: auto size the default abbreviation, 2016-09-30)
> for details) and this may have been a left over from the conversion
> to variable length abbreviated hashes. This could be a reminder
> we want to fix these, too. (Though not in this series?).
>
> Roughly like
>
>     trace_printf("\tmerge_from_diffs(base = %s, local = %s, remote = %s)\n",
>         find_unique_abbrev(base.hash, DEFAULT_ABBREV),
>         find_unique_abbrev(local.hash, DEFAULT_ABBREV)
>         find_unique_abbrev(remote.hash, DEFAULT_ABBREV));

Sharp eyes ;-) As you said, this is definitely outside the scope of
this series.

Re: [PATCH 25/33] notes-merge: convert verify_notes_filepair to struct object_id

[Junio C Hamano]

Brandon Williams <bmwill@google.com> writes:

> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  notes-merge.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/notes-merge.c b/notes-merge.c
> index 55dbb3659..962e9b1bc 100644
> --- a/notes-merge.c
> +++ b/notes-merge.c
> @@ -22,21 +22,21 @@ void init_notes_merge_options(struct notes_merge_options *o)
>  	o->verbosity = NOTES_MERGE_VERBOSITY_DEFAULT;
>  }
>  
> -static int path_to_sha1(const char *path, unsigned char *sha1)
> +static int path_to_oid(const char *path, struct object_id *oid)
>  {
> -	char hex_sha1[40];
> +	char hex_oid[GIT_SHA1_HEXSZ];
>  	int i = 0;
> -	while (*path && i < 40) {
> +	while (*path && i < GIT_SHA1_HEXSZ) {
>  		if (*path != '/')
> -			hex_sha1[i++] = *path;
> +			hex_oid[i++] = *path;
>  		path++;
>  	}

It's no brainer to do s/GIT_SHA1_HEXSZ/GIT_MAX_HEXSZ/ for all of the
above, but ...

> -	if (*path || i != 40)
> +	if (*path || i != GIT_SHA1_HEXSZ)
>  		return -1;

... this one is tricky.  

What's in our envisioned future?  Are we expecing to see object
names, named with two or more hash functions, in a same repository?
If so, and one is 20 bytes and another one is 32 bytes, then this
should check 'i' against 40 and 64 and pass if 'i' is one of these
expected lengths?

> -	return get_sha1_hex(hex_sha1, sha1);
> +	return get_oid_hex(hex_oid, oid);
>  }

Re: [PATCH 08/33] grep: convert to struct object_id

[Junio C Hamano]

Brandon Williams <bmwill@google.com> writes:

> Convert the remaining parts of grep to use struct object_id.
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
>  builtin/grep.c | 22 +++++++++++-----------
>  cache.h        |  7 +++++++
>  grep.c         | 17 ++++++++---------
>  grep.h         |  2 +-
>  4 files changed, 27 insertions(+), 21 deletions(-)
> ...
> +static inline struct object_id *oiddup(const struct object_id *src)
> +{
> +	struct object_id *dst = xmalloc(sizeof(struct object_id));
> +	oidcpy(dst, src);
> +	return dst;
> +}
> +
>  static inline void hashclr(unsigned char *hash)
>  {
>  	memset(hash, 0, GIT_SHA1_RAWSZ);
> diff --git a/grep.c b/grep.c
> index 47cee4506..a240b4cdb 100644
> --- a/grep.c
> +++ b/grep.c
> @@ -1747,9 +1747,8 @@ void grep_source_init(struct grep_source *gs, enum grep_source_type type,
>  		 * If the identifier is non-NULL (in the submodule case) it
>  		 * will be a SHA1 that needs to be copied.
>  		 */
> -	case GREP_SOURCE_SHA1:
> -		gs->identifier = xmalloc(20);
> -		hashcpy(gs->identifier, identifier);
> +	case GREP_SOURCE_OID:
> +		gs->identifier = oiddup(identifier);
>  		break;
>  	case GREP_SOURCE_BUF:
>  		gs->identifier = NULL;

These hunks triggered my "Huh?" meter a bit, simply because I tend
to wonder not just "is this useful in many other places?" but also
"is it a good idea to encourage the use of this thing?"; these look
OK.

Thanks.

Re: [PATCH 22/33] notes-merge: convert notes_merge* to struct object_id

[Junio C Hamano]

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

>> diff --git a/notes-merge.h b/notes-merge.h
>> index 0d890563b..eaa8e3b86 100644
>> --- a/notes-merge.h
>> +++ b/notes-merge.h
>> @@ -33,15 +33,15 @@ void init_notes_merge_options(struct notes_merge_options *o);
>>   *
>>   * 1. The merge trivially results in an existing commit (e.g. fast-forward or
>>   *    already-up-to-date). 'local_tree' is untouched, the SHA1 of the result
>> - *    is written into 'result_sha1' and 0 is returned.
>> + *    is written into 'result_oid' and 0 is returned.
>>   * 2. The merge successfully completes, producing a merge commit. local_tree
>>   *    contains the updated notes tree, the SHA1 of the resulting commit is
>> - *    written into 'result_sha1', and 1 is returned.
>> + *    written into 'result_oid', and 1 is returned.
>>   * 3. The merge results in conflicts. This is similar to #2 in that the
>>   *    partial merge result (i.e. merge result minus the unmerged entries)
>>   *    are stored in 'local_tree', and the SHA1 or the resulting commit
>>   *    (to be amended when the conflicts have been resolved) is written into
>> - *    'result_sha1'. The unmerged entries are written into the
>> + *    'result_oid'. The unmerged entries are written into the
>>   *    .git/NOTES_MERGE_WORKTREE directory with conflict markers.
>>   *    -1 is returned.
>>   *
>
> Did you want to change the comment to say "object ID" or "OID" instead
> of "SHA1" like you did in an earlier patch?

Yes, that's a good suggestion.  Will queue a trivial squash on top.

Re: [PATCH 28/33] builtin/diff-tree: cleanup references to sha1

[Junio C Hamano]

Brandon Williams <bmwill@google.com> writes:

> @@ -132,15 +131,14 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
>  	 * reverse the order of the objects if the second one
>  	 * is marked UNINTERESTING.
>  	 */
> -	nr_sha1 = opt->pending.nr;
> -	switch (nr_sha1) {
> +	switch (opt->pending.nr) {

Unrelated to the topic of this patch, but I notice that the comment
whose tail shows in the pre-context of this hunk is incorrect.

  We expect "a..b" to expand to "^a b" but it is perfectly possible
  for revision range parser to produce "b ^a", which means the same
  thing.  We we get the latter, i.e. the second one is marked
  UNINTERESTING, we recover the original "a..b" by swapping the
  order.

is probably what we wanted to say.  These days, I suspect that we
may be able to do a lot better by using rev_cmdline_info thing,
which did not exist when this code was written.

Re: [PATCH 30/33] tree-diff: convert diff_tree_paths to struct object_id

[Junio C Hamano]

Brandon Williams <bmwill@google.com> writes:

> @@ -220,7 +221,7 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
>  	if (emitthis) {
>  		int keep;
>  		struct combine_diff_path *pprev = p;
> -		p = path_appendnew(p, nparent, base, path, pathlen, mode, sha1);
> +		p = path_appendnew(p, nparent, base, path, pathlen, mode, oid ? oid->hash : NULL);

This is a correct conversion, but it shows that "struct oid" that
has "hash" sometimes makes things a bit cumbersome.  Perhaps
path_appendnew() and friends need to learn to take a pointer to
"struct oid" next.

Re: [PATCH 31/33] tree-diff: convert path_appendnew to object_id

[Junio C Hamano]

Brandon Williams <bmwill@google.com> writes:

> @@ -221,7 +221,7 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
>  	if (emitthis) {
>  		int keep;
>  		struct combine_diff_path *pprev = p;
> -		p = path_appendnew(p, nparent, base, path, pathlen, mode, oid ? oid->hash : NULL);
> +		p = path_appendnew(p, nparent, base, path, pathlen, mode, oid);
>  
>  		for (i = 0; i < nparent; ++i) {
>  			/*

Ahh, it happens here ;-)

And of course, the order of these two patches cannot be swapped, so
this is the best organization of the series.

Re: [PATCH 00/33] object id conversion (grep and diff)

[Junio C Hamano]

Brandon Williams <bmwill@google.com> writes:

> A month or so ago I thought I would lend a hand to Brian and do a round of
> conversions from sha1 -> struct object_id.  Now that Brian's latest series has
> hit master I can finally send these patches out.
>
> The first couple patches are from Brian which convert some of the notes logic
> to using 'struct object_id'.  The remaining patches are to convert the grep and
> diff machinery to using 'struct object_id'.

Nicely done for all of them.  Thanks.  Will queue (with tweaks
mentioned in the comments).

Re: [PATCH 20/33] combine-diff: convert find_paths_* to struct object_id

[Junio C Hamano]

Stefan Beller <sbeller@google.com> writes:

> On Tue, May 30, 2017 at 10:30 AM, Brandon Williams <bmwill@google.com> wrote:
>> Convert find_paths_generic and find_paths_multitree to use struct
>> object_id.
>>
>>
>
> double empty line?

Is it because

	sed -e 's/^Change-ID:.*//'

insead of

	sed -e '/^Change-ID:.*/d'

is used somewhere in a commit-message-rewriting-machinery?

In any case, it doesn't bother Git at all---stripspace squishes
duplicated empty lines into one while making other whitespace
clean-ups like trailing SP removal and trailing blank line removal.

Re: [PATCH 00/33] object id conversion (grep and diff)

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Brandon Williams <bmwill@google.com> writes:
>
>> A month or so ago I thought I would lend a hand to Brian and do a round of
>> conversions from sha1 -> struct object_id.  Now that Brian's latest series has
>> hit master I can finally send these patches out.
>>
>> The first couple patches are from Brian which convert some of the notes logic
>> to using 'struct object_id'.  The remaining patches are to convert the grep and
>> diff machinery to using 'struct object_id'.
>
> Nicely done for all of them.  Thanks.  Will queue (with tweaks
> mentioned in the comments).

Oops.  I won't be able to queue this for now as it heavily conflicts
with blame-lib topic.  The resolution should be trivial, mechanical
and boring, but takes time that I do not have today.

Re: [PATCH 00/33] object id conversion (grep and diff)

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Brandon Williams <bmwill@google.com> writes:
>>
>>> A month or so ago I thought I would lend a hand to Brian and do a round of
>>> conversions from sha1 -> struct object_id.  Now that Brian's latest series has
>>> hit master I can finally send these patches out.
>>>
>>> The first couple patches are from Brian which convert some of the notes logic
>>> to using 'struct object_id'.  The remaining patches are to convert the grep and
>>> diff machinery to using 'struct object_id'.
>>
>> Nicely done for all of them.  Thanks.  Will queue (with tweaks
>> mentioned in the comments).
>
> Oops.  I won't be able to queue this for now as it heavily conflicts
> with blame-lib topic.  The resolution should be trivial, mechanical
> and boring, but takes time that I do not have today.

I lied.  This also conflicts somewhat with Peff's diff-blob topic.
I think I resolved them correctly (there needs evil merges applied
to two files when merging this topic), and hopefully can push out
the result by the end of the day.

Thanks.

Re: [PATCH 00/33] object id conversion (grep and diff)

[Brandon Williams]

On 06/02, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Junio C Hamano <gitster@pobox.com> writes:
> >
> >> Brandon Williams <bmwill@google.com> writes:
> >>
> >>> A month or so ago I thought I would lend a hand to Brian and do a round of
> >>> conversions from sha1 -> struct object_id.  Now that Brian's latest series has
> >>> hit master I can finally send these patches out.
> >>>
> >>> The first couple patches are from Brian which convert some of the notes logic
> >>> to using 'struct object_id'.  The remaining patches are to convert the grep and
> >>> diff machinery to using 'struct object_id'.
> >>
> >> Nicely done for all of them.  Thanks.  Will queue (with tweaks
> >> mentioned in the comments).
> >
> > Oops.  I won't be able to queue this for now as it heavily conflicts
> > with blame-lib topic.  The resolution should be trivial, mechanical
> > and boring, but takes time that I do not have today.
> 
> I lied.  This also conflicts somewhat with Peff's diff-blob topic.
> I think I resolved them correctly (there needs evil merges applied
> to two files when merging this topic), and hopefully can push out
> the result by the end of the day.
> 
> Thanks.

If it ends up being too much of a headache for you to deal with, let me
know and I can rebase on top of those series.  That way you don't have to
deal with the conflict resolutions.  Just let me know what you'd like me
to do.

-- 
Brandon Williams

Re: [PATCH 22/33] notes-merge: convert notes_merge* to struct object_id

[Brandon Williams]

On 05/31, brian m. carlson wrote:
> On Tue, May 30, 2017 at 10:30:58AM -0700, Brandon Williams wrote:
> > @@ -596,47 +596,47 @@ int notes_merge(struct notes_merge_options *o,
> >  	/* Find merge bases */
> >  	bases = get_merge_bases(local, remote);
> >  	if (!bases) {
> > -		base_sha1 = null_sha1;
> > -		base_tree_sha1 = EMPTY_TREE_SHA1_BIN;
> > +		base_oid = &null_oid;
> > +		base_tree_oid = &empty_tree_oid;
> >  		if (o->verbosity >= 4)
> >  			printf("No merge base found; doing history-less merge\n");
> >  	} else if (!bases->next) {
> > -		base_sha1 = bases->item->object.oid.hash;
> > -		base_tree_sha1 = bases->item->tree->object.oid.hash;
> > +		base_oid = &bases->item->object.oid;
> > +		base_tree_oid = &bases->item->tree->object.oid;
> >  		if (o->verbosity >= 4)
> >  			printf("One merge base found (%.7s)\n",
> > -				sha1_to_hex(base_sha1));
> > +			       oid_to_hex(base_oid));
> 
> I noticed you fixed up the indentation.  Thanks.
> 
> > diff --git a/notes-merge.h b/notes-merge.h
> > index 0d890563b..eaa8e3b86 100644
> > --- a/notes-merge.h
> > +++ b/notes-merge.h
> > @@ -33,15 +33,15 @@ void init_notes_merge_options(struct notes_merge_options *o);
> >   *
> >   * 1. The merge trivially results in an existing commit (e.g. fast-forward or
> >   *    already-up-to-date). 'local_tree' is untouched, the SHA1 of the result
> > - *    is written into 'result_sha1' and 0 is returned.
> > + *    is written into 'result_oid' and 0 is returned.
> >   * 2. The merge successfully completes, producing a merge commit. local_tree
> >   *    contains the updated notes tree, the SHA1 of the resulting commit is
> > - *    written into 'result_sha1', and 1 is returned.
> > + *    written into 'result_oid', and 1 is returned.
> >   * 3. The merge results in conflicts. This is similar to #2 in that the
> >   *    partial merge result (i.e. merge result minus the unmerged entries)
> >   *    are stored in 'local_tree', and the SHA1 or the resulting commit
> >   *    (to be amended when the conflicts have been resolved) is written into
> > - *    'result_sha1'. The unmerged entries are written into the
> > + *    'result_oid'. The unmerged entries are written into the
> >   *    .git/NOTES_MERGE_WORKTREE directory with conflict markers.
> >   *    -1 is returned.
> >   *
> 
> Did you want to change the comment to say "object ID" or "OID" instead
> of "SHA1" like you did in an earlier patch?

Yep I can do that here and in the comment below this one.

> -- 
> brian m. carlson / brian with sandals: Houston, Texas, US
> https://www.crustytoothpaste.net/~bmc | My opinion only
> OpenPGP: https://keybase.io/bk2204



-- 
Brandon Williams

Re: [PATCH 25/33] notes-merge: convert verify_notes_filepair to struct object_id

[Brandon Williams]

On 06/02, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
> 
> > Signed-off-by: Brandon Williams <bmwill@google.com>
> > ---
> >  notes-merge.c | 20 ++++++++++----------
> >  1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/notes-merge.c b/notes-merge.c
> > index 55dbb3659..962e9b1bc 100644
> > --- a/notes-merge.c
> > +++ b/notes-merge.c
> > @@ -22,21 +22,21 @@ void init_notes_merge_options(struct notes_merge_options *o)
> >  	o->verbosity = NOTES_MERGE_VERBOSITY_DEFAULT;
> >  }
> >  
> > -static int path_to_sha1(const char *path, unsigned char *sha1)
> > +static int path_to_oid(const char *path, struct object_id *oid)
> >  {
> > -	char hex_sha1[40];
> > +	char hex_oid[GIT_SHA1_HEXSZ];
> >  	int i = 0;
> > -	while (*path && i < 40) {
> > +	while (*path && i < GIT_SHA1_HEXSZ) {
> >  		if (*path != '/')
> > -			hex_sha1[i++] = *path;
> > +			hex_oid[i++] = *path;
> >  		path++;
> >  	}
> 
> It's no brainer to do s/GIT_SHA1_HEXSZ/GIT_MAX_HEXSZ/ for all of the
> above, but ...

I'll fix this.

> 
> > -	if (*path || i != 40)
> > +	if (*path || i != GIT_SHA1_HEXSZ)
> >  		return -1;
> 
> ... this one is tricky.  
> 
> What's in our envisioned future?  Are we expecing to see object
> names, named with two or more hash functions, in a same repository?
> If so, and one is 20 bytes and another one is 32 bytes, then this
> should check 'i' against 40 and 64 and pass if 'i' is one of these
> expected lengths?

That's a good question, and at this point in time do we have an
envisioned future?  From some of our conversations I seem to remember
that we don't want a single repository to have objects based on two
different hash functions, but rather some translation layer to convert
between two hashing functions (for compatibility with other
non-converted repos).  Though nothing has been settled upon yet so I
don't know what the future would look like (and the best way to prepare
for it).

> 
> > -	return get_sha1_hex(hex_sha1, sha1);
> > +	return get_oid_hex(hex_oid, oid);
> >  }

-- 
Brandon Williams

Re: [PATCH 00/33] object id conversion (grep and diff)

[Brandon Williams]

On 05/31, brian m. carlson wrote:
> On Tue, May 30, 2017 at 10:30:36AM -0700, Brandon Williams wrote:
> > A month or so ago I thought I would lend a hand to Brian and do a round of
> > conversions from sha1 -> struct object_id.  Now that Brian's latest series has
> > hit master I can finally send these patches out.
> > 
> > The first couple patches are from Brian which convert some of the notes logic
> > to using 'struct object_id'.  The remaining patches are to convert the grep and
> > diff machinery to using 'struct object_id'.
> > 
> > Brandon Williams (26):
> >   grep: convert to struct object_id
> >   diff: convert get_stat_data to struct object_id
> >   diff: convert diff_index_show_file to struct object_id
> >   diff: convert diff_addremove to struct object_id
> >   diff: convert run_diff_files to struct object_id
> >   diff: convert diff_change to struct object_id
> >   diff: convert fill_filespec to struct object_id
> >   diff: convert reuse_worktree_file to struct object_id
> >   diff: finish conversion for prepare_temp_file to struct object_id
> >   patch-ids: convert to struct object_id
> >   diff: convert diff_flush_patch_id to struct object_id
> >   combine-diff: convert diff_tree_combined to struct object_id
> >   combine-diff: convert find_paths_* to struct object_id
> >   tree-diff: convert diff_root_tree_sha1 to struct object_id
> >   notes-merge: convert notes_merge* to struct object_id
> >   notes-merge: convert merge_from_diffs to struct object_id
> >   notes-merge: convert find_notes_merge_pair_ps to struct object_id
> >   notes-merge: convert verify_notes_filepair to struct object_id
> >   notes-merge: convert write_note_to_worktree to struct object_id
> >   diff-tree: convert diff_tree_sha1 to struct object_id
> 
> I've wanted to convert this function for some time.  I'm glad you got
> around to it.

Of course, glad to help!  After working on this stuff I realized how serial this sort
of conversion can be since its easy to have a function span many many
parts of the code base.

> 
> Other than the issue I pointed out, the fact that I'm obviously not
> qualified to review my own patches, and the issue that Stefan pointed
> out with GIT_MAX_HEXSZ, this looks good to me.
> -- 
> brian m. carlson / brian with sandals: Houston, Texas, US
> https://www.crustytoothpaste.net/~bmc | My opinion only
> OpenPGP: https://keybase.io/bk2204



-- 
Brandon Williams

Re: [PATCH 00/33] object id conversion (grep and diff)

[Junio C Hamano]

Brandon Williams <bmwill@google.com> writes:

> On 06/02, Junio C Hamano wrote:
>> 
>> I lied.  This also conflicts somewhat with Peff's diff-blob topic.
>> I think I resolved them correctly (there needs evil merges applied
>> to two files when merging this topic), and hopefully can push out
>> the result by the end of the day.
>> 
>> Thanks.
>
> If it ends up being too much of a headache for you to deal with, let me
> know and I can rebase on top of those series.  That way you don't have to
> deal with the conflict resolutions.  Just let me know what you'd like me
> to do.

Sorry, I forgot to push the result out, even though I double checked
the conflict resolution I did last night.  Now it is in the public
repository.  I have one squash queued at the right place to update
SHA1s to OIDs in the comment Brian pointed out.

If you ever need to rebase this on top of future 'master' that
already has js/blame-lib topic, fetching from me and checking
the evil merge I did may turn out to be helpful:

 $ git fetch git://github.com/gitster/git refs/merge-fix/bw/object-id
 $ git show FETCH_HEAD

but I can take patches based on the same old 'master'; as long as
the evil merge above is correct, that would probably be preferrable,
as it makes it easier to compare the two iterations of your series.

Repeating some backstory of "merge-fix" might be beneficial, if a
reader is interested.  Otherwise the remainder of this message can
safely be skipped.

After a topic (i.e. js/blame-lib) moves a lot of code around (i.e. a
bulk of what used to be in builtin/blame.c is now in blame.c and
some in diff.c), merging a topic that touches places in the code
that was moved in-place (i.e. bw/object-id, which updates the code
in builtin/blame.c) will leave a conflict that looks like:

    <<<<<<< HEAD
    ... very little that is left after moving
    ... bunch of code out of this file
    ||||||| common
    ... a lot of
    ... stuff before
    ... your change from SHA1 to OID
    ... is here
    =======
    ... a lot of
    ... stuff after
    ... your change from SHA1 to OID
    ... is here
    >>>>>>> theirs

As far as builtin/blame.c is concerned, the resolution for this
set of conflicts is just to take the HEAD version, ignoring all of
your SHA1-to-OID changes.  Once it is resolved, "rerere" can help
us remember this resolution to builtin/blame.c

But the ignored changes need to go somewhere else.

What the user who is doing a merge does at this point is to compare
what is between ||||||| and ======= (i.e. common ancestor's version)
with what is between ======= and >>>>>>> (i.e. their version) to
figure out what the branch being merged did.  And the user needs to
know where the common code went in the version in HEAD.

 $ git log [--no-merges] -p MERGE_HEAD.. -- builtin/blame.c

is helpful to locate the commit that lost the common lines from the
file.  And "git show" on it will tell us that they mostly went to
blame.c while some migrating to diff.c; we found out what you did by
comparing "common" and "theirs" in the previous step and we apply
these changes to these "new" places.

And that is the diff you see in refs/merge-fix/bw/object-id.  The
script I use to re-build 'pu' every day (probably I use it three
times a day on average) knows about that ref.  The script starts
from the tip of 'master', and for each topic, (1) run "git merge"
into HEAD, (2) take resolution recorded by "rerere" and (3) if
merge/fix/$topic exists, cherry-pick it on top to squash into the
merge made in (2).

Once I have taught my rerere database and refs/merge-fix/ about this
merge, it is not too big a deal to redo the merge to adjust to an
updated 'master' or a new interation of your series because of the
above mechanism.  And that is why I say it is OK for an updated series
to be based on the same old 'master'.

Thanks.

Re: [PATCH 25/33] notes-merge: convert verify_notes_filepair to struct object_id

[Junio C Hamano]

Brandon Williams <bmwill@google.com> writes:

> On 06/02, Junio C Hamano wrote:
>> 
>> > -static int path_to_sha1(const char *path, unsigned char *sha1)
>> > +static int path_to_oid(const char *path, struct object_id *oid)
>> >  {
>> > -	char hex_sha1[40];
>> > +	char hex_oid[GIT_SHA1_HEXSZ];
>> >  	int i = 0;
>> > -	while (*path && i < 40) {
>> > +	while (*path && i < GIT_SHA1_HEXSZ) {
>> >  		if (*path != '/')
>> > -			hex_sha1[i++] = *path;
>> > +			hex_oid[i++] = *path;
>> >  		path++;
>> >  	}
>> 
>> It's no brainer to do s/GIT_SHA1_HEXSZ/GIT_MAX_HEXSZ/ for all of the
>> above, but ...
>
> I'll fix this.
>
>> 
>> > -	if (*path || i != 40)
>> > +	if (*path || i != GIT_SHA1_HEXSZ)
>> >  		return -1;
>> 
>> ... this one is tricky.  
>> 
>> What's in our envisioned future?  Are we expecing to see object
>> names, named with two or more hash functions, in a same repository?
>> If so, and one is 20 bytes and another one is 32 bytes, then this
>> should check 'i' against 40 and 64 and pass if 'i' is one of these
>> expected lengths?
>
> That's a good question, and at this point in time do we have an
> envisioned future?  From some of our conversations I seem to remember
> that we don't want a single repository to have objects based on two
> different hash functions, but rather some translation layer to convert
> between two hashing functions (for compatibility with other
> non-converted repos).  Though nothing has been settled upon yet so I
> don't know what the future would look like (and the best way to prepare
> for it).

I do not think we know precisely what we want yet.  The "MAX" in the
allocation size is an indication that we are allocating for the
largest variant possible in the version of Git being compiled, which
is a hint that we anticipate that we may have multiple variants with
different sizes.  And that is where my "against 40 and 64 ... one of
these expected lengths" come from.  

But there is no such set of macros that define acceptable/expected
lengths and that tells me that nobody among who defined, reviewed
and accepted the MAX macro has thought this part through yet.

I see Jonathan started talking about experiments with "different
hash" elsewhere; I am reading his message as assuming one hash at a
time, and the repository tells us what the hash is.  And I think
that is one plausible design.

If we were to go that route, then

    - MAX will be the maximum among hashes we can choose at the
      beginning of a process (perhaps by consulting the repository
      extension).

    - In addition to GIT_SHA1_HEXSZ, GIT_SHA2_HEXSZ, and friends, we
      need a variable that records the length of the hash chosen for
      the process at the startup, GIT_OIDHASH_HEXSZ.  That would
      become one of the fields in your "struct repository",

        #define GIT_OIDHASH_HEXSZ (the_repository.oidhash_hexsz)

      among other things like what the oid for an empty blob is.

I'd say in the meantime we can do something like the attached and
use it here, perhaps?

 cache.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/cache.h b/cache.h
index 53999418d3..cafb13db9f 100644
--- a/cache.h
+++ b/cache.h
@@ -70,6 +70,10 @@ unsigned long git_deflate_bound(git_zstream *, unsigned long);
 #define GIT_MAX_RAWSZ GIT_SHA1_RAWSZ
 #define GIT_MAX_HEXSZ GIT_SHA1_HEXSZ
 
+/* The length for the object name hash */
+#define GIT_OIDHASH_RAWSZ GIT_SHA1_RAWSZ
+#define GIT_OIDHASH_HEXSZ GIT_SHA1_HEXSZ
+
 struct object_id {
 	unsigned char hash[GIT_MAX_RAWSZ];
 };

Re: [PATCH 00/33] object id conversion (grep and diff)

[Brandon Williams]

On 06/03, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
> 
> > On 06/02, Junio C Hamano wrote:
> >> 
> >> I lied.  This also conflicts somewhat with Peff's diff-blob topic.
> >> I think I resolved them correctly (there needs evil merges applied
> >> to two files when merging this topic), and hopefully can push out
> >> the result by the end of the day.
> >> 
> >> Thanks.
> >
> > If it ends up being too much of a headache for you to deal with, let me
> > know and I can rebase on top of those series.  That way you don't have to
> > deal with the conflict resolutions.  Just let me know what you'd like me
> > to do.
> 
> Sorry, I forgot to push the result out, even though I double checked
> the conflict resolution I did last night.  Now it is in the public
> repository.  I have one squash queued at the right place to update
> SHA1s to OIDs in the comment Brian pointed out.
> 

What you have at 'bw/object-id' matches the changes I made locally
(changing SHA1 to OID) and looks good to me!

> If you ever need to rebase this on top of future 'master' that
> already has js/blame-lib topic, fetching from me and checking
> the evil merge I did may turn out to be helpful:
> 
>  $ git fetch git://github.com/gitster/git refs/merge-fix/bw/object-id
>  $ git show FETCH_HEAD
> 
> but I can take patches based on the same old 'master'; as long as
> the evil merge above is correct, that would probably be preferrable,
> as it makes it easier to compare the two iterations of your series.

Sounds like basing it on the original 'master' would work easiest for
you, so I'll continue to do that :)

> 
> Repeating some backstory of "merge-fix" might be beneficial, if a
> reader is interested.  Otherwise the remainder of this message can
> safely be skipped.
> 
> After a topic (i.e. js/blame-lib) moves a lot of code around (i.e. a
> bulk of what used to be in builtin/blame.c is now in blame.c and
> some in diff.c), merging a topic that touches places in the code
> that was moved in-place (i.e. bw/object-id, which updates the code
> in builtin/blame.c) will leave a conflict that looks like:
> 
>     <<<<<<< HEAD
>     ... very little that is left after moving
>     ... bunch of code out of this file
>     ||||||| common
>     ... a lot of
>     ... stuff before
>     ... your change from SHA1 to OID
>     ... is here
>     =======
>     ... a lot of
>     ... stuff after
>     ... your change from SHA1 to OID
>     ... is here
>     >>>>>>> theirs
> 
> As far as builtin/blame.c is concerned, the resolution for this
> set of conflicts is just to take the HEAD version, ignoring all of
> your SHA1-to-OID changes.  Once it is resolved, "rerere" can help
> us remember this resolution to builtin/blame.c
> 
> But the ignored changes need to go somewhere else.
> 
> What the user who is doing a merge does at this point is to compare
> what is between ||||||| and ======= (i.e. common ancestor's version)
> with what is between ======= and >>>>>>> (i.e. their version) to
> figure out what the branch being merged did.  And the user needs to
> know where the common code went in the version in HEAD.
> 
>  $ git log [--no-merges] -p MERGE_HEAD.. -- builtin/blame.c
> 
> is helpful to locate the commit that lost the common lines from the
> file.  And "git show" on it will tell us that they mostly went to
> blame.c while some migrating to diff.c; we found out what you did by
> comparing "common" and "theirs" in the previous step and we apply
> these changes to these "new" places.
> 
> And that is the diff you see in refs/merge-fix/bw/object-id.  The
> script I use to re-build 'pu' every day (probably I use it three
> times a day on average) knows about that ref.  The script starts
> from the tip of 'master', and for each topic, (1) run "git merge"
> into HEAD, (2) take resolution recorded by "rerere" and (3) if
> merge/fix/$topic exists, cherry-pick it on top to squash into the
> merge made in (2).
> 
> Once I have taught my rerere database and refs/merge-fix/ about this
> merge, it is not too big a deal to redo the merge to adjust to an
> updated 'master' or a new interation of your series because of the
> above mechanism.  And that is why I say it is OK for an updated series
> to be based on the same old 'master'.
> 
> Thanks.
> 
> 

-- 
Brandon Williams

Re: [PATCH 30/33] tree-diff: convert diff_tree_paths to struct object_id

[René Scharfe]

Am 30.05.2017 um 19:31 schrieb Brandon Williams:
> @@ -273,21 +274,20 @@ static struct combine_diff_path *emit_path(struct combine_diff_path *p,
>   	}
>   
>   	if (recurse) {
> -		const unsigned char **parents_sha1;
> +		const struct object_id **parents_oid;
>   
> -		FAST_ARRAY_ALLOC(parents_sha1, nparent);
> +		FAST_ARRAY_ALLOC(parents_oid, nparent);
>   		for (i = 0; i < nparent; ++i) {
>   			/* same rule as in emitthis */
>   			int tpi_valid = tp && !(tp[i].entry.mode & S_IFXMIN_NEQ);
>   
> -			parents_sha1[i] = tpi_valid ? tp[i].entry.oid->hash
> -						    : NULL;
> +			parents_oid[i] = tpi_valid ? tp[i].entry.oid : NULL;
>   		}

So elements of parents_oid can be NULL...

>   
>   		strbuf_add(base, path, pathlen);
>   		strbuf_addch(base, '/');
> -		p = ll_diff_tree_paths(p, sha1, parents_sha1, nparent, base, opt);
> -		FAST_ARRAY_FREE(parents_sha1, nparent);
> +		p = ll_diff_tree_paths(p, oid, parents_oid, nparent, base, opt);

... and we pass that array to ll_diff_tree_paths()...

> +		FAST_ARRAY_FREE(parents_oid, nparent);
>   	}
>   
>   	strbuf_setlen(base, old_baselen);
> @@ -312,7 +312,7 @@ static void skip_uninteresting(struct tree_desc *t, struct strbuf *base,
>   
>   
>   /*
> - * generate paths for combined diff D(sha1,parents_sha1[])
> + * generate paths for combined diff D(sha1,parents_oid[])
>    *
>    * Resulting paths are appended to combine_diff_path linked list, and also, are
>    * emitted on the go via opt->pathchange() callback, so it is possible to
> @@ -404,8 +404,8 @@ static inline void update_tp_entries(struct tree_desc *tp, int nparent)
>   }
>   
>   static struct combine_diff_path *ll_diff_tree_paths(
> -	struct combine_diff_path *p, const unsigned char *sha1,
> -	const unsigned char **parents_sha1, int nparent,
> +	struct combine_diff_path *p, const struct object_id *oid,
> +	const struct object_id **parents_oid, int nparent,
>   	struct strbuf *base, struct diff_options *opt)
>   {
>   	struct tree_desc t, *tp;
> @@ -422,8 +422,8 @@ static struct combine_diff_path *ll_diff_tree_paths(
>   	 *   diff_tree_oid(parent, commit) )
>   	 */
>   	for (i = 0; i < nparent; ++i)
> -		tptree[i] = fill_tree_descriptor(&tp[i], parents_sha1[i]);
> -	ttree = fill_tree_descriptor(&t, sha1);
> +		tptree[i] = fill_tree_descriptor(&tp[i], parents_oid[i]->hash);

... and here we are in that function, dereferencing a pointer that may
be NULL.

> +	ttree = fill_tree_descriptor(&t, oid->hash);

oid here can also be NULL, but I think that's not as easy to see.

-- >8 --
Subject: [PATCH] tree-diff: don't access hash of NULL object_id pointer

The object_id pointers can be NULL for invalid entries.  Don't try to
dereference them and pass NULL along to fill_tree_descriptor() instead,
which handles them just fine.

Found with Clang's UBSan.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
fill_tree_descriptor() can easily be converted to object_id, by the
way, which would get us rid of the extra check introduced here, but
this patch is meant as a minimal fix.

 tree-diff.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tree-diff.c b/tree-diff.c
index bd6d65a409..2357f72899 100644
--- a/tree-diff.c
+++ b/tree-diff.c
@@ -421,8 +421,9 @@ static struct combine_diff_path *ll_diff_tree_paths(
 	 *   diff_tree_oid(parent, commit) )
 	 */
 	for (i = 0; i < nparent; ++i)
-		tptree[i] = fill_tree_descriptor(&tp[i], parents_oid[i]->hash);
-	ttree = fill_tree_descriptor(&t, oid->hash);
+		tptree[i] = fill_tree_descriptor(&tp[i],
+				parents_oid[i] ? parents_oid[i]->hash : NULL);
+	ttree = fill_tree_descriptor(&t, oid ? oid->hash : NULL);
 
 	/* Enable recursion indefinitely */
 	opt->pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);
-- 
2.13.3

Re: [PATCH 30/33] tree-diff: convert diff_tree_paths to struct object_id

[brian m. carlson]

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

On Sat, Jul 15, 2017 at 07:18:51PM +0200, René Scharfe wrote:
> -- >8 --
> Subject: [PATCH] tree-diff: don't access hash of NULL object_id pointer
> 
> The object_id pointers can be NULL for invalid entries.  Don't try to
> dereference them and pass NULL along to fill_tree_descriptor() instead,
> which handles them just fine.
> 
> Found with Clang's UBSan.
> 
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
> fill_tree_descriptor() can easily be converted to object_id, by the
> way, which would get us rid of the extra check introduced here, but
> this patch is meant as a minimal fix.
> 
>  tree-diff.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tree-diff.c b/tree-diff.c
> index bd6d65a409..2357f72899 100644
> --- a/tree-diff.c
> +++ b/tree-diff.c
> @@ -421,8 +421,9 @@ static struct combine_diff_path *ll_diff_tree_paths(
>  	 *   diff_tree_oid(parent, commit) )
>  	 */
>  	for (i = 0; i < nparent; ++i)
> -		tptree[i] = fill_tree_descriptor(&tp[i], parents_oid[i]->hash);
> -	ttree = fill_tree_descriptor(&t, oid->hash);
> +		tptree[i] = fill_tree_descriptor(&tp[i],
> +				parents_oid[i] ? parents_oid[i]->hash : NULL);
> +	ttree = fill_tree_descriptor(&t, oid ? oid->hash : NULL);

Good catch.  This seems obviously correct.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
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 04/33] notes: make get_note return pointer to struct object_id

[René Scharfe]

Am 30.05.2017 um 19:30 schrieb Brandon Williams:
> @@ -392,7 +392,7 @@ static int add(int argc, const char **argv, const char *prefix)
>   	const char *object_ref;
>   	struct notes_tree *t;
>   	unsigned char object[20], new_note[20];
> -	const unsigned char *note;
> +	const struct object_id *note;
>   	struct note_data d = { 0, 0, NULL, STRBUF_INIT };
>   	struct option options[] = {
>   		{ OPTION_CALLBACK, 'm', "message", &d, N_("message"),

In between here, note can be set to NULL...

> @@ -453,7 +453,7 @@ static int add(int argc, const char **argv, const char *prefix)
>   			sha1_to_hex(object));
>   	}
>   
> -	prepare_note_data(object, &d, note);
> +	prepare_note_data(object, &d, note->hash);

... which we then dereference here.

> @@ -598,13 +598,13 @@ static int append_edit(int argc, const char **argv, const char *prefix)
>   	t = init_notes_check(argv[0], NOTES_INIT_WRITABLE);
>   	note = get_note(t, object);
>   
> -	prepare_note_data(object, &d, edit ? note : NULL);
> +	prepare_note_data(object, &d, edit && note ? note->hash : NULL);

Here a NULL check was added; we need a similar one above as well.

-- >8 --
Subject: [PATCH] notes: don't access hash of NULL object_id pointer

Check if note is NULL, as we already do for different purposes a few
lines above, and pass a NULL pointer to prepare_note_data() in that
case instead of trying to access the hash member.

Found with Clang's UBSan.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
The third parameter of prepare_note_data() could easily be turned into
an object_id pointer (and it should), but this patch is meant to be a
minimal fix.

 builtin/notes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 77573cf1ea..4303848e04 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -456,7 +456,7 @@ static int add(int argc, const char **argv, const char *prefix)
 			oid_to_hex(&object));
 	}
 
-	prepare_note_data(&object, &d, note->hash);
+	prepare_note_data(&object, &d, note ? note->hash : NULL);
 	if (d.buf.len || allow_empty) {
 		write_note_data(&d, new_note.hash);
 		if (add_note(t, &object, &new_note, combine_notes_overwrite))
-- 
2.13.3

Re: [PATCH 04/33] notes: make get_note return pointer to struct object_id

[Brandon Williams]

On 07/15, René Scharfe wrote:
> Am 30.05.2017 um 19:30 schrieb Brandon Williams:
> > @@ -392,7 +392,7 @@ static int add(int argc, const char **argv, const char *prefix)
> >   	const char *object_ref;
> >   	struct notes_tree *t;
> >   	unsigned char object[20], new_note[20];
> > -	const unsigned char *note;
> > +	const struct object_id *note;
> >   	struct note_data d = { 0, 0, NULL, STRBUF_INIT };
> >   	struct option options[] = {
> >   		{ OPTION_CALLBACK, 'm', "message", &d, N_("message"),
> 
> In between here, note can be set to NULL...
> 
> > @@ -453,7 +453,7 @@ static int add(int argc, const char **argv, const char *prefix)
> >   			sha1_to_hex(object));
> >   	}
> >   
> > -	prepare_note_data(object, &d, note);
> > +	prepare_note_data(object, &d, note->hash);
> 
> ... which we then dereference here.
> 
> > @@ -598,13 +598,13 @@ static int append_edit(int argc, const char **argv, const char *prefix)
> >   	t = init_notes_check(argv[0], NOTES_INIT_WRITABLE);
> >   	note = get_note(t, object);
> >   
> > -	prepare_note_data(object, &d, edit ? note : NULL);
> > +	prepare_note_data(object, &d, edit && note ? note->hash : NULL);
> 
> Here a NULL check was added; we need a similar one above as well.
> 
> -- >8 --
> Subject: [PATCH] notes: don't access hash of NULL object_id pointer
> 
> Check if note is NULL, as we already do for different purposes a few
> lines above, and pass a NULL pointer to prepare_note_data() in that
> case instead of trying to access the hash member.

Looks good, thanks for catching this!

> 
> Found with Clang's UBSan.
> 
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
> The third parameter of prepare_note_data() could easily be turned into
> an object_id pointer (and it should), but this patch is meant to be a
> minimal fix.
> 
>  builtin/notes.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/builtin/notes.c b/builtin/notes.c
> index 77573cf1ea..4303848e04 100644
> --- a/builtin/notes.c
> +++ b/builtin/notes.c
> @@ -456,7 +456,7 @@ static int add(int argc, const char **argv, const char *prefix)
>  			oid_to_hex(&object));
>  	}
>  
> -	prepare_note_data(&object, &d, note->hash);
> +	prepare_note_data(&object, &d, note ? note->hash : NULL);
>  	if (d.buf.len || allow_empty) {
>  		write_note_data(&d, new_note.hash);
>  		if (add_note(t, &object, &new_note, combine_notes_overwrite))
> -- 
> 2.13.3

-- 
Brandon Williams