All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/19] object_id part 6
@ 2017-02-18  0:06 brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 01/19] builtin/commit: convert to struct object_id brian m. carlson
                   ` (18 more replies)
  0 siblings, 19 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

This is another series in the continuing conversion to struct object_id.

This series converts more of the builtin directory and some of the refs
code to use struct object_id. Additionally, it implements an
nth_packed_object_oid function which provides a struct object_id version
of the nth_packed_object function, and a parse_oid_hex function that
makes parsing easier.

The patch to use parse_oid_hex in the refs code has been split out into
its own patch, just because I'm wary of that code and potentially
breaking things, and I want it to be easy to revert in case things go
wrong.  I have no reason to believe it is anything other than fully
functional, however.

The conflicts in pu with v2 are probably still present, but I expect the
resolution should be easy (and I suspect Junio uses git rerere, so they'll be
even easier).

Changes from v2:
* Fix misnamed function in commit message.
* Improve parameter name of parse_oid_hex.
* Improve docstring of parse_oid_hex.
* Remove needless variable.
* Rebase on master.

Changes from v1:
* Implement parse_oid_hex and use it.
* Make nth_packed_object_oid take a variable into which to store the
  object ID.  This avoids concerns about unsafe casts.
* Rebase on master.

brian m. carlson (19):
  builtin/commit: convert to struct object_id
  builtin/diff-tree: convert to struct object_id
  builtin/describe: convert to struct object_id
  builtin/fast-export: convert to struct object_id
  builtin/fmt-merge-message: convert to struct object_id
  builtin/grep: convert to struct object_id
  builtin/branch: convert to struct object_id
  builtin/clone: convert to struct object_id
  builtin/merge: convert to struct object_id
  Convert remaining callers of resolve_refdup to object_id
  builtin/replace: convert to struct object_id
  reflog-walk: convert struct reflog_info to struct object_id
  refs: convert each_reflog_ent_fn to struct object_id
  hex: introduce parse_oid_hex
  refs: simplify parsing of reflog entries
  sha1_file: introduce an nth_packed_object_oid function
  Convert object iteration callbacks to struct object_id
  builtin/merge-base: convert to struct object_id
  wt-status: convert to struct object_id

 builtin/branch.c        |  26 +++++-----
 builtin/cat-file.c      |   8 +--
 builtin/clone.c         |  10 ++--
 builtin/commit.c        |  46 ++++++++---------
 builtin/count-objects.c |   4 +-
 builtin/describe.c      |  50 +++++++++---------
 builtin/diff-tree.c     |  38 +++++++-------
 builtin/fast-export.c   |  58 ++++++++++-----------
 builtin/fmt-merge-msg.c |  70 ++++++++++++-------------
 builtin/fsck.c          |  40 +++++++--------
 builtin/grep.c          |  24 ++++-----
 builtin/merge-base.c    |  30 +++++------
 builtin/merge.c         | 134 ++++++++++++++++++++++++------------------------
 builtin/notes.c         |  18 +++----
 builtin/pack-objects.c  |   6 +--
 builtin/prune-packed.c  |   4 +-
 builtin/prune.c         |   8 +--
 builtin/receive-pack.c  |   4 +-
 builtin/reflog.c        |   2 +-
 builtin/replace.c       | 112 ++++++++++++++++++++--------------------
 cache.h                 |  19 ++++++-
 hex.c                   |   8 +++
 reachable.c             |  30 +++++------
 ref-filter.c            |   4 +-
 reflog-walk.c           |  26 +++++-----
 refs.c                  |  24 ++++-----
 refs.h                  |   2 +-
 refs/files-backend.c    |  29 ++++++-----
 revision.c              |  12 ++---
 sha1_file.c             |  27 +++++++---
 sha1_name.c             |   2 +-
 transport.c             |   4 +-
 wt-status.c             |  52 +++++++++----------
 33 files changed, 483 insertions(+), 448 deletions(-)

-- 
2.11.0


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

* [PATCH v3 01/19] builtin/commit: convert to struct object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 02/19] builtin/diff-tree: " brian m. carlson
                   ` (17 subsequent siblings)
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

Convert most leaf functions to use struct object_id.

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

diff --git a/builtin/commit.c b/builtin/commit.c
index 2de5f6cc64..4e288bc513 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -496,7 +496,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
 static int run_status(FILE *fp, const char *index_file, const char *prefix, int nowarn,
 		      struct wt_status *s)
 {
-	unsigned char sha1[20];
+	struct object_id oid;
 
 	if (s->relative_paths)
 		s->prefix = prefix;
@@ -509,9 +509,9 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
 	s->index_file = index_file;
 	s->fp = fp;
 	s->nowarn = nowarn;
-	s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;
+	s->is_initial = get_sha1(s->reference, oid.hash) ? 1 : 0;
 	if (!s->is_initial)
-		hashcpy(s->sha1_commit, sha1);
+		hashcpy(s->sha1_commit, oid.hash);
 	s->status_format = status_format;
 	s->ignore_submodule_arg = ignore_submodule_arg;
 
@@ -885,7 +885,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		commitable = run_status(s->fp, index_file, prefix, 1, s);
 		s->use_color = saved_color_setting;
 	} else {
-		unsigned char sha1[20];
+		struct object_id oid;
 		const char *parent = "HEAD";
 
 		if (!active_nr && read_cache() < 0)
@@ -894,7 +894,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		if (amend)
 			parent = "HEAD^1";
 
-		if (get_sha1(parent, sha1)) {
+		if (get_sha1(parent, oid.hash)) {
 			int i, ita_nr = 0;
 
 			for (i = 0; i < active_nr; i++)
@@ -1332,7 +1332,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 {
 	static struct wt_status s;
 	int fd;
-	unsigned char sha1[20];
+	struct object_id oid;
 	static struct option builtin_status_options[] = {
 		OPT__VERBOSE(&verbose, N_("be verbose")),
 		OPT_SET_INT('s', "short", &status_format,
@@ -1382,9 +1382,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)
 
 	fd = hold_locked_index(&index_lock, 0);
 
-	s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
+	s.is_initial = get_sha1(s.reference, oid.hash) ? 1 : 0;
 	if (!s.is_initial)
-		hashcpy(s.sha1_commit, sha1);
+		hashcpy(s.sha1_commit, oid.hash);
 
 	s.ignore_submodule_arg = ignore_submodule_arg;
 	s.status_format = status_format;
@@ -1418,19 +1418,19 @@ static const char *implicit_ident_advice(void)
 
 }
 
-static void print_summary(const char *prefix, const unsigned char *sha1,
+static void print_summary(const char *prefix, const struct object_id *oid,
 			  int initial_commit)
 {
 	struct rev_info rev;
 	struct commit *commit;
 	struct strbuf format = STRBUF_INIT;
-	unsigned char junk_sha1[20];
+	struct object_id junk_oid;
 	const char *head;
 	struct pretty_print_context pctx = {0};
 	struct strbuf author_ident = STRBUF_INIT;
 	struct strbuf committer_ident = STRBUF_INIT;
 
-	commit = lookup_commit(sha1);
+	commit = lookup_commit(oid->hash);
 	if (!commit)
 		die(_("couldn't look up newly created commit"));
 	if (parse_commit(commit))
@@ -1477,7 +1477,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
 	rev.diffopt.break_opt = 0;
 	diff_setup_done(&rev.diffopt);
 
-	head = resolve_ref_unsafe("HEAD", 0, junk_sha1, NULL);
+	head = resolve_ref_unsafe("HEAD", 0, junk_oid.hash, NULL);
 	if (!strcmp(head, "HEAD"))
 		head = _("detached HEAD");
 	else
@@ -1522,8 +1522,8 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 	return git_status_config(k, v, s);
 }
 
-static int run_rewrite_hook(const unsigned char *oldsha1,
-			    const unsigned char *newsha1)
+static int run_rewrite_hook(const struct object_id *oldoid,
+			    const struct object_id *newoid)
 {
 	struct child_process proc = CHILD_PROCESS_INIT;
 	const char *argv[3];
@@ -1544,7 +1544,7 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
 	code = start_command(&proc);
 	if (code)
 		return code;
-	strbuf_addf(&sb, "%s %s\n", sha1_to_hex(oldsha1), sha1_to_hex(newsha1));
+	strbuf_addf(&sb, "%s %s\n", oid_to_hex(oldoid), oid_to_hex(newoid));
 	sigchain_push(SIGPIPE, SIG_IGN);
 	write_in_full(proc.in, sb.buf, sb.len);
 	close(proc.in);
@@ -1636,7 +1636,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	struct strbuf author_ident = STRBUF_INIT;
 	const char *index_file, *reflog_msg;
 	char *nl;
-	unsigned char sha1[20];
+	struct object_id oid;
 	struct commit_list *parents = NULL;
 	struct stat statbuf;
 	struct commit *current_head = NULL;
@@ -1651,10 +1651,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
 	s.colopts = 0;
 
-	if (get_sha1("HEAD", sha1))
+	if (get_sha1("HEAD", oid.hash))
 		current_head = NULL;
 	else {
-		current_head = lookup_commit_or_die(sha1, "HEAD");
+		current_head = lookup_commit_or_die(oid.hash, "HEAD");
 		if (parse_commit(current_head))
 			die(_("could not parse HEAD commit"));
 	}
@@ -1759,7 +1759,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	}
 
 	if (commit_tree_extended(sb.buf, sb.len, active_cache_tree->sha1,
-			 parents, sha1, author_ident.buf, sign_commit, extra)) {
+			 parents, oid.hash, author_ident.buf, sign_commit, extra)) {
 		rollback_index_files();
 		die(_("failed to write commit object"));
 	}
@@ -1776,7 +1776,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 
 	transaction = ref_transaction_begin(&err);
 	if (!transaction ||
-	    ref_transaction_update(transaction, "HEAD", sha1,
+	    ref_transaction_update(transaction, "HEAD", oid.hash,
 				   current_head
 				   ? current_head->object.oid.hash : null_sha1,
 				   0, sb.buf, &err) ||
@@ -1805,13 +1805,13 @@ 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, sha1);
+			copy_note_for_rewrite(cfg, current_head->object.oid.hash, oid.hash);
 			finish_copy_notes_for_rewrite(cfg, "Notes added by 'git commit --amend'");
 		}
-		run_rewrite_hook(current_head->object.oid.hash, sha1);
+		run_rewrite_hook(&current_head->object.oid, &oid);
 	}
 	if (!quiet)
-		print_summary(prefix, sha1, !current_head);
+		print_summary(prefix, &oid, !current_head);
 
 	strbuf_release(&err);
 	return 0;
-- 
2.11.0


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

* [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 01/19] builtin/commit: convert to struct object_id brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  1:18   ` Ramsay Jones
  2017-02-18  0:06 ` [PATCH v3 03/19] builtin/describe: " brian m. carlson
                   ` (16 subsequent siblings)
  18 siblings, 1 reply; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

Convert most leaf functions to struct object_id.  Rewrite several
hardcoded numbers in terms of GIT_SHA1_HEXSZ, using an intermediate
variable where that makes sense.

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

diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 8ce00480cd..1f1573bb2a 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -7,9 +7,9 @@
 
 static struct rev_info log_tree_opt;
 
-static int diff_tree_commit_sha1(const unsigned char *sha1)
+static int diff_tree_commit_sha1(const struct object_id *oid)
 {
-	struct commit *commit = lookup_commit_reference(sha1);
+	struct commit *commit = lookup_commit_reference(oid->hash);
 	if (!commit)
 		return -1;
 	return log_tree_commit(&log_tree_opt, commit);
@@ -18,22 +18,22 @@ static int diff_tree_commit_sha1(const unsigned char *sha1)
 /* Diff one or more commits. */
 static int stdin_diff_commit(struct commit *commit, char *line, int len)
 {
-	unsigned char sha1[20];
-	if (isspace(line[40]) && !get_sha1_hex(line+41, sha1)) {
+	struct object_id oid;
+	if (isspace(line[GIT_SHA1_HEXSZ]) && !get_oid_hex(line+GIT_SHA1_HEXSZ+1, &oid)) {
 		/* Graft the fake parents locally to the commit */
-		int pos = 41;
+		int pos = GIT_SHA1_HEXSZ + 1;
 		struct commit_list **pptr;
 
 		/* Free the real parent list */
 		free_commit_list(commit->parents);
 		commit->parents = NULL;
 		pptr = &(commit->parents);
-		while (line[pos] && !get_sha1_hex(line + pos, sha1)) {
-			struct commit *parent = lookup_commit(sha1);
+		while (line[pos] && !get_oid_hex(line + pos, &oid)) {
+			struct commit *parent = lookup_commit(oid.hash);
 			if (parent) {
 				pptr = &commit_list_insert(parent, pptr)->next;
 			}
-			pos += 41;
+			pos += GIT_SHA1_HEXSZ + 1;
 		}
 	}
 	return log_tree_commit(&log_tree_opt, commit);
@@ -42,11 +42,13 @@ static int stdin_diff_commit(struct commit *commit, char *line, int len)
 /* Diff two trees. */
 static int stdin_diff_trees(struct tree *tree1, char *line, int len)
 {
-	unsigned char sha1[20];
+	struct object_id oid;
 	struct tree *tree2;
-	if (len != 82 || !isspace(line[40]) || get_sha1_hex(line + 41, sha1))
+	const int chunksz = GIT_SHA1_HEXSZ + 1;
+	if (len != 2 * chunksz || !isspace(line[chunksz-1]) ||
+		get_sha1_hex(line + chunksz, oid.hash))
 		return error("Need exactly two trees, separated by a space");
-	tree2 = lookup_tree(sha1);
+	tree2 = lookup_tree(oid.hash);
 	if (!tree2 || parse_tree(tree2))
 		return -1;
 	printf("%s %s\n", oid_to_hex(&tree1->object.oid),
@@ -60,15 +62,15 @@ static int stdin_diff_trees(struct tree *tree1, char *line, int len)
 static int diff_tree_stdin(char *line)
 {
 	int len = strlen(line);
-	unsigned char sha1[20];
+	struct object_id oid;
 	struct object *obj;
 
 	if (!len || line[len-1] != '\n')
 		return -1;
 	line[len-1] = 0;
-	if (get_sha1_hex(line, sha1))
+	if (get_oid_hex(line, &oid))
 		return -1;
-	obj = parse_object(sha1);
+	obj = parse_object(oid.hash);
 	if (!obj)
 		return -1;
 	if (obj->type == OBJ_COMMIT)
@@ -76,7 +78,7 @@ static int diff_tree_stdin(char *line)
 	if (obj->type == OBJ_TREE)
 		return stdin_diff_trees((struct tree *)obj, line, len);
 	error("Object %s is a %s, not a commit or tree",
-	      sha1_to_hex(sha1), typename(obj->type));
+	      oid_to_hex(&oid), typename(obj->type));
 	return -1;
 }
 
@@ -141,7 +143,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 		break;
 	case 1:
 		tree1 = opt->pending.objects[0].item;
-		diff_tree_commit_sha1(tree1->oid.hash);
+		diff_tree_commit_sha1(&tree1->oid);
 		break;
 	case 2:
 		tree1 = opt->pending.objects[0].item;
@@ -164,9 +166,9 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
 			opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
 					       DIFF_SETUP_USE_CACHE);
 		while (fgets(line, sizeof(line), stdin)) {
-			unsigned char sha1[20];
+			struct object_id oid;
 
-			if (get_sha1_hex(line, sha1)) {
+			if (get_oid_hex(line, &oid)) {
 				fputs(line, stdout);
 				fflush(stdout);
 			}
-- 
2.11.0


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

* [PATCH v3 03/19] builtin/describe: convert to struct object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 01/19] builtin/commit: convert to struct object_id brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 02/19] builtin/diff-tree: " brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 04/19] builtin/fast-export: " brian m. carlson
                   ` (15 subsequent siblings)
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

Convert the functions in this file and struct commit_name  to struct
object_id.

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

diff --git a/builtin/describe.c b/builtin/describe.c
index 01490a157e..738e68f95b 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -39,11 +39,11 @@ static const char *diff_index_args[] = {
 
 struct commit_name {
 	struct hashmap_entry entry;
-	unsigned char peeled[20];
+	struct object_id peeled;
 	struct tag *tag;
 	unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */
 	unsigned name_checked:1;
-	unsigned char sha1[20];
+	struct object_id oid;
 	char *path;
 };
 
@@ -54,17 +54,17 @@ static const char *prio_names[] = {
 static int commit_name_cmp(const struct commit_name *cn1,
 		const struct commit_name *cn2, const void *peeled)
 {
-	return hashcmp(cn1->peeled, peeled ? peeled : cn2->peeled);
+	return oidcmp(&cn1->peeled, peeled ? peeled : &cn2->peeled);
 }
 
-static inline struct commit_name *find_commit_name(const unsigned char *peeled)
+static inline struct commit_name *find_commit_name(const struct object_id *peeled)
 {
-	return hashmap_get_from_hash(&names, sha1hash(peeled), peeled);
+	return hashmap_get_from_hash(&names, sha1hash(peeled->hash), peeled->hash);
 }
 
 static int replace_name(struct commit_name *e,
 			       int prio,
-			       const unsigned char *sha1,
+			       const struct object_id *oid,
 			       struct tag **tag)
 {
 	if (!e || e->prio < prio)
@@ -77,13 +77,13 @@ static int replace_name(struct commit_name *e,
 		struct tag *t;
 
 		if (!e->tag) {
-			t = lookup_tag(e->sha1);
+			t = lookup_tag(e->oid.hash);
 			if (!t || parse_tag(t))
 				return 1;
 			e->tag = t;
 		}
 
-		t = lookup_tag(sha1);
+		t = lookup_tag(oid->hash);
 		if (!t || parse_tag(t))
 			return 0;
 		*tag = t;
@@ -96,24 +96,24 @@ static int replace_name(struct commit_name *e,
 }
 
 static void add_to_known_names(const char *path,
-			       const unsigned char *peeled,
+			       const struct object_id *peeled,
 			       int prio,
-			       const unsigned char *sha1)
+			       const struct object_id *oid)
 {
 	struct commit_name *e = find_commit_name(peeled);
 	struct tag *tag = NULL;
-	if (replace_name(e, prio, sha1, &tag)) {
+	if (replace_name(e, prio, oid, &tag)) {
 		if (!e) {
 			e = xmalloc(sizeof(struct commit_name));
-			hashcpy(e->peeled, peeled);
-			hashmap_entry_init(e, sha1hash(peeled));
+			oidcpy(&e->peeled, peeled);
+			hashmap_entry_init(e, sha1hash(peeled->hash));
 			hashmap_add(&names, e);
 			e->path = NULL;
 		}
 		e->tag = tag;
 		e->prio = prio;
 		e->name_checked = 0;
-		hashcpy(e->sha1, sha1);
+		oidcpy(&e->oid, oid);
 		free(e->path);
 		e->path = xstrdup(path);
 	}
@@ -154,7 +154,7 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi
 	else
 		prio = 0;
 
-	add_to_known_names(all ? path + 5 : path + 10, peeled.hash, prio, oid->hash);
+	add_to_known_names(all ? path + 5 : path + 10, &peeled, prio, oid);
 	return 0;
 }
 
@@ -212,7 +212,7 @@ static unsigned long finish_depth_computation(
 static void display_name(struct commit_name *n)
 {
 	if (n->prio == 2 && !n->tag) {
-		n->tag = lookup_tag(n->sha1);
+		n->tag = lookup_tag(n->oid.hash);
 		if (!n->tag || parse_tag(n->tag))
 			die(_("annotated tag %s not available"), n->path);
 	}
@@ -230,14 +230,14 @@ static void display_name(struct commit_name *n)
 		printf("%s", n->path);
 }
 
-static void show_suffix(int depth, const unsigned char *sha1)
+static void show_suffix(int depth, const struct object_id *oid)
 {
-	printf("-%d-g%s", depth, find_unique_abbrev(sha1, abbrev));
+	printf("-%d-g%s", depth, find_unique_abbrev(oid->hash, abbrev));
 }
 
 static void describe(const char *arg, int last_one)
 {
-	unsigned char sha1[20];
+	struct object_id oid;
 	struct commit *cmit, *gave_up_on = NULL;
 	struct commit_list *list;
 	struct commit_name *n;
@@ -246,20 +246,20 @@ static void describe(const char *arg, int last_one)
 	unsigned long seen_commits = 0;
 	unsigned int unannotated_cnt = 0;
 
-	if (get_sha1(arg, sha1))
+	if (get_oid(arg, &oid))
 		die(_("Not a valid object name %s"), arg);
-	cmit = lookup_commit_reference(sha1);
+	cmit = lookup_commit_reference(oid.hash);
 	if (!cmit)
 		die(_("%s is not a valid '%s' object"), arg, commit_type);
 
-	n = find_commit_name(cmit->object.oid.hash);
+	n = find_commit_name(&cmit->object.oid);
 	if (n && (tags || all || n->prio == 2)) {
 		/*
 		 * Exact match to an existing ref.
 		 */
 		display_name(n);
 		if (longformat)
-			show_suffix(0, n->tag ? n->tag->tagged->oid.hash : sha1);
+			show_suffix(0, n->tag ? &n->tag->tagged->oid : &oid);
 		if (dirty)
 			printf("%s", dirty);
 		printf("\n");
@@ -276,7 +276,7 @@ static void describe(const char *arg, int last_one)
 		struct commit *c;
 		struct commit_name *n = hashmap_iter_first(&names, &iter);
 		for (; n; n = hashmap_iter_next(&iter)) {
-			c = lookup_commit_reference_gently(n->peeled, 1);
+			c = lookup_commit_reference_gently(n->peeled.hash, 1);
 			if (c)
 				c->util = n;
 		}
@@ -380,7 +380,7 @@ static void describe(const char *arg, int last_one)
 
 	display_name(all_matches[0].name);
 	if (abbrev)
-		show_suffix(all_matches[0].depth, cmit->object.oid.hash);
+		show_suffix(all_matches[0].depth, &cmit->object.oid);
 	if (dirty)
 		printf("%s", dirty);
 	printf("\n");
-- 
2.11.0


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

* [PATCH v3 04/19] builtin/fast-export: convert to struct object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (2 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 03/19] builtin/describe: " brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 05/19] builtin/fmt-merge-message: " brian m. carlson
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

In addition to converting to struct object_id, write some hardcoded
buffer sizes in terms of GIT_SHA1_RAWSZ.

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

diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 1e815b5577..e0220630d0 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -212,7 +212,7 @@ static char *anonymize_blob(unsigned long *size)
 	return strbuf_detach(&out, NULL);
 }
 
-static void export_blob(const unsigned char *sha1)
+static void export_blob(const struct object_id *oid)
 {
 	unsigned long size;
 	enum object_type type;
@@ -223,34 +223,34 @@ static void export_blob(const unsigned char *sha1)
 	if (no_data)
 		return;
 
-	if (is_null_sha1(sha1))
+	if (is_null_oid(oid))
 		return;
 
-	object = lookup_object(sha1);
+	object = lookup_object(oid->hash);
 	if (object && object->flags & SHOWN)
 		return;
 
 	if (anonymize) {
 		buf = anonymize_blob(&size);
-		object = (struct object *)lookup_blob(sha1);
+		object = (struct object *)lookup_blob(oid->hash);
 		eaten = 0;
 	} else {
-		buf = read_sha1_file(sha1, &type, &size);
+		buf = read_sha1_file(oid->hash, &type, &size);
 		if (!buf)
-			die ("Could not read blob %s", sha1_to_hex(sha1));
-		if (check_sha1_signature(sha1, buf, size, typename(type)) < 0)
-			die("sha1 mismatch in blob %s", sha1_to_hex(sha1));
-		object = parse_object_buffer(sha1, type, size, buf, &eaten);
+			die ("Could not read blob %s", oid_to_hex(oid));
+		if (check_sha1_signature(oid->hash, buf, size, typename(type)) < 0)
+			die("sha1 mismatch in blob %s", oid_to_hex(oid));
+		object = parse_object_buffer(oid->hash, type, size, buf, &eaten);
 	}
 
 	if (!object)
-		die("Could not read blob %s", sha1_to_hex(sha1));
+		die("Could not read blob %s", oid_to_hex(oid));
 
 	mark_next_object(object);
 
 	printf("blob\nmark :%"PRIu32"\ndata %lu\n", last_idnum, size);
 	if (size && fwrite(buf, size, 1, stdout) != 1)
-		die_errno ("Could not write blob '%s'", sha1_to_hex(sha1));
+		die_errno ("Could not write blob '%s'", oid_to_hex(oid));
 	printf("\n");
 
 	show_progress();
@@ -323,19 +323,19 @@ static void print_path(const char *path)
 	}
 }
 
-static void *generate_fake_sha1(const void *old, size_t *len)
+static void *generate_fake_oid(const void *old, size_t *len)
 {
 	static uint32_t counter = 1; /* avoid null sha1 */
-	unsigned char *out = xcalloc(20, 1);
-	put_be32(out + 16, counter++);
+	unsigned char *out = xcalloc(GIT_SHA1_RAWSZ, 1);
+	put_be32(out + GIT_SHA1_RAWSZ - 4, counter++);
 	return out;
 }
 
-static const unsigned char *anonymize_sha1(const unsigned char *sha1)
+static const unsigned char *anonymize_sha1(const struct object_id *oid)
 {
 	static struct hashmap sha1s;
-	size_t len = 20;
-	return anonymize_mem(&sha1s, generate_fake_sha1, sha1, &len);
+	size_t len = GIT_SHA1_RAWSZ;
+	return anonymize_mem(&sha1s, generate_fake_oid, oid, &len);
 }
 
 static void show_filemodify(struct diff_queue_struct *q,
@@ -383,7 +383,7 @@ static void show_filemodify(struct diff_queue_struct *q,
 			if (no_data || S_ISGITLINK(spec->mode))
 				printf("M %06o %s ", spec->mode,
 				       sha1_to_hex(anonymize ?
-						   anonymize_sha1(spec->oid.hash) :
+						   anonymize_sha1(&spec->oid) :
 						   spec->oid.hash));
 			else {
 				struct object *object = lookup_object(spec->oid.hash);
@@ -572,7 +572,7 @@ static void handle_commit(struct commit *commit, struct rev_info *rev)
 	/* Export the referenced blobs, and remember the marks. */
 	for (i = 0; i < diff_queued_diff.nr; i++)
 		if (!S_ISGITLINK(diff_queued_diff.queue[i]->two->mode))
-			export_blob(diff_queued_diff.queue[i]->two->oid.hash);
+			export_blob(&diff_queued_diff.queue[i]->two->oid);
 
 	refname = commit->util;
 	if (anonymize) {
@@ -797,14 +797,14 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info)
 
 	for (i = 0; i < info->nr; i++) {
 		struct rev_cmdline_entry *e = info->rev + i;
-		unsigned char sha1[20];
+		struct object_id oid;
 		struct commit *commit;
 		char *full_name;
 
 		if (e->flags & UNINTERESTING)
 			continue;
 
-		if (dwim_ref(e->name, strlen(e->name), sha1, &full_name) != 1)
+		if (dwim_ref(e->name, strlen(e->name), oid.hash, &full_name) != 1)
 			continue;
 
 		if (refspecs) {
@@ -828,7 +828,7 @@ static void get_tags_and_duplicates(struct rev_cmdline_info *info)
 		case OBJ_COMMIT:
 			break;
 		case OBJ_BLOB:
-			export_blob(commit->object.oid.hash);
+			export_blob(&commit->object.oid);
 			continue;
 		default: /* OBJ_TAG (nested tags) is already handled */
 			warning("Tag points to object of unexpected type %s, skipping.",
@@ -912,7 +912,7 @@ static void import_marks(char *input_file)
 	while (fgets(line, sizeof(line), f)) {
 		uint32_t mark;
 		char *line_end, *mark_end;
-		unsigned char sha1[20];
+		struct object_id oid;
 		struct object *object;
 		struct commit *commit;
 		enum object_type type;
@@ -924,28 +924,28 @@ static void import_marks(char *input_file)
 
 		mark = strtoumax(line + 1, &mark_end, 10);
 		if (!mark || mark_end == line + 1
-			|| *mark_end != ' ' || get_sha1_hex(mark_end + 1, sha1))
+			|| *mark_end != ' ' || get_oid_hex(mark_end + 1, &oid))
 			die("corrupt mark line: %s", line);
 
 		if (last_idnum < mark)
 			last_idnum = mark;
 
-		type = sha1_object_info(sha1, NULL);
+		type = sha1_object_info(oid.hash, NULL);
 		if (type < 0)
-			die("object not found: %s", sha1_to_hex(sha1));
+			die("object not found: %s", oid_to_hex(&oid));
 
 		if (type != OBJ_COMMIT)
 			/* only commits */
 			continue;
 
-		commit = lookup_commit(sha1);
+		commit = lookup_commit(oid.hash);
 		if (!commit)
-			die("not a commit? can't happen: %s", sha1_to_hex(sha1));
+			die("not a commit? can't happen: %s", oid_to_hex(&oid));
 
 		object = &commit->object;
 
 		if (object->flags & SHOWN)
-			error("Object %s already has a mark", sha1_to_hex(sha1));
+			error("Object %s already has a mark", oid_to_hex(&oid));
 
 		mark_object(object, mark);
 
-- 
2.11.0


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

* [PATCH v3 05/19] builtin/fmt-merge-message: convert to struct object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (3 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 04/19] builtin/fast-export: " brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 06/19] builtin/grep: " brian m. carlson
                   ` (13 subsequent siblings)
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

Convert most of the code to use struct object_id, including struct
origin_data and struct merge_parents.  Convert several instances of
hardcoded numbers into references to GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/fmt-merge-msg.c | 70 ++++++++++++++++++++++++-------------------------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index efab62fd85..6faa3c0d24 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -41,7 +41,7 @@ struct src_data {
 };
 
 struct origin_data {
-	unsigned char sha1[20];
+	struct object_id oid;
 	unsigned is_local_branch:1;
 };
 
@@ -59,8 +59,8 @@ static struct string_list origins = STRING_LIST_INIT_DUP;
 struct merge_parents {
 	int alloc, nr;
 	struct merge_parent {
-		unsigned char given[20];
-		unsigned char commit[20];
+		struct object_id given;
+		struct object_id commit;
 		unsigned char used;
 	} *item;
 };
@@ -70,14 +70,14 @@ struct merge_parents {
  * hundreds of heads at a time anyway.
  */
 static struct merge_parent *find_merge_parent(struct merge_parents *table,
-					      unsigned char *given,
-					      unsigned char *commit)
+					      struct object_id *given,
+					      struct object_id *commit)
 {
 	int i;
 	for (i = 0; i < table->nr; i++) {
-		if (given && hashcmp(table->item[i].given, given))
+		if (given && oidcmp(&table->item[i].given, given))
 			continue;
-		if (commit && hashcmp(table->item[i].commit, commit))
+		if (commit && oidcmp(&table->item[i].commit, commit))
 			continue;
 		return &table->item[i];
 	}
@@ -85,14 +85,14 @@ static struct merge_parent *find_merge_parent(struct merge_parents *table,
 }
 
 static void add_merge_parent(struct merge_parents *table,
-			     unsigned char *given,
-			     unsigned char *commit)
+			     struct object_id *given,
+			     struct object_id *commit)
 {
 	if (table->nr && find_merge_parent(table, given, commit))
 		return;
 	ALLOC_GROW(table->item, table->nr + 1, table->alloc);
-	hashcpy(table->item[table->nr].given, given);
-	hashcpy(table->item[table->nr].commit, commit);
+	oidcpy(&table->item[table->nr].given, given);
+	oidcpy(&table->item[table->nr].commit, commit);
 	table->item[table->nr].used = 0;
 	table->nr++;
 }
@@ -106,30 +106,30 @@ static int handle_line(char *line, struct merge_parents *merge_parents)
 	struct src_data *src_data;
 	struct string_list_item *item;
 	int pulling_head = 0;
-	unsigned char sha1[20];
+	struct object_id oid;
 
-	if (len < 43 || line[40] != '\t')
+	if (len < GIT_SHA1_HEXSZ + 3 || line[GIT_SHA1_HEXSZ] != '\t')
 		return 1;
 
-	if (starts_with(line + 41, "not-for-merge"))
+	if (starts_with(line + GIT_SHA1_HEXSZ + 1, "not-for-merge"))
 		return 0;
 
-	if (line[41] != '\t')
+	if (line[GIT_SHA1_HEXSZ + 1] != '\t')
 		return 2;
 
-	i = get_sha1_hex(line, sha1);
+	i = get_oid_hex(line, &oid);
 	if (i)
 		return 3;
 
-	if (!find_merge_parent(merge_parents, sha1, NULL))
+	if (!find_merge_parent(merge_parents, &oid, NULL))
 		return 0; /* subsumed by other parents */
 
 	origin_data = xcalloc(1, sizeof(struct origin_data));
-	hashcpy(origin_data->sha1, sha1);
+	oidcpy(&origin_data->oid, &oid);
 
 	if (line[len - 1] == '\n')
 		line[len - 1] = 0;
-	line += 42;
+	line += GIT_SHA1_HEXSZ + 2;
 
 	/*
 	 * At this point, line points at the beginning of comment e.g.
@@ -338,10 +338,10 @@ static void shortlog(const char *name,
 	struct string_list committers = STRING_LIST_INIT_DUP;
 	int flags = UNINTERESTING | TREESAME | SEEN | SHOWN | ADDED;
 	struct strbuf sb = STRBUF_INIT;
-	const unsigned char *sha1 = origin_data->sha1;
+	const struct object_id *oid = &origin_data->oid;
 	int limit = opts->shortlog_len;
 
-	branch = deref_tag(parse_object(sha1), sha1_to_hex(sha1), 40);
+	branch = deref_tag(parse_object(oid->hash), oid_to_hex(oid), GIT_SHA1_HEXSZ);
 	if (!branch || branch->type != OBJ_COMMIT)
 		return;
 
@@ -531,7 +531,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
 }
 
 static void find_merge_parents(struct merge_parents *result,
-			       struct strbuf *in, unsigned char *head)
+			       struct strbuf *in, struct object_id *head)
 {
 	struct commit_list *parents;
 	struct commit *head_commit;
@@ -542,31 +542,31 @@ static void find_merge_parents(struct merge_parents *result,
 		int len;
 		char *p = in->buf + pos;
 		char *newline = strchr(p, '\n');
-		unsigned char sha1[20];
+		struct object_id oid;
 		struct commit *parent;
 		struct object *obj;
 
 		len = newline ? newline - p : strlen(p);
 		pos += len + !!newline;
 
-		if (len < 43 ||
-		    get_sha1_hex(p, sha1) ||
-		    p[40] != '\t' ||
-		    p[41] != '\t')
+		if (len < GIT_SHA1_HEXSZ + 3 ||
+		    get_oid_hex(p, &oid) ||
+		    p[GIT_SHA1_HEXSZ] != '\t' ||
+		    p[GIT_SHA1_HEXSZ + 1] != '\t')
 			continue; /* skip not-for-merge */
 		/*
 		 * Do not use get_merge_parent() here; we do not have
 		 * "name" here and we do not want to contaminate its
 		 * util field yet.
 		 */
-		obj = parse_object(sha1);
+		obj = parse_object(oid.hash);
 		parent = (struct commit *)peel_to_type(NULL, 0, obj, OBJ_COMMIT);
 		if (!parent)
 			continue;
 		commit_list_insert(parent, &parents);
-		add_merge_parent(result, obj->oid.hash, parent->object.oid.hash);
+		add_merge_parent(result, &obj->oid, &parent->object.oid);
 	}
-	head_commit = lookup_commit(head);
+	head_commit = lookup_commit(head->hash);
 	if (head_commit)
 		commit_list_insert(head_commit, &parents);
 	parents = reduce_heads(parents);
@@ -574,7 +574,7 @@ static void find_merge_parents(struct merge_parents *result,
 	while (parents) {
 		struct commit *cmit = pop_commit(&parents);
 		for (i = 0; i < result->nr; i++)
-			if (!hashcmp(result->item[i].commit, cmit->object.oid.hash))
+			if (!oidcmp(&result->item[i].commit, &cmit->object.oid))
 				result->item[i].used = 1;
 	}
 
@@ -592,7 +592,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 		  struct fmt_merge_msg_opts *opts)
 {
 	int i = 0, pos = 0;
-	unsigned char head_sha1[20];
+	struct object_id head_oid;
 	const char *current_branch;
 	void *current_branch_to_free;
 	struct merge_parents merge_parents;
@@ -601,13 +601,13 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 
 	/* get current branch */
 	current_branch = current_branch_to_free =
-		resolve_refdup("HEAD", RESOLVE_REF_READING, head_sha1, NULL);
+		resolve_refdup("HEAD", RESOLVE_REF_READING, head_oid.hash, NULL);
 	if (!current_branch)
 		die("No current branch");
 	if (starts_with(current_branch, "refs/heads/"))
 		current_branch += 11;
 
-	find_merge_parents(&merge_parents, in, head_sha1);
+	find_merge_parents(&merge_parents, in, &head_oid);
 
 	/* get a line */
 	while (pos < in->len) {
@@ -633,7 +633,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
 		struct commit *head;
 		struct rev_info rev;
 
-		head = lookup_commit_or_die(head_sha1, "HEAD");
+		head = lookup_commit_or_die(head_oid.hash, "HEAD");
 		init_revisions(&rev, NULL);
 		rev.commit_format = CMIT_FMT_ONELINE;
 		rev.ignore_merges = 1;
-- 
2.11.0


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

* [PATCH v3 06/19] builtin/grep: convert to struct object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (4 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 05/19] builtin/fmt-merge-message: " brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 07/19] builtin/branch: " brian m. carlson
                   ` (12 subsequent siblings)
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

Convert several functions to use struct object_id, and rename them so
that they no longer refer to SHA-1.

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

diff --git a/builtin/grep.c b/builtin/grep.c
index 2c727ef499..0393b0fdc4 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -294,17 +294,17 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)
 	return st;
 }
 
-static void *lock_and_read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size)
+static void *lock_and_read_oid_file(const struct object_id *oid, enum object_type *type, unsigned long *size)
 {
 	void *data;
 
 	grep_read_lock();
-	data = read_sha1_file(sha1, type, size);
+	data = read_sha1_file(oid->hash, type, size);
 	grep_read_unlock();
 	return data;
 }
 
-static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
+static int grep_oid(struct grep_opt *opt, const struct object_id *oid,
 		     const char *filename, int tree_name_len,
 		     const char *path)
 {
@@ -323,7 +323,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
 
 #ifndef NO_PTHREADS
 	if (num_threads) {
-		add_work(opt, GREP_SOURCE_SHA1, pathbuf.buf, path, sha1);
+		add_work(opt, GREP_SOURCE_SHA1, pathbuf.buf, path, oid);
 		strbuf_release(&pathbuf);
 		return 0;
 	} else
@@ -332,7 +332,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
 		struct grep_source gs;
 		int hit;
 
-		grep_source_init(&gs, GREP_SOURCE_SHA1, pathbuf.buf, path, sha1);
+		grep_source_init(&gs, GREP_SOURCE_SHA1, pathbuf.buf, path, oid);
 		strbuf_release(&pathbuf);
 		hit = grep_source(opt, &gs);
 
@@ -690,7 +690,7 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec,
 			    ce_skip_worktree(ce)) {
 				if (ce_stage(ce) || ce_intent_to_add(ce))
 					continue;
-				hit |= grep_sha1(opt, ce->oid.hash, ce->name,
+				hit |= grep_oid(opt, &ce->oid, ce->name,
 						 0, ce->name);
 			} else {
 				hit |= grep_file(opt, ce->name);
@@ -750,7 +750,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 		strbuf_add(base, entry.path, te_len);
 
 		if (S_ISREG(entry.mode)) {
-			hit |= grep_sha1(opt, entry.oid->hash, base->buf, tn_len,
+			hit |= grep_oid(opt, entry.oid, base->buf, tn_len,
 					 check_attr ? base->buf + tn_len : NULL);
 		} else if (S_ISDIR(entry.mode)) {
 			enum object_type type;
@@ -758,7 +758,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
 			void *data;
 			unsigned long size;
 
-			data = lock_and_read_sha1_file(entry.oid->hash, &type, &size);
+			data = lock_and_read_oid_file(entry.oid, &type, &size);
 			if (!data)
 				die(_("unable to read tree (%s)"),
 				    oid_to_hex(entry.oid));
@@ -787,7 +787,7 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
 		       struct object *obj, const char *name, const char *path)
 {
 	if (obj->type == OBJ_BLOB)
-		return grep_sha1(opt, obj->oid.hash, name, 0, path);
+		return grep_oid(opt, &obj->oid, name, 0, path);
 	if (obj->type == OBJ_COMMIT || obj->type == OBJ_TREE) {
 		struct tree_desc tree;
 		void *data;
@@ -1152,11 +1152,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	/* Check revs and then paths */
 	for (i = 0; i < argc; i++) {
 		const char *arg = argv[i];
-		unsigned char sha1[20];
+		struct object_id oid;
 		struct object_context oc;
 		/* Is it a rev? */
-		if (!get_sha1_with_context(arg, 0, sha1, &oc)) {
-			struct object *object = parse_object_or_die(sha1, arg);
+		if (!get_sha1_with_context(arg, 0, oid.hash, &oc)) {
+			struct object *object = parse_object_or_die(oid.hash, arg);
 			if (!seen_dashdash)
 				verify_non_filename(prefix, arg);
 			add_object_array_with_path(object, arg, &list, oc.mode, oc.path);
-- 
2.11.0


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

* [PATCH v3 07/19] builtin/branch: convert to struct object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (5 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 06/19] builtin/grep: " brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 08/19] builtin/clone: " brian m. carlson
                   ` (11 subsequent siblings)
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

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

diff --git a/builtin/branch.c b/builtin/branch.c
index 9d30f55b0b..faf472ff8f 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -32,7 +32,7 @@ static const char * const builtin_branch_usage[] = {
 };
 
 static const char *head;
-static unsigned char head_sha1[20];
+static struct object_id head_oid;
 
 static int branch_use_color = -1;
 static char branch_colors[][COLOR_MAXLEN] = {
@@ -117,13 +117,13 @@ static int branch_merged(int kind, const char *name,
 	if (kind == FILTER_REFS_BRANCHES) {
 		struct branch *branch = branch_get(name);
 		const char *upstream = branch_get_upstream(branch, NULL);
-		unsigned char sha1[20];
+		struct object_id oid;
 
 		if (upstream &&
 		    (reference_name = reference_name_to_free =
 		     resolve_refdup(upstream, RESOLVE_REF_READING,
-				    sha1, NULL)) != NULL)
-			reference_rev = lookup_commit_reference(sha1);
+				    oid.hash, NULL)) != NULL)
+			reference_rev = lookup_commit_reference(oid.hash);
 	}
 	if (!reference_rev)
 		reference_rev = head_rev;
@@ -153,10 +153,10 @@ static int branch_merged(int kind, const char *name,
 }
 
 static int check_branch_commit(const char *branchname, const char *refname,
-			       const unsigned char *sha1, struct commit *head_rev,
+			       const struct object_id *oid, struct commit *head_rev,
 			       int kinds, int force)
 {
-	struct commit *rev = lookup_commit_reference(sha1);
+	struct commit *rev = lookup_commit_reference(oid->hash);
 	if (!rev) {
 		error(_("Couldn't look up commit object for '%s'"), refname);
 		return -1;
@@ -183,7 +183,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 			   int quiet)
 {
 	struct commit *head_rev = NULL;
-	unsigned char sha1[20];
+	struct object_id oid;
 	char *name = NULL;
 	const char *fmt;
 	int i;
@@ -207,7 +207,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 	}
 
 	if (!force) {
-		head_rev = lookup_commit_reference(head_sha1);
+		head_rev = lookup_commit_reference(head_oid.hash);
 		if (!head_rev)
 			die(_("Couldn't look up commit object for HEAD"));
 	}
@@ -235,7 +235,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 					RESOLVE_REF_READING
 					| RESOLVE_REF_NO_RECURSE
 					| RESOLVE_REF_ALLOW_BAD_NAME,
-					sha1, &flags);
+					oid.hash, &flags);
 		if (!target) {
 			error(remote_branch
 			      ? _("remote-tracking branch '%s' not found.")
@@ -245,13 +245,13 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 		}
 
 		if (!(flags & (REF_ISSYMREF|REF_ISBROKEN)) &&
-		    check_branch_commit(bname.buf, name, sha1, head_rev, kinds,
+		    check_branch_commit(bname.buf, name, &oid, head_rev, kinds,
 					force)) {
 			ret = 1;
 			goto next;
 		}
 
-		if (delete_ref(name, is_null_sha1(sha1) ? NULL : sha1,
+		if (delete_ref(name, is_null_oid(&oid) ? NULL : oid.hash,
 			       REF_NODEREF)) {
 			error(remote_branch
 			      ? _("Error deleting remote-tracking branch '%s'")
@@ -267,7 +267,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,
 			       bname.buf,
 			       (flags & REF_ISBROKEN) ? "broken"
 			       : (flags & REF_ISSYMREF) ? target
-			       : find_unique_abbrev(sha1, DEFAULT_ABBREV));
+			       : find_unique_abbrev(oid.hash, DEFAULT_ABBREV));
 		}
 		delete_branch_config(bname.buf);
 
@@ -693,7 +693,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 
 	track = git_branch_track;
 
-	head = resolve_refdup("HEAD", 0, head_sha1, NULL);
+	head = resolve_refdup("HEAD", 0, head_oid.hash, NULL);
 	if (!head)
 		die(_("Failed to resolve HEAD as a valid ref."));
 	if (!strcmp(head, "HEAD"))
-- 
2.11.0


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

* [PATCH v3 08/19] builtin/clone: convert to struct object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (6 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 07/19] builtin/branch: " brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 09/19] builtin/merge: " brian m. carlson
                   ` (10 subsequent siblings)
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

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

diff --git a/builtin/clone.c b/builtin/clone.c
index 3f63edbbf9..b4c929bb8a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -681,7 +681,7 @@ static void update_head(const struct ref *our, const struct ref *remote,
 
 static int checkout(int submodule_progress)
 {
-	unsigned char sha1[20];
+	struct object_id oid;
 	char *head;
 	struct lock_file *lock_file;
 	struct unpack_trees_options opts;
@@ -692,7 +692,7 @@ static int checkout(int submodule_progress)
 	if (option_no_checkout)
 		return 0;
 
-	head = resolve_refdup("HEAD", RESOLVE_REF_READING, sha1, NULL);
+	head = resolve_refdup("HEAD", RESOLVE_REF_READING, oid.hash, NULL);
 	if (!head) {
 		warning(_("remote HEAD refers to nonexistent ref, "
 			  "unable to checkout.\n"));
@@ -700,7 +700,7 @@ static int checkout(int submodule_progress)
 	}
 	if (!strcmp(head, "HEAD")) {
 		if (advice_detached_head)
-			detach_advice(sha1_to_hex(sha1));
+			detach_advice(oid_to_hex(&oid));
 	} else {
 		if (!starts_with(head, "refs/heads/"))
 			die(_("HEAD not found below refs/heads!"));
@@ -721,7 +721,7 @@ static int checkout(int submodule_progress)
 	opts.src_index = &the_index;
 	opts.dst_index = &the_index;
 
-	tree = parse_tree_indirect(sha1);
+	tree = parse_tree_indirect(oid.hash);
 	parse_tree(tree);
 	init_tree_desc(&t, tree->buffer, tree->size);
 	if (unpack_trees(1, &t, &opts) < 0)
@@ -731,7 +731,7 @@ static int checkout(int submodule_progress)
 		die(_("unable to write new index file"));
 
 	err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
-			   sha1_to_hex(sha1), "1", NULL);
+			   oid_to_hex(&oid), "1", NULL);
 
 	if (!err && option_recursive) {
 		struct argv_array args = ARGV_ARRAY_INIT;
-- 
2.11.0


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

* [PATCH v3 09/19] builtin/merge: convert to struct object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (7 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 08/19] builtin/clone: " brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 10/19] Convert remaining callers of resolve_refdup to object_id brian m. carlson
                   ` (9 subsequent siblings)
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

Additionally convert several uses of the constant 40 into
GIT_SHA1_HEXSZ.

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

diff --git a/builtin/merge.c b/builtin/merge.c
index a96d4fb501..099cfab447 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -244,7 +244,7 @@ static void drop_save(void)
 	unlink(git_path_merge_mode());
 }
 
-static int save_state(unsigned char *stash)
+static int save_state(struct object_id *stash)
 {
 	int len;
 	struct child_process cp = CHILD_PROCESS_INIT;
@@ -265,7 +265,7 @@ static int save_state(unsigned char *stash)
 	else if (!len)		/* no changes */
 		return -1;
 	strbuf_setlen(&buffer, buffer.len-1);
-	if (get_sha1(buffer.buf, stash))
+	if (get_oid(buffer.buf, stash))
 		die(_("not a valid object: %s"), buffer.buf);
 	return 0;
 }
@@ -305,18 +305,18 @@ static void reset_hard(unsigned const char *sha1, int verbose)
 		die(_("read-tree failed"));
 }
 
-static void restore_state(const unsigned char *head,
-			  const unsigned char *stash)
+static void restore_state(const struct object_id *head,
+			  const struct object_id *stash)
 {
 	struct strbuf sb = STRBUF_INIT;
 	const char *args[] = { "stash", "apply", NULL, NULL };
 
-	if (is_null_sha1(stash))
+	if (is_null_oid(stash))
 		return;
 
-	reset_hard(head, 1);
+	reset_hard(head->hash, 1);
 
-	args[2] = sha1_to_hex(stash);
+	args[2] = oid_to_hex(stash);
 
 	/*
 	 * It is OK to ignore error here, for example when there was
@@ -376,10 +376,10 @@ static void squash_message(struct commit *commit, struct commit_list *remotehead
 
 static void finish(struct commit *head_commit,
 		   struct commit_list *remoteheads,
-		   const unsigned char *new_head, const char *msg)
+		   const struct object_id *new_head, const char *msg)
 {
 	struct strbuf reflog_message = STRBUF_INIT;
-	const unsigned char *head = head_commit->object.oid.hash;
+	const struct object_id *head = &head_commit->object.oid;
 
 	if (!msg)
 		strbuf_addstr(&reflog_message, getenv("GIT_REFLOG_ACTION"));
@@ -397,7 +397,7 @@ static void finish(struct commit *head_commit,
 		else {
 			const char *argv_gc_auto[] = { "gc", "--auto", NULL };
 			update_ref(reflog_message.buf, "HEAD",
-				new_head, head, 0,
+				new_head->hash, head->hash, 0,
 				UPDATE_REFS_DIE_ON_ERR);
 			/*
 			 * We ignore errors in 'gc --auto', since the
@@ -416,7 +416,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, new_head, "", &opts);
+		diff_tree_sha1(head->hash, new_head->hash, "", &opts);
 		diffcore_std(&opts);
 		diff_flush(&opts);
 	}
@@ -431,7 +431,7 @@ static void finish(struct commit *head_commit,
 static void merge_name(const char *remote, struct strbuf *msg)
 {
 	struct commit *remote_head;
-	unsigned char branch_head[20];
+	struct object_id branch_head;
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf bname = STRBUF_INIT;
 	const char *ptr;
@@ -441,25 +441,25 @@ static void merge_name(const char *remote, struct strbuf *msg)
 	strbuf_branchname(&bname, remote);
 	remote = bname.buf;
 
-	memset(branch_head, 0, sizeof(branch_head));
+	oidclr(&branch_head);
 	remote_head = get_merge_parent(remote);
 	if (!remote_head)
 		die(_("'%s' does not point to a commit"), remote);
 
-	if (dwim_ref(remote, strlen(remote), branch_head, &found_ref) > 0) {
+	if (dwim_ref(remote, strlen(remote), branch_head.hash, &found_ref) > 0) {
 		if (starts_with(found_ref, "refs/heads/")) {
 			strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
-				    sha1_to_hex(branch_head), remote);
+				    oid_to_hex(&branch_head), remote);
 			goto cleanup;
 		}
 		if (starts_with(found_ref, "refs/tags/")) {
 			strbuf_addf(msg, "%s\t\ttag '%s' of .\n",
-				    sha1_to_hex(branch_head), remote);
+				    oid_to_hex(&branch_head), remote);
 			goto cleanup;
 		}
 		if (starts_with(found_ref, "refs/remotes/")) {
 			strbuf_addf(msg, "%s\t\tremote-tracking branch '%s' of .\n",
-				    sha1_to_hex(branch_head), remote);
+				    oid_to_hex(&branch_head), remote);
 			goto cleanup;
 		}
 	}
@@ -590,8 +590,8 @@ static int git_merge_config(const char *k, const char *v, void *cb)
 	return git_diff_ui_config(k, v, cb);
 }
 
-static int read_tree_trivial(unsigned char *common, unsigned char *head,
-			     unsigned char *one)
+static int read_tree_trivial(struct object_id *common, struct object_id *head,
+			     struct object_id *one)
 {
 	int i, nr_trees = 0;
 	struct tree *trees[MAX_UNPACK_TREES];
@@ -606,13 +606,13 @@ static int read_tree_trivial(unsigned char *common, unsigned char *head,
 	opts.verbose_update = 1;
 	opts.trivial_merges_only = 1;
 	opts.merge = 1;
-	trees[nr_trees] = parse_tree_indirect(common);
+	trees[nr_trees] = parse_tree_indirect(common->hash);
 	if (!trees[nr_trees++])
 		return -1;
-	trees[nr_trees] = parse_tree_indirect(head);
+	trees[nr_trees] = parse_tree_indirect(head->hash);
 	if (!trees[nr_trees++])
 		return -1;
-	trees[nr_trees] = parse_tree_indirect(one);
+	trees[nr_trees] = parse_tree_indirect(one->hash);
 	if (!trees[nr_trees++])
 		return -1;
 	opts.fn = threeway_merge;
@@ -626,9 +626,9 @@ static int read_tree_trivial(unsigned char *common, unsigned char *head,
 	return 0;
 }
 
-static void write_tree_trivial(unsigned char *sha1)
+static void write_tree_trivial(struct object_id *oid)
 {
-	if (write_cache_as_tree(sha1, 0, NULL))
+	if (write_cache_as_tree(oid->hash, 0, NULL))
 		die(_("git write-tree failed to write a tree"));
 }
 
@@ -781,7 +781,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)
 
 static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
 {
-	unsigned char result_tree[20], result_commit[20];
+	struct object_id result_tree, result_commit;
 	struct commit_list *parents, **pptr = &parents;
 	static struct lock_file lock;
 
@@ -792,15 +792,15 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
 		return error(_("Unable to write index."));
 	rollback_lock_file(&lock);
 
-	write_tree_trivial(result_tree);
+	write_tree_trivial(&result_tree);
 	printf(_("Wonderful.\n"));
 	pptr = commit_list_append(head, pptr);
 	pptr = commit_list_append(remoteheads->item, pptr);
 	prepare_to_commit(remoteheads);
-	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
-			result_commit, NULL, sign_commit))
+	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree.hash, parents,
+			result_commit.hash, NULL, sign_commit))
 		die(_("failed to write commit object"));
-	finish(head, remoteheads, result_commit, "In-index merge");
+	finish(head, remoteheads, &result_commit, "In-index merge");
 	drop_save();
 	return 0;
 }
@@ -809,12 +809,12 @@ static int finish_automerge(struct commit *head,
 			    int head_subsumed,
 			    struct commit_list *common,
 			    struct commit_list *remoteheads,
-			    unsigned char *result_tree,
+			    struct object_id *result_tree,
 			    const char *wt_strategy)
 {
 	struct commit_list *parents = NULL;
 	struct strbuf buf = STRBUF_INIT;
-	unsigned char result_commit[20];
+	struct object_id result_commit;
 
 	free_commit_list(common);
 	parents = remoteheads;
@@ -822,11 +822,11 @@ static int finish_automerge(struct commit *head,
 		commit_list_insert(head, &parents);
 	strbuf_addch(&merge_msg, '\n');
 	prepare_to_commit(remoteheads);
-	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree, parents,
-			result_commit, NULL, sign_commit))
+	if (commit_tree(merge_msg.buf, merge_msg.len, result_tree->hash, parents,
+			result_commit.hash, NULL, sign_commit))
 		die(_("failed to write commit object"));
 	strbuf_addf(&buf, "Merge made by the '%s' strategy.", wt_strategy);
-	finish(head, remoteheads, result_commit, buf.buf);
+	finish(head, remoteheads, &result_commit, buf.buf);
 	strbuf_release(&buf);
 	drop_save();
 	return 0;
@@ -854,18 +854,18 @@ static int suggest_conflicts(void)
 }
 
 static struct commit *is_old_style_invocation(int argc, const char **argv,
-					      const unsigned char *head)
+					      const struct object_id *head)
 {
 	struct commit *second_token = NULL;
 	if (argc > 2) {
-		unsigned char second_sha1[20];
+		struct object_id second_oid;
 
-		if (get_sha1(argv[1], second_sha1))
+		if (get_oid(argv[1], &second_oid))
 			return NULL;
-		second_token = lookup_commit_reference_gently(second_sha1, 0);
+		second_token = lookup_commit_reference_gently(second_oid.hash, 0);
 		if (!second_token)
 			die(_("'%s' is not a commit"), argv[1]);
-		if (hashcmp(second_token->object.oid.hash, head))
+		if (oidcmp(&second_token->object.oid, head))
 			return NULL;
 	}
 	return second_token;
@@ -1038,7 +1038,7 @@ static void handle_fetch_head(struct commit_list **remotes, struct strbuf *merge
 		die_errno(_("could not close '%s'"), filename);
 
 	for (pos = 0; pos < merge_names->len; pos = npos) {
-		unsigned char sha1[20];
+		struct object_id oid;
 		char *ptr;
 		struct commit *commit;
 
@@ -1048,16 +1048,16 @@ static void handle_fetch_head(struct commit_list **remotes, struct strbuf *merge
 		else
 			npos = merge_names->len;
 
-		if (npos - pos < 40 + 2 ||
-		    get_sha1_hex(merge_names->buf + pos, sha1))
+		if (npos - pos < GIT_SHA1_HEXSZ + 2 ||
+		    get_oid_hex(merge_names->buf + pos, &oid))
 			commit = NULL; /* bad */
-		else if (memcmp(merge_names->buf + pos + 40, "\t\t", 2))
+		else if (memcmp(merge_names->buf + pos + GIT_SHA1_HEXSZ, "\t\t", 2))
 			continue; /* not-for-merge */
 		else {
-			char saved = merge_names->buf[pos + 40];
-			merge_names->buf[pos + 40] = '\0';
+			char saved = merge_names->buf[pos + GIT_SHA1_HEXSZ];
+			merge_names->buf[pos + GIT_SHA1_HEXSZ] = '\0';
 			commit = get_merge_parent(merge_names->buf + pos);
-			merge_names->buf[pos + 40] = saved;
+			merge_names->buf[pos + GIT_SHA1_HEXSZ] = saved;
 		}
 		if (!commit) {
 			if (ptr)
@@ -1117,9 +1117,7 @@ static struct commit_list *collect_parents(struct commit *head_commit,
 
 int cmd_merge(int argc, const char **argv, const char *prefix)
 {
-	unsigned char result_tree[20];
-	unsigned char stash[20];
-	unsigned char head_sha1[20];
+	struct object_id result_tree, stash, head_oid;
 	struct commit *head_commit;
 	struct strbuf buf = STRBUF_INIT;
 	const char *head_arg;
@@ -1138,13 +1136,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 * Check if we are _not_ on a detached HEAD, i.e. if there is a
 	 * current branch.
 	 */
-	branch = branch_to_free = resolve_refdup("HEAD", 0, head_sha1, NULL);
+	branch = branch_to_free = resolve_refdup("HEAD", 0, head_oid.hash, NULL);
 	if (branch && starts_with(branch, "refs/heads/"))
 		branch += 11;
-	if (!branch || is_null_sha1(head_sha1))
+	if (!branch || is_null_oid(&head_oid))
 		head_commit = NULL;
 	else
-		head_commit = lookup_commit_or_die(head_sha1, "HEAD");
+		head_commit = lookup_commit_or_die(head_oid.hash, "HEAD");
 
 	init_diff_ui_defaults();
 	git_config(git_merge_config, NULL);
@@ -1242,7 +1240,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		 * to forbid "git merge" into a branch yet to be born.
 		 * We do the same for "git pull".
 		 */
-		unsigned char *remote_head_sha1;
+		struct object_id *remote_head_oid;
 		if (squash)
 			die(_("Squash commit into empty head not supported yet"));
 		if (fast_forward == FF_NO)
@@ -1254,9 +1252,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			die(_("%s - not something we can merge"), argv[0]);
 		if (remoteheads->next)
 			die(_("Can merge only exactly one commit into empty head"));
-		remote_head_sha1 = remoteheads->item->object.oid.hash;
-		read_empty(remote_head_sha1, 0);
-		update_ref("initial pull", "HEAD", remote_head_sha1,
+		remote_head_oid = &remoteheads->item->object.oid;
+		read_empty(remote_head_oid->hash, 0);
+		update_ref("initial pull", "HEAD", remote_head_oid->hash,
 			   NULL, 0, UPDATE_REFS_DIE_ON_ERR);
 		goto done;
 	}
@@ -1270,7 +1268,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 * additional safety measure to check for it.
 	 */
 	if (!have_message &&
-	    is_old_style_invocation(argc, argv, head_commit->object.oid.hash)) {
+	    is_old_style_invocation(argc, argv, &head_commit->object.oid)) {
 		warning("old-style 'git merge <msg> HEAD <commit>' is deprecated.");
 		strbuf_addstr(&merge_msg, argv[0]);
 		head_arg = argv[1];
@@ -1422,7 +1420,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			goto done;
 		}
 
-		finish(head_commit, remoteheads, commit->object.oid.hash, msg.buf);
+		finish(head_commit, remoteheads, &commit->object.oid, msg.buf);
 		drop_save();
 		goto done;
 	} else if (!remoteheads->next && common->next)
@@ -1441,9 +1439,9 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 			/* See if it is really trivial. */
 			git_committer_info(IDENT_STRICT);
 			printf(_("Trying really trivial in-index merge...\n"));
-			if (!read_tree_trivial(common->item->object.oid.hash,
-					       head_commit->object.oid.hash,
-					       remoteheads->item->object.oid.hash)) {
+			if (!read_tree_trivial(&common->item->object.oid,
+					       &head_commit->object.oid,
+					       &remoteheads->item->object.oid)) {
 				ret = merge_trivial(head_commit, remoteheads);
 				goto done;
 			}
@@ -1495,14 +1493,14 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	    /*
 	     * Stash away the local changes so that we can try more than one.
 	     */
-	    save_state(stash))
-		hashclr(stash);
+	    save_state(&stash))
+		oidclr(&stash);
 
 	for (i = 0; i < use_strategies_nr; i++) {
 		int ret;
 		if (i) {
 			printf(_("Rewinding the tree to pristine...\n"));
-			restore_state(head_commit->object.oid.hash, stash);
+			restore_state(&head_commit->object.oid, &stash);
 		}
 		if (use_strategies_nr != 1)
 			printf(_("Trying merge strategy %s...\n"),
@@ -1547,7 +1545,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		}
 
 		/* Automerge succeeded. */
-		write_tree_trivial(result_tree);
+		write_tree_trivial(&result_tree);
 		automerge_was_ok = 1;
 		break;
 	}
@@ -1559,7 +1557,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	if (automerge_was_ok) {
 		ret = finish_automerge(head_commit, head_subsumed,
 				       common, remoteheads,
-				       result_tree, wt_strategy);
+				       &result_tree, wt_strategy);
 		goto done;
 	}
 
@@ -1568,7 +1566,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 	 * it up.
 	 */
 	if (!best_strategy) {
-		restore_state(head_commit->object.oid.hash, stash);
+		restore_state(&head_commit->object.oid, &stash);
 		if (use_strategies_nr > 1)
 			fprintf(stderr,
 				_("No merge strategy handled the merge.\n"));
@@ -1581,7 +1579,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
 		; /* We already have its result in the working tree. */
 	else {
 		printf(_("Rewinding the tree to pristine...\n"));
-		restore_state(head_commit->object.oid.hash, stash);
+		restore_state(&head_commit->object.oid, &stash);
 		printf(_("Using the %s to prepare resolving by hand.\n"),
 			best_strategy);
 		try_merge_strategy(best_strategy, common, remoteheads,
-- 
2.11.0


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

* [PATCH v3 10/19] Convert remaining callers of resolve_refdup to object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (8 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 09/19] builtin/merge: " brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 11/19] builtin/replace: convert to struct object_id brian m. carlson
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

There are a few leaf functions in various files that call
resolve_refdup.  Convert these functions to use struct object_id
internally to prepare for transitioning resolve_refdup itself.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/notes.c        | 18 +++++++++---------
 builtin/receive-pack.c |  4 ++--
 ref-filter.c           |  4 ++--
 reflog-walk.c          | 12 ++++++------
 transport.c            |  4 ++--
 wt-status.c            |  4 ++--
 6 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/builtin/notes.c b/builtin/notes.c
index 5248a9bad8..8c569a49a0 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -693,7 +693,7 @@ static int merge_abort(struct notes_merge_options *o)
 static int merge_commit(struct notes_merge_options *o)
 {
 	struct strbuf msg = STRBUF_INIT;
-	unsigned char sha1[20], parent_sha1[20];
+	struct object_id oid, parent_oid;
 	struct notes_tree *t;
 	struct commit *partial;
 	struct pretty_print_context pretty_ctx;
@@ -705,27 +705,27 @@ static int merge_commit(struct notes_merge_options *o)
 	 * and target notes ref from .git/NOTES_MERGE_REF.
 	 */
 
-	if (get_sha1("NOTES_MERGE_PARTIAL", sha1))
+	if (get_oid("NOTES_MERGE_PARTIAL", &oid))
 		die(_("failed to read ref NOTES_MERGE_PARTIAL"));
-	else if (!(partial = lookup_commit_reference(sha1)))
+	else if (!(partial = lookup_commit_reference(oid.hash)))
 		die(_("could not find commit from NOTES_MERGE_PARTIAL."));
 	else if (parse_commit(partial))
 		die(_("could not parse commit from NOTES_MERGE_PARTIAL."));
 
 	if (partial->parents)
-		hashcpy(parent_sha1, partial->parents->item->object.oid.hash);
+		oidcpy(&parent_oid, &partial->parents->item->object.oid);
 	else
-		hashclr(parent_sha1);
+		oidclr(&parent_oid);
 
 	t = xcalloc(1, sizeof(struct notes_tree));
 	init_notes(t, "NOTES_MERGE_PARTIAL", combine_notes_overwrite, 0);
 
 	o->local_ref = local_ref_to_free =
-		resolve_refdup("NOTES_MERGE_REF", 0, sha1, NULL);
+		resolve_refdup("NOTES_MERGE_REF", 0, oid.hash, NULL);
 	if (!o->local_ref)
 		die(_("failed to resolve NOTES_MERGE_REF"));
 
-	if (notes_merge_commit(o, t, partial, sha1))
+	if (notes_merge_commit(o, t, partial, oid.hash))
 		die(_("failed to finalize notes merge"));
 
 	/* Reuse existing commit message in reflog message */
@@ -733,8 +733,8 @@ static int merge_commit(struct notes_merge_options *o)
 	format_commit_message(partial, "%s", &msg, &pretty_ctx);
 	strbuf_trim(&msg);
 	strbuf_insert(&msg, 0, "notes: ", 7);
-	update_ref(msg.buf, o->local_ref, sha1,
-		   is_null_sha1(parent_sha1) ? NULL : parent_sha1,
+	update_ref(msg.buf, o->local_ref, oid.hash,
+		   is_null_oid(&parent_oid) ? NULL : parent_oid.hash,
 		   0, UPDATE_REFS_DIE_ON_ERR);
 
 	free_notes(t);
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index 1dbb8a0692..7966f4f4df 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1414,7 +1414,7 @@ static void execute_commands(struct command *commands,
 {
 	struct check_connected_options opt = CHECK_CONNECTED_INIT;
 	struct command *cmd;
-	unsigned char sha1[20];
+	struct object_id oid;
 	struct iterate_data data;
 	struct async muxer;
 	int err_fd = 0;
@@ -1471,7 +1471,7 @@ static void execute_commands(struct command *commands,
 	check_aliased_updates(commands);
 
 	free(head_name_to_free);
-	head_name = head_name_to_free = resolve_refdup("HEAD", 0, sha1, NULL);
+	head_name = head_name_to_free = resolve_refdup("HEAD", 0, oid.hash, NULL);
 
 	if (use_atomic)
 		execute_commands_atomic(commands, si);
diff --git a/ref-filter.c b/ref-filter.c
index 3820b21cc7..f0de30e2ef 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -961,9 +961,9 @@ static void populate_value(struct ref_array_item *ref)
 	ref->value = xcalloc(used_atom_cnt, sizeof(struct atom_value));
 
 	if (need_symref && (ref->flag & REF_ISSYMREF) && !ref->symref) {
-		unsigned char unused1[20];
+		struct object_id unused1;
 		ref->symref = resolve_refdup(ref->refname, RESOLVE_REF_READING,
-					     unused1, NULL);
+					     unused1.hash, NULL);
 		if (!ref->symref)
 			ref->symref = "";
 	}
diff --git a/reflog-walk.c b/reflog-walk.c
index a246af2767..f98748e2ae 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -45,11 +45,11 @@ static struct complete_reflogs *read_complete_reflog(const char *ref)
 	reflogs->ref = xstrdup(ref);
 	for_each_reflog_ent(ref, read_one_reflog, reflogs);
 	if (reflogs->nr == 0) {
-		unsigned char sha1[20];
+		struct object_id oid;
 		const char *name;
 		void *name_to_free;
 		name = name_to_free = resolve_refdup(ref, RESOLVE_REF_READING,
-						     sha1, NULL);
+						     oid.hash, NULL);
 		if (name) {
 			for_each_reflog_ent(name, read_one_reflog, reflogs);
 			free(name_to_free);
@@ -172,18 +172,18 @@ int add_reflog_for_walk(struct reflog_walk_info *info,
 		reflogs = item->util;
 	else {
 		if (*branch == '\0') {
-			unsigned char sha1[20];
+			struct object_id oid;
 			free(branch);
-			branch = resolve_refdup("HEAD", 0, sha1, NULL);
+			branch = resolve_refdup("HEAD", 0, oid.hash, NULL);
 			if (!branch)
 				die ("No current branch");
 
 		}
 		reflogs = read_complete_reflog(branch);
 		if (!reflogs || reflogs->nr == 0) {
-			unsigned char sha1[20];
+			struct object_id oid;
 			char *b;
-			if (dwim_log(branch, strlen(branch), sha1, &b) == 1) {
+			if (dwim_log(branch, strlen(branch), oid.hash, &b) == 1) {
 				if (reflogs) {
 					free(reflogs->ref);
 					free(reflogs);
diff --git a/transport.c b/transport.c
index d72e089484..141af31e8e 100644
--- a/transport.c
+++ b/transport.c
@@ -467,11 +467,11 @@ void transport_print_push_status(const char *dest, struct ref *refs,
 {
 	struct ref *ref;
 	int n = 0;
-	unsigned char head_sha1[20];
+	struct object_id head_oid;
 	char *head;
 	int summary_width = transport_summary_width(refs);
 
-	head = resolve_refdup("HEAD", RESOLVE_REF_READING, head_sha1, NULL);
+	head = resolve_refdup("HEAD", RESOLVE_REF_READING, head_oid.hash, NULL);
 
 	if (verbose) {
 		for (ref = refs; ref; ref = ref->next)
diff --git a/wt-status.c b/wt-status.c
index d47012048f..0ec090a338 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -121,7 +121,7 @@ static void status_printf_more(struct wt_status *s, const char *color,
 
 void wt_status_prepare(struct wt_status *s)
 {
-	unsigned char sha1[20];
+	struct object_id oid;
 
 	memset(s, 0, sizeof(*s));
 	memcpy(s->color_palette, default_wt_status_colors,
@@ -129,7 +129,7 @@ void wt_status_prepare(struct wt_status *s)
 	s->show_untracked_files = SHOW_NORMAL_UNTRACKED_FILES;
 	s->use_color = -1;
 	s->relative_paths = 1;
-	s->branch = resolve_refdup("HEAD", 0, sha1, NULL);
+	s->branch = resolve_refdup("HEAD", 0, oid.hash, NULL);
 	s->reference = "HEAD";
 	s->fp = stdout;
 	s->index_file = get_index_file();
-- 
2.11.0


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

* [PATCH v3 11/19] builtin/replace: convert to struct object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (9 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 10/19] Convert remaining callers of resolve_refdup to object_id brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 12/19] reflog-walk: convert struct reflog_info " brian m. carlson
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

Convert various uses of unsigned char [20] to struct object_id.  Rename
replace_object_sha1 to replace_object_oid.  Finally, specify a constant
in terms of GIT_SHA1_HEXSZ.

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

diff --git a/builtin/replace.c b/builtin/replace.c
index b58c714cb8..f7716a5472 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -88,78 +88,78 @@ static int list_replace_refs(const char *pattern, const char *format)
 }
 
 typedef int (*each_replace_name_fn)(const char *name, const char *ref,
-				    const unsigned char *sha1);
+				    const struct object_id *oid);
 
 static int for_each_replace_name(const char **argv, each_replace_name_fn fn)
 {
 	const char **p, *full_hex;
 	char ref[PATH_MAX];
 	int had_error = 0;
-	unsigned char sha1[20];
+	struct object_id oid;
 
 	for (p = argv; *p; p++) {
-		if (get_sha1(*p, sha1)) {
+		if (get_oid(*p, &oid)) {
 			error("Failed to resolve '%s' as a valid ref.", *p);
 			had_error = 1;
 			continue;
 		}
-		full_hex = sha1_to_hex(sha1);
+		full_hex = oid_to_hex(&oid);
 		snprintf(ref, sizeof(ref), "%s%s", git_replace_ref_base, full_hex);
 		/* read_ref() may reuse the buffer */
 		full_hex = ref + strlen(git_replace_ref_base);
-		if (read_ref(ref, sha1)) {
+		if (read_ref(ref, oid.hash)) {
 			error("replace ref '%s' not found.", full_hex);
 			had_error = 1;
 			continue;
 		}
-		if (fn(full_hex, ref, sha1))
+		if (fn(full_hex, ref, &oid))
 			had_error = 1;
 	}
 	return had_error;
 }
 
 static int delete_replace_ref(const char *name, const char *ref,
-			      const unsigned char *sha1)
+			      const struct object_id *oid)
 {
-	if (delete_ref(ref, sha1, 0))
+	if (delete_ref(ref, oid->hash, 0))
 		return 1;
 	printf("Deleted replace ref '%s'\n", name);
 	return 0;
 }
 
-static void check_ref_valid(unsigned char object[20],
-			    unsigned char prev[20],
+static void check_ref_valid(struct object_id *object,
+			    struct object_id *prev,
 			    char *ref,
 			    int ref_size,
 			    int force)
 {
 	if (snprintf(ref, ref_size,
 		     "%s%s", git_replace_ref_base,
-		     sha1_to_hex(object)) > ref_size - 1)
+		     oid_to_hex(object)) > ref_size - 1)
 		die("replace ref name too long: %.*s...", 50, ref);
 	if (check_refname_format(ref, 0))
 		die("'%s' is not a valid ref name.", ref);
 
-	if (read_ref(ref, prev))
-		hashclr(prev);
+	if (read_ref(ref, prev->hash))
+		oidclr(prev);
 	else if (!force)
 		die("replace ref '%s' already exists", ref);
 }
 
-static int replace_object_sha1(const char *object_ref,
-			       unsigned char object[20],
+static int replace_object_oid(const char *object_ref,
+			       struct object_id *object,
 			       const char *replace_ref,
-			       unsigned char repl[20],
+			       struct object_id *repl,
 			       int force)
 {
-	unsigned char prev[20];
+	struct object_id prev;
 	enum object_type obj_type, repl_type;
 	char ref[PATH_MAX];
 	struct ref_transaction *transaction;
 	struct strbuf err = STRBUF_INIT;
 
-	obj_type = sha1_object_info(object, NULL);
-	repl_type = sha1_object_info(repl, NULL);
+	obj_type = sha1_object_info(object->hash, NULL);
+	repl_type = sha1_object_info(repl->hash, NULL);
 	if (!force && obj_type != repl_type)
 		die("Objects must be of the same type.\n"
 		    "'%s' points to a replaced object of type '%s'\n"
@@ -167,11 +167,11 @@ static int replace_object_sha1(const char *object_ref,
 		    object_ref, typename(obj_type),
 		    replace_ref, typename(repl_type));
 
-	check_ref_valid(object, prev, ref, sizeof(ref), force);
+	check_ref_valid(object, &prev, ref, sizeof(ref), force);
 
 	transaction = ref_transaction_begin(&err);
 	if (!transaction ||
-	    ref_transaction_update(transaction, ref, repl, prev,
+	    ref_transaction_update(transaction, ref, repl->hash, prev.hash,
 				   0, NULL, &err) ||
 	    ref_transaction_commit(transaction, &err))
 		die("%s", err.buf);
@@ -182,14 +182,14 @@ static int replace_object_sha1(const char *object_ref,
 
 static int replace_object(const char *object_ref, const char *replace_ref, int force)
 {
-	unsigned char object[20], repl[20];
+	struct object_id object, repl;
 
-	if (get_sha1(object_ref, object))
+	if (get_oid(object_ref, &object))
 		die("Failed to resolve '%s' as a valid ref.", object_ref);
-	if (get_sha1(replace_ref, repl))
+	if (get_oid(replace_ref, &repl))
 		die("Failed to resolve '%s' as a valid ref.", replace_ref);
 
-	return replace_object_sha1(object_ref, object, replace_ref, repl, force);
+	return replace_object_oid(object_ref, &object, replace_ref, &repl, force);
 }
 
 /*
@@ -197,7 +197,7 @@ static int replace_object(const char *object_ref, const char *replace_ref, int f
  * If "raw" is true, then the object's raw contents are printed according to
  * "type". Otherwise, we pretty-print the contents for human editing.
  */
-static void export_object(const unsigned char *sha1, enum object_type type,
+static void export_object(const struct object_id *oid, enum object_type type,
 			  int raw, const char *filename)
 {
 	struct child_process cmd = CHILD_PROCESS_INIT;
@@ -213,7 +213,7 @@ static void export_object(const unsigned char *sha1, enum object_type type,
 		argv_array_push(&cmd.args, typename(type));
 	else
 		argv_array_push(&cmd.args, "-p");
-	argv_array_push(&cmd.args, sha1_to_hex(sha1));
+	argv_array_push(&cmd.args, oid_to_hex(oid));
 	cmd.git_cmd = 1;
 	cmd.out = fd;
 
@@ -226,7 +226,7 @@ static void export_object(const unsigned char *sha1, enum object_type type,
  * interpreting it as "type", and writing the result to the object database.
  * The sha1 of the written object is returned via sha1.
  */
-static void import_object(unsigned char *sha1, enum object_type type,
+static void import_object(struct object_id *oid, enum object_type type,
 			  int raw, const char *filename)
 {
 	int fd;
@@ -254,7 +254,7 @@ static void import_object(unsigned char *sha1, enum object_type type,
 
 		if (finish_command(&cmd))
 			die("mktree reported failure");
-		if (get_sha1_hex(result.buf, sha1) < 0)
+		if (get_oid_hex(result.buf, oid) < 0)
 			die("mktree did not return an object name");
 
 		strbuf_release(&result);
@@ -264,7 +264,7 @@ static void import_object(unsigned char *sha1, enum object_type type,
 
 		if (fstat(fd, &st) < 0)
 			die_errno("unable to fstat %s", filename);
-		if (index_fd(sha1, fd, &st, type, NULL, flags) < 0)
+		if (index_fd(oid->hash, fd, &st, type, NULL, flags) < 0)
 			die("unable to write object to database");
 		/* index_fd close()s fd for us */
 	}
@@ -279,29 +279,29 @@ static int edit_and_replace(const char *object_ref, int force, int raw)
 {
 	char *tmpfile = git_pathdup("REPLACE_EDITOBJ");
 	enum object_type type;
-	unsigned char old[20], new[20], prev[20];
+	struct object_id old, new, prev;
 	char ref[PATH_MAX];
 
-	if (get_sha1(object_ref, old) < 0)
+	if (get_oid(object_ref, &old) < 0)
 		die("Not a valid object name: '%s'", object_ref);
 
-	type = sha1_object_info(old, NULL);
+	type = sha1_object_info(old.hash, NULL);
 	if (type < 0)
-		die("unable to get object type for %s", sha1_to_hex(old));
+		die("unable to get object type for %s", oid_to_hex(&old));
 
-	check_ref_valid(old, prev, ref, sizeof(ref), force);
+	check_ref_valid(&old, &prev, ref, sizeof(ref), force);
 
-	export_object(old, type, raw, tmpfile);
+	export_object(&old, type, raw, tmpfile);
 	if (launch_editor(tmpfile, NULL, NULL) < 0)
 		die("editing object file failed");
-	import_object(new, type, raw, tmpfile);
+	import_object(&new, type, raw, tmpfile);
 
 	free(tmpfile);
 
-	if (!hashcmp(old, new))
-		return error("new object is the same as the old one: '%s'", sha1_to_hex(old));
+	if (!oidcmp(&old, &new))
+		return error("new object is the same as the old one: '%s'", oid_to_hex(&old));
 
-	return replace_object_sha1(object_ref, old, "replacement", new, force);
+	return replace_object_oid(object_ref, &old, "replacement", &new, force);
 }
 
 static void replace_parents(struct strbuf *buf, int argc, const char **argv)
@@ -312,7 +312,7 @@ static void replace_parents(struct strbuf *buf, int argc, const char **argv)
 
 	/* find existing parents */
 	parent_start = buf->buf;
-	parent_start += 46; /* "tree " + "hex sha1" + "\n" */
+	parent_start += GIT_SHA1_HEXSZ + 6; /* "tree " + "hex sha1" + "\n" */
 	parent_end = parent_start;
 
 	while (starts_with(parent_end, "parent "))
@@ -320,11 +320,11 @@ static void replace_parents(struct strbuf *buf, int argc, const char **argv)
 
 	/* prepare new parents */
 	for (i = 0; i < argc; i++) {
-		unsigned char sha1[20];
-		if (get_sha1(argv[i], sha1) < 0)
+		struct object_id oid;
+		if (get_oid(argv[i], &oid) < 0)
 			die(_("Not a valid object name: '%s'"), argv[i]);
-		lookup_commit_or_die(sha1, argv[i]);
-		strbuf_addf(&new_parents, "parent %s\n", sha1_to_hex(sha1));
+		lookup_commit_or_die(oid.hash, argv[i]);
+		strbuf_addf(&new_parents, "parent %s\n", oid_to_hex(&oid));
 	}
 
 	/* replace existing parents with new ones */
@@ -345,12 +345,12 @@ static void check_one_mergetag(struct commit *commit,
 {
 	struct check_mergetag_data *mergetag_data = (struct check_mergetag_data *)data;
 	const char *ref = mergetag_data->argv[0];
-	unsigned char tag_sha1[20];
+	struct object_id tag_oid;
 	struct tag *tag;
 	int i;
 
-	hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), tag_sha1);
-	tag = lookup_tag(tag_sha1);
+	hash_sha1_file(extra->value, extra->len, typename(OBJ_TAG), tag_oid.hash);
+	tag = lookup_tag(tag_oid.hash);
 	if (!tag)
 		die(_("bad mergetag in commit '%s'"), ref);
 	if (parse_tag_buffer(tag, extra->value, extra->len))
@@ -366,7 +366,7 @@ static void check_one_mergetag(struct commit *commit,
 	}
 
 	die(_("original commit '%s' contains mergetag '%s' that is discarded; "
-	      "use --edit instead of --graft"), ref, sha1_to_hex(tag_sha1));
+	      "use --edit instead of --graft"), ref, oid_to_hex(&tag_oid));
 }
 
 static void check_mergetags(struct commit *commit, int argc, const char **argv)
@@ -380,16 +380,16 @@ static void check_mergetags(struct commit *commit, int argc, const char **argv)
 
 static int create_graft(int argc, const char **argv, int force)
 {
-	unsigned char old[20], new[20];
+	struct object_id old, new;
 	const char *old_ref = argv[0];
 	struct commit *commit;
 	struct strbuf buf = STRBUF_INIT;
 	const char *buffer;
 	unsigned long size;
 
-	if (get_sha1(old_ref, old) < 0)
+	if (get_oid(old_ref, &old) < 0)
 		die(_("Not a valid object name: '%s'"), old_ref);
-	commit = lookup_commit_or_die(old, old_ref);
+	commit = lookup_commit_or_die(old.hash, old_ref);
 
 	buffer = get_commit_buffer(commit, &size);
 	strbuf_add(&buf, buffer, size);
@@ -404,15 +404,15 @@ static int create_graft(int argc, const char **argv, int force)
 
 	check_mergetags(commit, argc, argv);
 
-	if (write_sha1_file(buf.buf, buf.len, commit_type, new))
+	if (write_sha1_file(buf.buf, buf.len, commit_type, new.hash))
 		die(_("could not write replacement commit for: '%s'"), old_ref);
 
 	strbuf_release(&buf);
 
-	if (!hashcmp(old, new))
-		return error("new commit is the same as the old one: '%s'", sha1_to_hex(old));
+	if (!oidcmp(&old, &new))
+		return error("new commit is the same as the old one: '%s'", oid_to_hex(&old));
 
-	return replace_object_sha1(old_ref, old, "replacement", new, force);
+	return replace_object_oid(old_ref, &old, "replacement", &new, force);
 }
 
 int cmd_replace(int argc, const char **argv, const char *prefix)
-- 
2.11.0


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

* [PATCH v3 12/19] reflog-walk: convert struct reflog_info to struct object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (10 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 11/19] builtin/replace: convert to struct object_id brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 13/19] refs: convert each_reflog_ent_fn " brian m. carlson
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

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

@@
struct reflog_info E1;
@@
- E1.osha1
+ E1.ooid.hash

@@
struct reflog_info *E1;
@@
- E1->osha1
+ E1->ooid.hash

@@
struct reflog_info E1;
@@
- E1.nsha1
+ E1.noid.hash

@@
struct reflog_info *E1;
@@
- E1->nsha1
+ E1->noid.hash

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

diff --git a/reflog-walk.c b/reflog-walk.c
index f98748e2ae..fe5be41471 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -10,7 +10,7 @@ struct complete_reflogs {
 	char *ref;
 	const char *short_ref;
 	struct reflog_info {
-		unsigned char osha1[20], nsha1[20];
+		struct object_id ooid, noid;
 		char *email;
 		unsigned long timestamp;
 		int tz;
@@ -28,8 +28,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1,
 
 	ALLOC_GROW(array->items, array->nr + 1, array->alloc);
 	item = array->items + array->nr;
-	hashcpy(item->osha1, osha1);
-	hashcpy(item->nsha1, nsha1);
+	hashcpy(item->ooid.hash, osha1);
+	hashcpy(item->noid.hash, nsha1);
 	item->email = xstrdup(email);
 	item->timestamp = timestamp;
 	item->tz = tz;
@@ -238,13 +238,13 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
 	do {
 		reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
 		commit_reflog->recno--;
-		logobj = parse_object(reflog->osha1);
+		logobj = parse_object(reflog->ooid.hash);
 	} while (commit_reflog->recno && (logobj && logobj->type != OBJ_COMMIT));
 
-	if (!logobj && commit_reflog->recno >= 0 && is_null_sha1(reflog->osha1)) {
+	if (!logobj && commit_reflog->recno >= 0 && is_null_sha1(reflog->ooid.hash)) {
 		/* a root commit, but there are still more entries to show */
 		reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
-		logobj = parse_object(reflog->nsha1);
+		logobj = parse_object(reflog->noid.hash);
 	}
 
 	if (!logobj || logobj->type != OBJ_COMMIT) {
-- 
2.11.0


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

* [PATCH v3 13/19] refs: convert each_reflog_ent_fn to struct object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (11 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 12/19] reflog-walk: convert struct reflog_info " brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 14/19] hex: introduce parse_oid_hex brian m. carlson
                   ` (5 subsequent siblings)
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

Make each_reflog_ent_fn take two struct object_id pointers instead of
two pointers to unsigned char.  Convert the various callbacks to use
struct object_id as well.  Also, rename fsck_handle_reflog_sha1 to
fsck_handle_reflog_oid.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/fsck.c       | 16 ++++++++--------
 builtin/merge-base.c |  6 +++---
 builtin/reflog.c     |  2 +-
 reflog-walk.c        |  6 +++---
 refs.c               | 24 ++++++++++++------------
 refs.h               |  2 +-
 refs/files-backend.c | 24 ++++++++++++------------
 revision.c           | 12 ++++++------
 sha1_name.c          |  2 +-
 wt-status.c          |  6 +++---
 10 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 1a5caccd0f..9b37606858 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -396,13 +396,13 @@ static int fsck_obj_buffer(const unsigned char *sha1, enum object_type type,
 
 static int default_refs;
 
-static void fsck_handle_reflog_sha1(const char *refname, unsigned char *sha1,
+static void fsck_handle_reflog_oid(const char *refname, struct object_id *oid,
 	unsigned long timestamp)
 {
 	struct object *obj;
 
-	if (!is_null_sha1(sha1)) {
-		obj = lookup_object(sha1);
+	if (!is_null_oid(oid)) {
+		obj = lookup_object(oid->hash);
 		if (obj && (obj->flags & HAS_OBJ)) {
 			if (timestamp && name_objects)
 				add_decoration(fsck_walk_options.object_names,
@@ -411,13 +411,13 @@ static void fsck_handle_reflog_sha1(const char *refname, unsigned char *sha1,
 			obj->used = 1;
 			mark_object_reachable(obj);
 		} else {
-			error("%s: invalid reflog entry %s", refname, sha1_to_hex(sha1));
+			error("%s: invalid reflog entry %s", refname, oid_to_hex(oid));
 			errors_found |= ERROR_REACHABLE;
 		}
 	}
 }
 
-static int fsck_handle_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id *noid,
 		const char *email, unsigned long timestamp, int tz,
 		const char *message, void *cb_data)
 {
@@ -425,10 +425,10 @@ static int fsck_handle_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 
 	if (verbose)
 		fprintf(stderr, "Checking reflog %s->%s\n",
-			sha1_to_hex(osha1), sha1_to_hex(nsha1));
+			oid_to_hex(ooid), oid_to_hex(noid));
 
-	fsck_handle_reflog_sha1(refname, osha1, 0);
-	fsck_handle_reflog_sha1(refname, nsha1, timestamp);
+	fsck_handle_reflog_oid(refname, ooid, 0);
+	fsck_handle_reflog_oid(refname, noid, timestamp);
 	return 0;
 }
 
diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index b572a37c26..db95bc29cf 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -131,7 +131,7 @@ static void add_one_commit(unsigned char *sha1, struct rev_collect *revs)
 	commit->object.flags |= TMP_MARK;
 }
 
-static int collect_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int collect_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
 				  const char *ident, unsigned long timestamp,
 				  int tz, const char *message, void *cbdata)
 {
@@ -139,9 +139,9 @@ static int collect_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 
 	if (revs->initial) {
 		revs->initial = 0;
-		add_one_commit(osha1, revs);
+		add_one_commit(ooid->hash, revs);
 	}
-	add_one_commit(nsha1, revs);
+	add_one_commit(noid->hash, revs);
 	return 0;
 }
 
diff --git a/builtin/reflog.c b/builtin/reflog.c
index 7a7136e53e..7472775778 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -615,7 +615,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
 	return status;
 }
 
-static int count_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int count_reflog_ent(struct object_id *ooid, struct object_id *noid,
 		const char *email, unsigned long timestamp, int tz,
 		const char *message, void *cb_data)
 {
diff --git a/reflog-walk.c b/reflog-walk.c
index fe5be41471..99679f5825 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -19,7 +19,7 @@ struct complete_reflogs {
 	int nr, alloc;
 };
 
-static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1,
+static int read_one_reflog(struct object_id *ooid, struct object_id *noid,
 		const char *email, unsigned long timestamp, int tz,
 		const char *message, void *cb_data)
 {
@@ -28,8 +28,8 @@ static int read_one_reflog(unsigned char *osha1, unsigned char *nsha1,
 
 	ALLOC_GROW(array->items, array->nr + 1, array->alloc);
 	item = array->items + array->nr;
-	hashcpy(item->ooid.hash, osha1);
-	hashcpy(item->noid.hash, nsha1);
+	oidcpy(&item->ooid, ooid);
+	oidcpy(&item->noid, noid);
 	item->email = xstrdup(email);
 	item->timestamp = timestamp;
 	item->tz = tz;
diff --git a/refs.c b/refs.c
index cd36b64ed9..b900626d3f 100644
--- a/refs.c
+++ b/refs.c
@@ -674,7 +674,7 @@ struct read_ref_at_cb {
 	int *cutoff_cnt;
 };
 
-static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
+static int read_ref_at_ent(struct object_id *ooid, struct object_id *noid,
 		const char *email, unsigned long timestamp, int tz,
 		const char *message, void *cb_data)
 {
@@ -698,30 +698,30 @@ static int read_ref_at_ent(unsigned char *osha1, unsigned char *nsha1,
 		 * hold the values for the previous record.
 		 */
 		if (!is_null_sha1(cb->osha1)) {
-			hashcpy(cb->sha1, nsha1);
-			if (hashcmp(cb->osha1, nsha1))
+			hashcpy(cb->sha1, noid->hash);
+			if (hashcmp(cb->osha1, noid->hash))
 				warning("Log for ref %s has gap after %s.",
 					cb->refname, show_date(cb->date, cb->tz, DATE_MODE(RFC2822)));
 		}
 		else if (cb->date == cb->at_time)
-			hashcpy(cb->sha1, nsha1);
-		else if (hashcmp(nsha1, cb->sha1))
+			hashcpy(cb->sha1, noid->hash);
+		else if (hashcmp(noid->hash, cb->sha1))
 			warning("Log for ref %s unexpectedly ended on %s.",
 				cb->refname, show_date(cb->date, cb->tz,
 						       DATE_MODE(RFC2822)));
-		hashcpy(cb->osha1, osha1);
-		hashcpy(cb->nsha1, nsha1);
+		hashcpy(cb->osha1, ooid->hash);
+		hashcpy(cb->nsha1, noid->hash);
 		cb->found_it = 1;
 		return 1;
 	}
-	hashcpy(cb->osha1, osha1);
-	hashcpy(cb->nsha1, nsha1);
+	hashcpy(cb->osha1, ooid->hash);
+	hashcpy(cb->nsha1, noid->hash);
 	if (cb->cnt > 0)
 		cb->cnt--;
 	return 0;
 }
 
-static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
+static int read_ref_at_ent_oldest(struct object_id *ooid, struct object_id *noid,
 				  const char *email, unsigned long timestamp,
 				  int tz, const char *message, void *cb_data)
 {
@@ -735,9 +735,9 @@ static int read_ref_at_ent_oldest(unsigned char *osha1, unsigned char *nsha1,
 		*cb->cutoff_tz = tz;
 	if (cb->cutoff_cnt)
 		*cb->cutoff_cnt = cb->reccnt;
-	hashcpy(cb->sha1, osha1);
+	hashcpy(cb->sha1, ooid->hash);
 	if (is_null_sha1(cb->sha1))
-		hashcpy(cb->sha1, nsha1);
+		hashcpy(cb->sha1, noid->hash);
 	/* We just want the first entry */
 	return 1;
 }
diff --git a/refs.h b/refs.h
index 9fbff90e79..a075117a2a 100644
--- a/refs.h
+++ b/refs.h
@@ -292,7 +292,7 @@ int delete_reflog(const char *refname);
 
 /* iterate over reflog entries */
 typedef int each_reflog_ent_fn(
-		unsigned char *old_sha1, unsigned char *new_sha1,
+		struct object_id *old_oid, struct object_id *new_oid,
 		const char *committer, unsigned long timestamp,
 		int tz, const char *msg, void *cb_data);
 
diff --git a/refs/files-backend.c b/refs/files-backend.c
index c041d4ba21..d7a5fd2a7c 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3113,15 +3113,15 @@ static int files_delete_reflog(struct ref_store *ref_store,
 
 static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *cb_data)
 {
-	unsigned char osha1[20], nsha1[20];
+	struct object_id ooid, noid;
 	char *email_end, *message;
 	unsigned long timestamp;
 	int tz;
 
 	/* old SP new SP name <email> SP time TAB msg LF */
 	if (sb->len < 83 || sb->buf[sb->len - 1] != '\n' ||
-	    get_sha1_hex(sb->buf, osha1) || sb->buf[40] != ' ' ||
-	    get_sha1_hex(sb->buf + 41, nsha1) || sb->buf[81] != ' ' ||
+	    get_oid_hex(sb->buf, &ooid) || sb->buf[40] != ' ' ||
+	    get_oid_hex(sb->buf + 41, &noid) || sb->buf[81] != ' ' ||
 	    !(email_end = strchr(sb->buf + 82, '>')) ||
 	    email_end[1] != ' ' ||
 	    !(timestamp = strtoul(email_end + 2, &message, 10)) ||
@@ -3136,7 +3136,7 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c
 		message += 6;
 	else
 		message += 7;
-	return fn(osha1, nsha1, sb->buf + 82, timestamp, tz, message, cb_data);
+	return fn(&ooid, &noid, sb->buf + 82, timestamp, tz, message, cb_data);
 }
 
 static char *find_beginning_of_line(char *bob, char *scan)
@@ -3936,10 +3936,10 @@ struct expire_reflog_cb {
 	reflog_expiry_should_prune_fn *should_prune_fn;
 	void *policy_cb;
 	FILE *newlog;
-	unsigned char last_kept_sha1[20];
+	struct object_id last_kept_oid;
 };
 
-static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
 			     const char *email, unsigned long timestamp, int tz,
 			     const char *message, void *cb_data)
 {
@@ -3947,9 +3947,9 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 	struct expire_reflog_policy_cb *policy_cb = cb->policy_cb;
 
 	if (cb->flags & EXPIRE_REFLOGS_REWRITE)
-		osha1 = cb->last_kept_sha1;
+		ooid = &cb->last_kept_oid;
 
-	if ((*cb->should_prune_fn)(osha1, nsha1, email, timestamp, tz,
+	if ((*cb->should_prune_fn)(ooid->hash, noid->hash, email, timestamp, tz,
 				   message, policy_cb)) {
 		if (!cb->newlog)
 			printf("would prune %s", message);
@@ -3958,9 +3958,9 @@ static int expire_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
 	} else {
 		if (cb->newlog) {
 			fprintf(cb->newlog, "%s %s %s %lu %+05d\t%s",
-				sha1_to_hex(osha1), sha1_to_hex(nsha1),
+				oid_to_hex(ooid), oid_to_hex(noid),
 				email, timestamp, tz, message);
-			hashcpy(cb->last_kept_sha1, nsha1);
+			oidcpy(&cb->last_kept_oid, noid);
 		}
 		if (cb->flags & EXPIRE_REFLOGS_VERBOSE)
 			printf("keep %s", message);
@@ -4047,14 +4047,14 @@ static int files_reflog_expire(struct ref_store *ref_store,
 		 */
 		int update = (flags & EXPIRE_REFLOGS_UPDATE_REF) &&
 			!(type & REF_ISSYMREF) &&
-			!is_null_sha1(cb.last_kept_sha1);
+			!is_null_oid(&cb.last_kept_oid);
 
 		if (close_lock_file(&reflog_lock)) {
 			status |= error("couldn't write %s: %s", log_file,
 					strerror(errno));
 		} else if (update &&
 			   (write_in_full(get_lock_file_fd(lock->lk),
-				sha1_to_hex(cb.last_kept_sha1), 40) != 40 ||
+				oid_to_hex(&cb.last_kept_oid), GIT_SHA1_HEXSZ) != GIT_SHA1_HEXSZ ||
 			    write_str_in_full(get_lock_file_fd(lock->lk), "\n") != 1 ||
 			    close_ref(lock) < 0)) {
 			status |= error("couldn't write %s",
diff --git a/revision.c b/revision.c
index b37dbec378..d9fe73318a 100644
--- a/revision.c
+++ b/revision.c
@@ -1196,11 +1196,11 @@ static void handle_refs(const char *submodule, struct rev_info *revs, unsigned f
 	for_each(submodule, handle_one_ref, &cb);
 }
 
-static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
+static void handle_one_reflog_commit(struct object_id *oid, void *cb_data)
 {
 	struct all_refs_cb *cb = cb_data;
-	if (!is_null_sha1(sha1)) {
-		struct object *o = parse_object(sha1);
+	if (!is_null_oid(oid)) {
+		struct object *o = parse_object(oid->hash);
 		if (o) {
 			o->flags |= cb->all_flags;
 			/* ??? CMDLINEFLAGS ??? */
@@ -1214,12 +1214,12 @@ static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
 	}
 }
 
-static int handle_one_reflog_ent(unsigned char *osha1, unsigned char *nsha1,
+static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,
 		const char *email, unsigned long timestamp, int tz,
 		const char *message, void *cb_data)
 {
-	handle_one_reflog_commit(osha1, cb_data);
-	handle_one_reflog_commit(nsha1, cb_data);
+	handle_one_reflog_commit(ooid, cb_data);
+	handle_one_reflog_commit(noid, cb_data);
 	return 0;
 }
 
diff --git a/sha1_name.c b/sha1_name.c
index 73a915ff1b..744e9f884a 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1051,7 +1051,7 @@ struct grab_nth_branch_switch_cbdata {
 	struct strbuf buf;
 };
 
-static int grab_nth_branch_switch(unsigned char *osha1, unsigned char *nsha1,
+static int grab_nth_branch_switch(struct object_id *ooid, struct object_id *noid,
 				  const char *email, unsigned long timestamp, int tz,
 				  const char *message, void *cb_data)
 {
diff --git a/wt-status.c b/wt-status.c
index 0ec090a338..5fac8437b0 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1373,7 +1373,7 @@ struct grab_1st_switch_cbdata {
 	unsigned char nsha1[20];
 };
 
-static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
+static int grab_1st_switch(struct object_id *ooid, struct object_id *noid,
 			   const char *email, unsigned long timestamp, int tz,
 			   const char *message, void *cb_data)
 {
@@ -1387,13 +1387,13 @@ static int grab_1st_switch(unsigned char *osha1, unsigned char *nsha1,
 		return 0;
 	target += strlen(" to ");
 	strbuf_reset(&cb->buf);
-	hashcpy(cb->nsha1, nsha1);
+	hashcpy(cb->nsha1, noid->hash);
 	end = strchrnul(target, '\n');
 	strbuf_add(&cb->buf, target, end - target);
 	if (!strcmp(cb->buf.buf, "HEAD")) {
 		/* HEAD is relative. Resolve it to the right reflog entry. */
 		strbuf_reset(&cb->buf);
-		strbuf_add_unique_abbrev(&cb->buf, nsha1, DEFAULT_ABBREV);
+		strbuf_add_unique_abbrev(&cb->buf, noid->hash, DEFAULT_ABBREV);
 	}
 	return 1;
 }
-- 
2.11.0


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

* [PATCH v3 14/19] hex: introduce parse_oid_hex
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (12 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 13/19] refs: convert each_reflog_ent_fn " brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 15/19] refs: simplify parsing of reflog entries brian m. carlson
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

Introduce a function, parse_oid_hex, which parses a hexadecimal object
ID and if successful, sets a pointer to just beyond the last character.
This allows for simpler, more robust parsing without needing to
hard-code integer values throughout the codebase.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 cache.h | 9 +++++++++
 hex.c   | 8 ++++++++
 2 files changed, 17 insertions(+)

diff --git a/cache.h b/cache.h
index 61fc86e6d7..e03a672d15 100644
--- a/cache.h
+++ b/cache.h
@@ -1319,6 +1319,15 @@ extern char *oid_to_hex_r(char *out, const struct object_id *oid);
 extern char *sha1_to_hex(const unsigned char *sha1);	/* static buffer result! */
 extern char *oid_to_hex(const struct object_id *oid);	/* same static buffer as sha1_to_hex */
 
+/*
+ * Parse a 40-character hexadecimal object ID starting from hex, updating the
+ * pointer specified by end when parsing stops.  The resulting object ID is
+ * stored in oid.  Returns 0 on success.  Parsing will stop on the first NUL or
+ * other invalid character.  end is only updated on success; otherwise, it is
+ * unmodified.
+ */
+extern int parse_oid_hex(const char *hex, struct object_id *oid, const char **end);
+
 extern int interpret_branch_name(const char *str, int len, struct strbuf *);
 extern int get_oid_mb(const char *str, struct object_id *oid);
 
diff --git a/hex.c b/hex.c
index 845b01a874..eab7b626ee 100644
--- a/hex.c
+++ b/hex.c
@@ -53,6 +53,14 @@ int get_oid_hex(const char *hex, struct object_id *oid)
 	return get_sha1_hex(hex, oid->hash);
 }
 
+int parse_oid_hex(const char *hex, struct object_id *oid, const char **end)
+{
+	int ret = get_oid_hex(hex, oid);
+	if (!ret)
+		*end = hex + GIT_SHA1_HEXSZ;
+	return ret;
+}
+
 char *sha1_to_hex_r(char *buffer, const unsigned char *sha1)
 {
 	static const char hex[] = "0123456789abcdef";
-- 
2.11.0


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

* [PATCH v3 15/19] refs: simplify parsing of reflog entries
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (13 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 14/19] hex: introduce parse_oid_hex brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 16/19] sha1_file: introduce an nth_packed_object_oid function brian m. carlson
                   ` (3 subsequent siblings)
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

The current code for reflog entries uses a lot of hard-coded constants,
making it hard to read and modify.  Use parse_oid_hex and two temporary
variables to simplify the code and reduce the use of magic constants.

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

diff --git a/refs/files-backend.c b/refs/files-backend.c
index d7a5fd2a7c..fea20e99fe 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -3117,12 +3117,13 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c
 	char *email_end, *message;
 	unsigned long timestamp;
 	int tz;
+	const char *p = sb->buf;
 
 	/* old SP new SP name <email> SP time TAB msg LF */
-	if (sb->len < 83 || sb->buf[sb->len - 1] != '\n' ||
-	    get_oid_hex(sb->buf, &ooid) || sb->buf[40] != ' ' ||
-	    get_oid_hex(sb->buf + 41, &noid) || sb->buf[81] != ' ' ||
-	    !(email_end = strchr(sb->buf + 82, '>')) ||
+	if (!sb->len || sb->buf[sb->len - 1] != '\n' ||
+	    parse_oid_hex(p, &ooid, &p) || *p++ != ' ' ||
+	    parse_oid_hex(p, &noid, &p) || *p++ != ' ' ||
+	    !(email_end = strchr(p, '>')) ||
 	    email_end[1] != ' ' ||
 	    !(timestamp = strtoul(email_end + 2, &message, 10)) ||
 	    !message || message[0] != ' ' ||
@@ -3136,7 +3137,7 @@ static int show_one_reflog_ent(struct strbuf *sb, each_reflog_ent_fn fn, void *c
 		message += 6;
 	else
 		message += 7;
-	return fn(&ooid, &noid, sb->buf + 82, timestamp, tz, message, cb_data);
+	return fn(&ooid, &noid, p, timestamp, tz, message, cb_data);
 }
 
 static char *find_beginning_of_line(char *bob, char *scan)
-- 
2.11.0


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

* [PATCH v3 16/19] sha1_file: introduce an nth_packed_object_oid function
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (14 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 15/19] refs: simplify parsing of reflog entries brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  1:24   ` Ramsay Jones
  2017-02-18  0:06 ` [PATCH v3 17/19] Convert object iteration callbacks to struct object_id brian m. carlson
                   ` (2 subsequent siblings)
  18 siblings, 1 reply; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

There are places in the code where we would like to provide a struct
object_id *, yet read the hash directly from the pack.  Provide an
nth_packed_object_oid function that is similar to the
nth_packed_object_sha1 function.

In order to avoid a potentially invalid cast, nth_packed_object_oid
provides a variable into which to store the value, which it returns on
success; on error, it returns NULL, as nth_packed_object_sha1 does.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 cache.h     |  6 ++++++
 sha1_file.c | 17 ++++++++++++++---
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/cache.h b/cache.h
index e03a672d15..4f3bfc5ee7 100644
--- a/cache.h
+++ b/cache.h
@@ -1608,6 +1608,12 @@ extern void check_pack_index_ptr(const struct packed_git *p, const void *ptr);
  * error.
  */
 extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t n);
+/*
+ * Like nth_packed_object_oid, but write the data into the object specified by
+ * the the first argument.  Returns the first argument on success, and NULL on
+ * error.
+ */
+extern const struct object_id *nth_packed_object_oid(struct object_id *, struct packed_git *, uint32_t n);
 
 /*
  * Return the offset of the nth object within the specified packfile.
diff --git a/sha1_file.c b/sha1_file.c
index ec957db5e1..777b8e8eae 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2628,6 +2628,17 @@ const unsigned char *nth_packed_object_sha1(struct packed_git *p,
 	}
 }
 
+const struct object_id *nth_packed_object_oid(struct object_id *oid,
+					      struct packed_git *p,
+					      uint32_t n)
+{
+	const unsigned char *hash = nth_packed_object_sha1(p, n);
+	if (!hash)
+		return NULL;
+	hashcpy(oid->hash, hash);
+	return oid;
+}
+
 void check_pack_index_ptr(const struct packed_git *p, const void *vptr)
 {
 	const unsigned char *ptr = vptr;
@@ -3788,13 +3799,13 @@ static int for_each_object_in_pack(struct packed_git *p, each_packed_object_fn c
 	int r = 0;
 
 	for (i = 0; i < p->num_objects; i++) {
-		const unsigned char *sha1 = nth_packed_object_sha1(p, i);
+		struct object_id oid;
 
-		if (!sha1)
+		if (!nth_packed_object_oid(&oid, p, i))
 			return error("unable to get sha1 of object %u in %s",
 				     i, p->pack_name);
 
-		r = cb(sha1, p, i, data);
+		r = cb(oid.hash, p, i, data);
 		if (r)
 			break;
 	}
-- 
2.11.0


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

* [PATCH v3 17/19] Convert object iteration callbacks to struct object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (15 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 16/19] sha1_file: introduce an nth_packed_object_oid function brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 18/19] builtin/merge-base: convert " brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 19/19] wt-status: " brian m. carlson
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

Convert each_loose_object_fn and each_packed_object_fn to take a pointer
to struct object_id.  Update the various callbacks.  Convert several
40-based constants to use GIT_SHA1_HEXSZ.

Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
 builtin/cat-file.c      |  8 ++++----
 builtin/count-objects.c |  4 ++--
 builtin/fsck.c          | 24 ++++++++++++------------
 builtin/pack-objects.c  |  6 +++---
 builtin/prune-packed.c  |  4 ++--
 builtin/prune.c         |  8 ++++----
 cache.h                 |  4 ++--
 reachable.c             | 30 +++++++++++++++---------------
 sha1_file.c             | 12 ++++++------
 9 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 30383e9eb4..8b85cb8cf0 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -409,20 +409,20 @@ static int batch_object_cb(const unsigned char sha1[20], void *vdata)
 	return 0;
 }
 
-static int batch_loose_object(const unsigned char *sha1,
+static int batch_loose_object(const struct object_id *oid,
 			      const char *path,
 			      void *data)
 {
-	sha1_array_append(data, sha1);
+	sha1_array_append(data, oid->hash);
 	return 0;
 }
 
-static int batch_packed_object(const unsigned char *sha1,
+static int batch_packed_object(const struct object_id *oid,
 			       struct packed_git *pack,
 			       uint32_t pos,
 			       void *data)
 {
-	sha1_array_append(data, sha1);
+	sha1_array_append(data, oid->hash);
 	return 0;
 }
 
diff --git a/builtin/count-objects.c b/builtin/count-objects.c
index a04b4f2ef3..acb05940fc 100644
--- a/builtin/count-objects.c
+++ b/builtin/count-objects.c
@@ -53,7 +53,7 @@ static void loose_garbage(const char *path)
 		report_garbage(PACKDIR_FILE_GARBAGE, path);
 }
 
-static int count_loose(const unsigned char *sha1, const char *path, void *data)
+static int count_loose(const struct object_id *oid, const char *path, void *data)
 {
 	struct stat st;
 
@@ -62,7 +62,7 @@ static int count_loose(const unsigned char *sha1, const char *path, void *data)
 	else {
 		loose_size += on_disk_bytes(st);
 		loose++;
-		if (verbose && has_sha1_pack(sha1))
+		if (verbose && has_sha1_pack(oid->hash))
 			packed_loose++;
 	}
 	return 0;
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 9b37606858..f76e4163ab 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -491,7 +491,7 @@ static void get_default_heads(void)
 	}
 }
 
-static struct object *parse_loose_object(const unsigned char *sha1,
+static struct object *parse_loose_object(const struct object_id *oid,
 					 const char *path)
 {
 	struct object *obj;
@@ -500,27 +500,27 @@ static struct object *parse_loose_object(const unsigned char *sha1,
 	unsigned long size;
 	int eaten;
 
-	if (read_loose_object(path, sha1, &type, &size, &contents) < 0)
+	if (read_loose_object(path, oid->hash, &type, &size, &contents) < 0)
 		return NULL;
 
 	if (!contents && type != OBJ_BLOB)
 		die("BUG: read_loose_object streamed a non-blob");
 
-	obj = parse_object_buffer(sha1, type, size, contents, &eaten);
+	obj = parse_object_buffer(oid->hash, type, size, contents, &eaten);
 
 	if (!eaten)
 		free(contents);
 	return obj;
 }
 
-static int fsck_loose(const unsigned char *sha1, const char *path, void *data)
+static int fsck_loose(const struct object_id *oid, const char *path, void *data)
 {
-	struct object *obj = parse_loose_object(sha1, path);
+	struct object *obj = parse_loose_object(oid, path);
 
 	if (!obj) {
 		errors_found |= ERROR_OBJECT;
 		error("%s: object corrupt or missing: %s",
-		      sha1_to_hex(sha1), path);
+		      oid_to_hex(oid), path);
 		return 0; /* keep checking other objects */
 	}
 
@@ -619,26 +619,26 @@ static int fsck_cache_tree(struct cache_tree *it)
 	return err;
 }
 
-static void mark_object_for_connectivity(const unsigned char *sha1)
+static void mark_object_for_connectivity(const struct object_id *oid)
 {
-	struct object *obj = lookup_unknown_object(sha1);
+	struct object *obj = lookup_unknown_object(oid->hash);
 	obj->flags |= HAS_OBJ;
 }
 
-static int mark_loose_for_connectivity(const unsigned char *sha1,
+static int mark_loose_for_connectivity(const struct object_id *oid,
 				       const char *path,
 				       void *data)
 {
-	mark_object_for_connectivity(sha1);
+	mark_object_for_connectivity(oid);
 	return 0;
 }
 
-static int mark_packed_for_connectivity(const unsigned char *sha1,
+static int mark_packed_for_connectivity(const struct object_id *oid,
 					struct packed_git *pack,
 					uint32_t pos,
 					void *data)
 {
-	mark_object_for_connectivity(sha1);
+	mark_object_for_connectivity(oid);
 	return 0;
 }
 
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 8841f8b366..6e7c3a6575 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2534,17 +2534,17 @@ static void add_objects_in_unpacked_packs(struct rev_info *revs)
 	free(in_pack.array);
 }
 
-static int add_loose_object(const unsigned char *sha1, const char *path,
+static int add_loose_object(const struct object_id *oid, const char *path,
 			    void *data)
 {
-	enum object_type type = sha1_object_info(sha1, NULL);
+	enum object_type type = sha1_object_info(oid->hash, NULL);
 
 	if (type < 0) {
 		warning("loose object at %s could not be examined", path);
 		return 0;
 	}
 
-	add_object_entry(sha1, type, "", 0);
+	add_object_entry(oid->hash, type, "", 0);
 	return 0;
 }
 
diff --git a/builtin/prune-packed.c b/builtin/prune-packed.c
index 7cf900ea07..c026299e78 100644
--- a/builtin/prune-packed.c
+++ b/builtin/prune-packed.c
@@ -19,12 +19,12 @@ static int prune_subdir(int nr, const char *path, void *data)
 	return 0;
 }
 
-static int prune_object(const unsigned char *sha1, const char *path,
+static int prune_object(const struct object_id *oid, const char *path,
 			 void *data)
 {
 	int *opts = data;
 
-	if (!has_sha1_pack(sha1))
+	if (!has_sha1_pack(oid->hash))
 		return 0;
 
 	if (*opts & PRUNE_PACKED_DRY_RUN)
diff --git a/builtin/prune.c b/builtin/prune.c
index 8f4f052285..42633e0c6e 100644
--- a/builtin/prune.c
+++ b/builtin/prune.c
@@ -30,7 +30,7 @@ static int prune_tmp_file(const char *fullpath)
 	return 0;
 }
 
-static int prune_object(const unsigned char *sha1, const char *fullpath,
+static int prune_object(const struct object_id *oid, const char *fullpath,
 			void *data)
 {
 	struct stat st;
@@ -39,7 +39,7 @@ static int prune_object(const unsigned char *sha1, const char *fullpath,
 	 * Do we know about this object?
 	 * It must have been reachable
 	 */
-	if (lookup_object(sha1))
+	if (lookup_object(oid->hash))
 		return 0;
 
 	if (lstat(fullpath, &st)) {
@@ -50,8 +50,8 @@ static int prune_object(const unsigned char *sha1, const char *fullpath,
 	if (st.st_mtime > expire)
 		return 0;
 	if (show_only || verbose) {
-		enum object_type type = sha1_object_info(sha1, NULL);
-		printf("%s %s\n", sha1_to_hex(sha1),
+		enum object_type type = sha1_object_info(oid->hash, NULL);
+		printf("%s %s\n", oid_to_hex(oid),
 		       (type > 0) ? typename(type) : "unknown");
 	}
 	if (!show_only)
diff --git a/cache.h b/cache.h
index 4f3bfc5ee7..592e27ac8f 100644
--- a/cache.h
+++ b/cache.h
@@ -1655,7 +1655,7 @@ extern int unpack_object_header(struct packed_git *, struct pack_window **, off_
  * scratch buffer, but restored to its original contents before
  * the function returns.
  */
-typedef int each_loose_object_fn(const unsigned char *sha1,
+typedef int each_loose_object_fn(const struct object_id *oid,
 				 const char *path,
 				 void *data);
 typedef int each_loose_cruft_fn(const char *basename,
@@ -1681,7 +1681,7 @@ int for_each_loose_file_in_objdir_buf(struct strbuf *path,
  * LOCAL_ONLY flag is set).
  */
 #define FOR_EACH_OBJECT_LOCAL_ONLY 0x1
-typedef int each_packed_object_fn(const unsigned char *sha1,
+typedef int each_packed_object_fn(const struct object_id *oid,
 				  struct packed_git *pack,
 				  uint32_t pos,
 				  void *data);
diff --git a/reachable.c b/reachable.c
index d0199cace4..a8a979bd4f 100644
--- a/reachable.c
+++ b/reachable.c
@@ -58,7 +58,7 @@ struct recent_data {
 	unsigned long timestamp;
 };
 
-static void add_recent_object(const unsigned char *sha1,
+static void add_recent_object(const struct object_id *oid,
 			      unsigned long mtime,
 			      struct recent_data *data)
 {
@@ -75,37 +75,37 @@ static void add_recent_object(const unsigned char *sha1,
 	 * later processing, and the revision machinery expects
 	 * commits and tags to have been parsed.
 	 */
-	type = sha1_object_info(sha1, NULL);
+	type = sha1_object_info(oid->hash, NULL);
 	if (type < 0)
-		die("unable to get object info for %s", sha1_to_hex(sha1));
+		die("unable to get object info for %s", oid_to_hex(oid));
 
 	switch (type) {
 	case OBJ_TAG:
 	case OBJ_COMMIT:
-		obj = parse_object_or_die(sha1, NULL);
+		obj = parse_object_or_die(oid->hash, NULL);
 		break;
 	case OBJ_TREE:
-		obj = (struct object *)lookup_tree(sha1);
+		obj = (struct object *)lookup_tree(oid->hash);
 		break;
 	case OBJ_BLOB:
-		obj = (struct object *)lookup_blob(sha1);
+		obj = (struct object *)lookup_blob(oid->hash);
 		break;
 	default:
 		die("unknown object type for %s: %s",
-		    sha1_to_hex(sha1), typename(type));
+		    oid_to_hex(oid), typename(type));
 	}
 
 	if (!obj)
-		die("unable to lookup %s", sha1_to_hex(sha1));
+		die("unable to lookup %s", oid_to_hex(oid));
 
 	add_pending_object(data->revs, obj, "");
 }
 
-static int add_recent_loose(const unsigned char *sha1,
+static int add_recent_loose(const struct object_id *oid,
 			    const char *path, void *data)
 {
 	struct stat st;
-	struct object *obj = lookup_object(sha1);
+	struct object *obj = lookup_object(oid->hash);
 
 	if (obj && obj->flags & SEEN)
 		return 0;
@@ -119,22 +119,22 @@ static int add_recent_loose(const unsigned char *sha1,
 		 */
 		if (errno == ENOENT)
 			return 0;
-		return error_errno("unable to stat %s", sha1_to_hex(sha1));
+		return error_errno("unable to stat %s", oid_to_hex(oid));
 	}
 
-	add_recent_object(sha1, st.st_mtime, data);
+	add_recent_object(oid, st.st_mtime, data);
 	return 0;
 }
 
-static int add_recent_packed(const unsigned char *sha1,
+static int add_recent_packed(const struct object_id *oid,
 			     struct packed_git *p, uint32_t pos,
 			     void *data)
 {
-	struct object *obj = lookup_object(sha1);
+	struct object *obj = lookup_object(oid->hash);
 
 	if (obj && obj->flags & SEEN)
 		return 0;
-	add_recent_object(sha1, p->mtime, data);
+	add_recent_object(oid, p->mtime, data);
 	return 0;
 }
 
diff --git a/sha1_file.c b/sha1_file.c
index 777b8e8eae..6b74c7d3bc 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -3685,15 +3685,15 @@ static int for_each_file_in_obj_subdir(int subdir_nr,
 		strbuf_setlen(path, baselen);
 		strbuf_addf(path, "/%s", de->d_name);
 
-		if (strlen(de->d_name) == 38)  {
-			char hex[41];
-			unsigned char sha1[20];
+		if (strlen(de->d_name) == GIT_SHA1_HEXSZ - 2)  {
+			char hex[GIT_SHA1_HEXSZ+1];
+			struct object_id oid;
 
 			snprintf(hex, sizeof(hex), "%02x%s",
 				 subdir_nr, de->d_name);
-			if (!get_sha1_hex(hex, sha1)) {
+			if (!get_oid_hex(hex, &oid)) {
 				if (obj_cb) {
-					r = obj_cb(sha1, path->buf, data);
+					r = obj_cb(&oid, path->buf, data);
 					if (r)
 						break;
 				}
@@ -3805,7 +3805,7 @@ static int for_each_object_in_pack(struct packed_git *p, each_packed_object_fn c
 			return error("unable to get sha1 of object %u in %s",
 				     i, p->pack_name);
 
-		r = cb(oid.hash, p, i, data);
+		r = cb(&oid, p, i, data);
 		if (r)
 			break;
 	}
-- 
2.11.0


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

* [PATCH v3 18/19] builtin/merge-base: convert to struct object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (16 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 17/19] Convert object iteration callbacks to struct object_id brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  2017-02-18  0:06 ` [PATCH v3 19/19] wt-status: " brian m. carlson
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

Convert the remaining uses of unsigned char [20] to struct object_id.

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

diff --git a/builtin/merge-base.c b/builtin/merge-base.c
index db95bc29cf..cfe2a796f8 100644
--- a/builtin/merge-base.c
+++ b/builtin/merge-base.c
@@ -36,12 +36,12 @@ static const char * const merge_base_usage[] = {
 
 static struct commit *get_commit_reference(const char *arg)
 {
-	unsigned char revkey[20];
+	struct object_id revkey;
 	struct commit *r;
 
-	if (get_sha1(arg, revkey))
+	if (get_oid(arg, &revkey))
 		die("Not a valid object name %s", arg);
-	r = lookup_commit_reference(revkey);
+	r = lookup_commit_reference(revkey.hash);
 	if (!r)
 		die("Not a valid commit name %s", arg);
 
@@ -113,14 +113,14 @@ struct rev_collect {
 	unsigned int initial : 1;
 };
 
-static void add_one_commit(unsigned char *sha1, struct rev_collect *revs)
+static void add_one_commit(struct object_id *oid, struct rev_collect *revs)
 {
 	struct commit *commit;
 
-	if (is_null_sha1(sha1))
+	if (is_null_oid(oid))
 		return;
 
-	commit = lookup_commit(sha1);
+	commit = lookup_commit(oid->hash);
 	if (!commit ||
 	    (commit->object.flags & TMP_MARK) ||
 	    parse_commit(commit))
@@ -139,15 +139,15 @@ static int collect_one_reflog_ent(struct object_id *ooid, struct object_id *noid
 
 	if (revs->initial) {
 		revs->initial = 0;
-		add_one_commit(ooid->hash, revs);
+		add_one_commit(ooid, revs);
 	}
-	add_one_commit(noid->hash, revs);
+	add_one_commit(noid, revs);
 	return 0;
 }
 
 static int handle_fork_point(int argc, const char **argv)
 {
-	unsigned char sha1[20];
+	struct object_id oid;
 	char *refname;
 	const char *commitname;
 	struct rev_collect revs;
@@ -155,7 +155,7 @@ static int handle_fork_point(int argc, const char **argv)
 	struct commit_list *bases;
 	int i, ret = 0;
 
-	switch (dwim_ref(argv[0], strlen(argv[0]), sha1, &refname)) {
+	switch (dwim_ref(argv[0], strlen(argv[0]), oid.hash, &refname)) {
 	case 0:
 		die("No such ref: '%s'", argv[0]);
 	case 1:
@@ -165,16 +165,16 @@ static int handle_fork_point(int argc, const char **argv)
 	}
 
 	commitname = (argc == 2) ? argv[1] : "HEAD";
-	if (get_sha1(commitname, sha1))
+	if (get_oid(commitname, &oid))
 		die("Not a valid object name: '%s'", commitname);
 
-	derived = lookup_commit_reference(sha1);
+	derived = lookup_commit_reference(oid.hash);
 	memset(&revs, 0, sizeof(revs));
 	revs.initial = 1;
 	for_each_reflog_ent(refname, collect_one_reflog_ent, &revs);
 
-	if (!revs.nr && !get_sha1(refname, sha1))
-		add_one_commit(sha1, &revs);
+	if (!revs.nr && !get_oid(refname, &oid))
+		add_one_commit(&oid, &revs);
 
 	for (i = 0; i < revs.nr; i++)
 		revs.commit[i]->object.flags &= ~TMP_MARK;
-- 
2.11.0


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

* [PATCH v3 19/19] wt-status: convert to struct object_id
  2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
                   ` (17 preceding siblings ...)
  2017-02-18  0:06 ` [PATCH v3 18/19] builtin/merge-base: convert " brian m. carlson
@ 2017-02-18  0:06 ` brian m. carlson
  18 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  0:06 UTC (permalink / raw)
  To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano

Convert the remaining uses of unsigned char [20] to struct object_id.

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

diff --git a/wt-status.c b/wt-status.c
index 5fac8437b0..a8d1faf80d 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1115,16 +1115,16 @@ static void abbrev_sha1_in_line(struct strbuf *line)
 
 	split = strbuf_split_max(line, ' ', 3);
 	if (split[0] && split[1]) {
-		unsigned char sha1[20];
+		struct object_id oid;
 
 		/*
 		 * strbuf_split_max left a space. Trim it and re-add
 		 * it after abbreviation.
 		 */
 		strbuf_trim(split[1]);
-		if (!get_sha1(split[1]->buf, sha1)) {
+		if (!get_oid(split[1]->buf, &oid)) {
 			strbuf_reset(split[1]);
-			strbuf_add_unique_abbrev(split[1], sha1,
+			strbuf_add_unique_abbrev(split[1], oid.hash,
 						 DEFAULT_ABBREV);
 			strbuf_addch(split[1], ' ');
 			strbuf_reset(line);
@@ -1340,7 +1340,7 @@ static void show_bisect_in_progress(struct wt_status *s,
 static char *get_branch(const struct worktree *wt, const char *path)
 {
 	struct strbuf sb = STRBUF_INIT;
-	unsigned char sha1[20];
+	struct object_id oid;
 	const char *branch_name;
 
 	if (strbuf_read_file(&sb, worktree_git_path(wt, "%s", path), 0) <= 0)
@@ -1354,9 +1354,9 @@ static char *get_branch(const struct worktree *wt, const char *path)
 		strbuf_remove(&sb, 0, branch_name - sb.buf);
 	else if (starts_with(sb.buf, "refs/"))
 		;
-	else if (!get_sha1_hex(sb.buf, sha1)) {
+	else if (!get_oid_hex(sb.buf, &oid)) {
 		strbuf_reset(&sb);
-		strbuf_add_unique_abbrev(&sb, sha1, DEFAULT_ABBREV);
+		strbuf_add_unique_abbrev(&sb, oid.hash, DEFAULT_ABBREV);
 	} else if (!strcmp(sb.buf, "detached HEAD")) /* rebase */
 		goto got_nothing;
 	else			/* bisect */
@@ -1370,7 +1370,7 @@ static char *get_branch(const struct worktree *wt, const char *path)
 
 struct grab_1st_switch_cbdata {
 	struct strbuf buf;
-	unsigned char nsha1[20];
+	struct object_id noid;
 };
 
 static int grab_1st_switch(struct object_id *ooid, struct object_id *noid,
@@ -1387,7 +1387,7 @@ static int grab_1st_switch(struct object_id *ooid, struct object_id *noid,
 		return 0;
 	target += strlen(" to ");
 	strbuf_reset(&cb->buf);
-	hashcpy(cb->nsha1, noid->hash);
+	oidcpy(&cb->noid, noid);
 	end = strchrnul(target, '\n');
 	strbuf_add(&cb->buf, target, end - target);
 	if (!strcmp(cb->buf.buf, "HEAD")) {
@@ -1402,7 +1402,7 @@ static void wt_status_get_detached_from(struct wt_status_state *state)
 {
 	struct grab_1st_switch_cbdata cb;
 	struct commit *commit;
-	unsigned char sha1[20];
+	struct object_id oid;
 	char *ref = NULL;
 
 	strbuf_init(&cb.buf, 0);
@@ -1411,22 +1411,22 @@ static void wt_status_get_detached_from(struct wt_status_state *state)
 		return;
 	}
 
-	if (dwim_ref(cb.buf.buf, cb.buf.len, sha1, &ref) == 1 &&
+	if (dwim_ref(cb.buf.buf, cb.buf.len, oid.hash, &ref) == 1 &&
 	    /* sha1 is a commit? match without further lookup */
-	    (!hashcmp(cb.nsha1, sha1) ||
+	    (!oidcmp(&cb.noid, &oid) ||
 	     /* perhaps sha1 is a tag, try to dereference to a commit */
-	     ((commit = lookup_commit_reference_gently(sha1, 1)) != NULL &&
-	      !hashcmp(cb.nsha1, commit->object.oid.hash)))) {
+	     ((commit = lookup_commit_reference_gently(oid.hash, 1)) != NULL &&
+	      !oidcmp(&cb.noid, &commit->object.oid)))) {
 		const char *from = ref;
 		if (!skip_prefix(from, "refs/tags/", &from))
 			skip_prefix(from, "refs/remotes/", &from);
 		state->detached_from = xstrdup(from);
 	} else
 		state->detached_from =
-			xstrdup(find_unique_abbrev(cb.nsha1, DEFAULT_ABBREV));
-	hashcpy(state->detached_sha1, cb.nsha1);
-	state->detached_at = !get_sha1("HEAD", sha1) &&
-			     !hashcmp(sha1, state->detached_sha1);
+			xstrdup(find_unique_abbrev(cb.noid.hash, DEFAULT_ABBREV));
+	hashcpy(state->detached_sha1, cb.noid.hash);
+	state->detached_at = !get_oid("HEAD", &oid) &&
+			     !hashcmp(oid.hash, state->detached_sha1);
 
 	free(ref);
 	strbuf_release(&cb.buf);
@@ -1476,22 +1476,22 @@ void wt_status_get_state(struct wt_status_state *state,
 			 int get_detached_from)
 {
 	struct stat st;
-	unsigned char sha1[20];
+	struct object_id oid;
 
 	if (!stat(git_path_merge_head(), &st)) {
 		state->merge_in_progress = 1;
 	} else if (wt_status_check_rebase(NULL, state)) {
 		;		/* all set */
 	} else if (!stat(git_path_cherry_pick_head(), &st) &&
-			!get_sha1("CHERRY_PICK_HEAD", sha1)) {
+			!get_oid("CHERRY_PICK_HEAD", &oid)) {
 		state->cherry_pick_in_progress = 1;
-		hashcpy(state->cherry_pick_head_sha1, sha1);
+		hashcpy(state->cherry_pick_head_sha1, oid.hash);
 	}
 	wt_status_check_bisect(NULL, state);
 	if (!stat(git_path_revert_head(), &st) &&
-	    !get_sha1("REVERT_HEAD", sha1)) {
+	    !get_oid("REVERT_HEAD", &oid)) {
 		state->revert_in_progress = 1;
-		hashcpy(state->revert_head_sha1, sha1);
+		hashcpy(state->revert_head_sha1, oid.hash);
 	}
 
 	if (get_detached_from)
-- 
2.11.0


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

* Re: [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id
  2017-02-18  0:06 ` [PATCH v3 02/19] builtin/diff-tree: " brian m. carlson
@ 2017-02-18  1:18   ` Ramsay Jones
  2017-02-18  1:26     ` brian m. carlson
  0 siblings, 1 reply; 28+ messages in thread
From: Ramsay Jones @ 2017-02-18  1:18 UTC (permalink / raw)
  To: brian m. carlson, git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano



On 18/02/17 00:06, brian m. carlson wrote:
> Convert most leaf functions to struct object_id.  Rewrite several
> hardcoded numbers in terms of GIT_SHA1_HEXSZ, using an intermediate
> variable where that makes sense.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  builtin/diff-tree.c | 38 ++++++++++++++++++++------------------
>  1 file changed, 20 insertions(+), 18 deletions(-)
> 
> diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
> index 8ce00480cd..1f1573bb2a 100644
> --- a/builtin/diff-tree.c
> +++ b/builtin/diff-tree.c
> @@ -7,9 +7,9 @@
>  
>  static struct rev_info log_tree_opt;
>  
> -static int diff_tree_commit_sha1(const unsigned char *sha1)
> +static int diff_tree_commit_sha1(const struct object_id *oid)
>  {
> -	struct commit *commit = lookup_commit_reference(sha1);
> +	struct commit *commit = lookup_commit_reference(oid->hash);
>  	if (!commit)
>  		return -1;
>  	return log_tree_commit(&log_tree_opt, commit);
> @@ -18,22 +18,22 @@ static int diff_tree_commit_sha1(const unsigned char *sha1)
>  /* Diff one or more commits. */
>  static int stdin_diff_commit(struct commit *commit, char *line, int len)
>  {
> -	unsigned char sha1[20];
> -	if (isspace(line[40]) && !get_sha1_hex(line+41, sha1)) {
> +	struct object_id oid;
> +	if (isspace(line[GIT_SHA1_HEXSZ]) && !get_oid_hex(line+GIT_SHA1_HEXSZ+1, &oid)) {
>  		/* Graft the fake parents locally to the commit */
> -		int pos = 41;
> +		int pos = GIT_SHA1_HEXSZ + 1;
>  		struct commit_list **pptr;
>  
>  		/* Free the real parent list */
>  		free_commit_list(commit->parents);
>  		commit->parents = NULL;
>  		pptr = &(commit->parents);
> -		while (line[pos] && !get_sha1_hex(line + pos, sha1)) {
> -			struct commit *parent = lookup_commit(sha1);
> +		while (line[pos] && !get_oid_hex(line + pos, &oid)) {
> +			struct commit *parent = lookup_commit(oid.hash);
>  			if (parent) {
>  				pptr = &commit_list_insert(parent, pptr)->next;
>  			}
> -			pos += 41;
> +			pos += GIT_SHA1_HEXSZ + 1;
>  		}
>  	}
>  	return log_tree_commit(&log_tree_opt, commit);
> @@ -42,11 +42,13 @@ static int stdin_diff_commit(struct commit *commit, char *line, int len)
>  /* Diff two trees. */
>  static int stdin_diff_trees(struct tree *tree1, char *line, int len)
>  {
> -	unsigned char sha1[20];
> +	struct object_id oid;
>  	struct tree *tree2;
> -	if (len != 82 || !isspace(line[40]) || get_sha1_hex(line + 41, sha1))
> +	const int chunksz = GIT_SHA1_HEXSZ + 1;
> +	if (len != 2 * chunksz || !isspace(line[chunksz-1]) ||
> +		get_sha1_hex(line + chunksz, oid.hash))

I'm not sure that this is an improvement. The input expected in 'line'
is supposed to look like: '<sha1> + <space> + <sha1> + <\n>'. So your
'chunk' would be a <sha1> plus one 'char' of some sort. Except that the
caller of this function has already replaced the newline character with
a '\0' char (so strlen(line) would return 81), but still passes the
original line length! Also, note that this (and other functions in this
file) actually test for 'isspace(char)' rather than for a ' ' char!

Hmm, maybe just:

if (len < (2 * GIT_SHA1_HEXSZ + 1) || line[GIT_SHA1_HEXSZ] != ' ' ||
    get_sha1_hex(line + GIT_SHA1_HEXSZ + 1, oid.hash))

(or, perhaps, still call isspace() in this patch ...)

ATB,
Ramsay Jones

>  		return error("Need exactly two trees, separated by a space");
> -	tree2 = lookup_tree(sha1);
> +	tree2 = lookup_tree(oid.hash);
>  	if (!tree2 || parse_tree(tree2))
>  		return -1;
>  	printf("%s %s\n", oid_to_hex(&tree1->object.oid),
> @@ -60,15 +62,15 @@ static int stdin_diff_trees(struct tree *tree1, char *line, int len)
>  static int diff_tree_stdin(char *line)
>  {
>  	int len = strlen(line);
> -	unsigned char sha1[20];
> +	struct object_id oid;
>  	struct object *obj;
>  
>  	if (!len || line[len-1] != '\n')
>  		return -1;
>  	line[len-1] = 0;
> -	if (get_sha1_hex(line, sha1))
> +	if (get_oid_hex(line, &oid))
>  		return -1;
> -	obj = parse_object(sha1);
> +	obj = parse_object(oid.hash);
>  	if (!obj)
>  		return -1;
>  	if (obj->type == OBJ_COMMIT)
> @@ -76,7 +78,7 @@ static int diff_tree_stdin(char *line)
>  	if (obj->type == OBJ_TREE)
>  		return stdin_diff_trees((struct tree *)obj, line, len);
>  	error("Object %s is a %s, not a commit or tree",
> -	      sha1_to_hex(sha1), typename(obj->type));
> +	      oid_to_hex(&oid), typename(obj->type));
>  	return -1;
>  }
>  
> @@ -141,7 +143,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
>  		break;
>  	case 1:
>  		tree1 = opt->pending.objects[0].item;
> -		diff_tree_commit_sha1(tree1->oid.hash);
> +		diff_tree_commit_sha1(&tree1->oid);
>  		break;
>  	case 2:
>  		tree1 = opt->pending.objects[0].item;
> @@ -164,9 +166,9 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
>  			opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
>  					       DIFF_SETUP_USE_CACHE);
>  		while (fgets(line, sizeof(line), stdin)) {
> -			unsigned char sha1[20];
> +			struct object_id oid;
>  
> -			if (get_sha1_hex(line, sha1)) {
> +			if (get_oid_hex(line, &oid)) {
>  				fputs(line, stdout);
>  				fflush(stdout);
>  			}
> 

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

* Re: [PATCH v3 16/19] sha1_file: introduce an nth_packed_object_oid function
  2017-02-18  0:06 ` [PATCH v3 16/19] sha1_file: introduce an nth_packed_object_oid function brian m. carlson
@ 2017-02-18  1:24   ` Ramsay Jones
  2017-02-18  1:29     ` brian m. carlson
  0 siblings, 1 reply; 28+ messages in thread
From: Ramsay Jones @ 2017-02-18  1:24 UTC (permalink / raw)
  To: brian m. carlson, git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano



On 18/02/17 00:06, brian m. carlson wrote:
> There are places in the code where we would like to provide a struct
> object_id *, yet read the hash directly from the pack.  Provide an
> nth_packed_object_oid function that is similar to the
> nth_packed_object_sha1 function.
> 
> In order to avoid a potentially invalid cast, nth_packed_object_oid
> provides a variable into which to store the value, which it returns on
> success; on error, it returns NULL, as nth_packed_object_sha1 does.
> 
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  cache.h     |  6 ++++++
>  sha1_file.c | 17 ++++++++++++++---
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/cache.h b/cache.h
> index e03a672d15..4f3bfc5ee7 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1608,6 +1608,12 @@ extern void check_pack_index_ptr(const struct packed_git *p, const void *ptr);
>   * error.
>   */
>  extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t n);
> +/*
> + * Like nth_packed_object_oid, but write the data into the object specified by
                             ^^^
... Like nth_packed_object_sha1, but ...

Having said that, if the intent is to eventually replace that function with
the new nth_packed_object_oid(), then it is probably not a good idea to
describe this function in terms of the function it will obsolete. ;-)

ATB,
Ramsay Jones

> + * the the first argument.  Returns the first argument on success, and NULL on
> + * error.
> + */
> +extern const struct object_id *nth_packed_object_oid(struct object_id *, struct packed_git *, uint32_t n);
>  
>  /*
>   * Return the offset of the nth object within the specified packfile.
> diff --git a/sha1_file.c b/sha1_file.c
> index ec957db5e1..777b8e8eae 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2628,6 +2628,17 @@ const unsigned char *nth_packed_object_sha1(struct packed_git *p,
>  	}
>  }
>  
> +const struct object_id *nth_packed_object_oid(struct object_id *oid,
> +					      struct packed_git *p,
> +					      uint32_t n)
> +{
> +	const unsigned char *hash = nth_packed_object_sha1(p, n);
> +	if (!hash)
> +		return NULL;
> +	hashcpy(oid->hash, hash);
> +	return oid;
> +}
> +
>  void check_pack_index_ptr(const struct packed_git *p, const void *vptr)
>  {
>  	const unsigned char *ptr = vptr;
> @@ -3788,13 +3799,13 @@ static int for_each_object_in_pack(struct packed_git *p, each_packed_object_fn c
>  	int r = 0;
>  
>  	for (i = 0; i < p->num_objects; i++) {
> -		const unsigned char *sha1 = nth_packed_object_sha1(p, i);
> +		struct object_id oid;
>  
> -		if (!sha1)
> +		if (!nth_packed_object_oid(&oid, p, i))
>  			return error("unable to get sha1 of object %u in %s",
>  				     i, p->pack_name);
>  
> -		r = cb(sha1, p, i, data);
> +		r = cb(oid.hash, p, i, data);
>  		if (r)
>  			break;
>  	}
> 

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

* Re: [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id
  2017-02-18  1:18   ` Ramsay Jones
@ 2017-02-18  1:26     ` brian m. carlson
  2017-02-18  1:42       ` Jeff King
  0 siblings, 1 reply; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  1:26 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: git, Jeff King, Michael Haggerty, Junio C Hamano

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

On Sat, Feb 18, 2017 at 01:18:11AM +0000, Ramsay Jones wrote:
> 
> 
> On 18/02/17 00:06, brian m. carlson wrote:
> > Convert most leaf functions to struct object_id.  Rewrite several
> > hardcoded numbers in terms of GIT_SHA1_HEXSZ, using an intermediate
> > variable where that makes sense.
> > 
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> >  builtin/diff-tree.c | 38 ++++++++++++++++++++------------------
> >  1 file changed, 20 insertions(+), 18 deletions(-)
> > 
> > diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
> > index 8ce00480cd..1f1573bb2a 100644
> > --- a/builtin/diff-tree.c
> > +++ b/builtin/diff-tree.c
> > @@ -7,9 +7,9 @@
> >  
> >  static struct rev_info log_tree_opt;
> >  
> > -static int diff_tree_commit_sha1(const unsigned char *sha1)
> > +static int diff_tree_commit_sha1(const struct object_id *oid)
> >  {
> > -	struct commit *commit = lookup_commit_reference(sha1);
> > +	struct commit *commit = lookup_commit_reference(oid->hash);
> >  	if (!commit)
> >  		return -1;
> >  	return log_tree_commit(&log_tree_opt, commit);
> > @@ -18,22 +18,22 @@ static int diff_tree_commit_sha1(const unsigned char *sha1)
> >  /* Diff one or more commits. */
> >  static int stdin_diff_commit(struct commit *commit, char *line, int len)
> >  {
> > -	unsigned char sha1[20];
> > -	if (isspace(line[40]) && !get_sha1_hex(line+41, sha1)) {
> > +	struct object_id oid;
> > +	if (isspace(line[GIT_SHA1_HEXSZ]) && !get_oid_hex(line+GIT_SHA1_HEXSZ+1, &oid)) {
> >  		/* Graft the fake parents locally to the commit */
> > -		int pos = 41;
> > +		int pos = GIT_SHA1_HEXSZ + 1;
> >  		struct commit_list **pptr;
> >  
> >  		/* Free the real parent list */
> >  		free_commit_list(commit->parents);
> >  		commit->parents = NULL;
> >  		pptr = &(commit->parents);
> > -		while (line[pos] && !get_sha1_hex(line + pos, sha1)) {
> > -			struct commit *parent = lookup_commit(sha1);
> > +		while (line[pos] && !get_oid_hex(line + pos, &oid)) {
> > +			struct commit *parent = lookup_commit(oid.hash);
> >  			if (parent) {
> >  				pptr = &commit_list_insert(parent, pptr)->next;
> >  			}
> > -			pos += 41;
> > +			pos += GIT_SHA1_HEXSZ + 1;
> >  		}
> >  	}
> >  	return log_tree_commit(&log_tree_opt, commit);
> > @@ -42,11 +42,13 @@ static int stdin_diff_commit(struct commit *commit, char *line, int len)
> >  /* Diff two trees. */
> >  static int stdin_diff_trees(struct tree *tree1, char *line, int len)
> >  {
> > -	unsigned char sha1[20];
> > +	struct object_id oid;
> >  	struct tree *tree2;
> > -	if (len != 82 || !isspace(line[40]) || get_sha1_hex(line + 41, sha1))
> > +	const int chunksz = GIT_SHA1_HEXSZ + 1;
> > +	if (len != 2 * chunksz || !isspace(line[chunksz-1]) ||
> > +		get_sha1_hex(line + chunksz, oid.hash))
> 
> I'm not sure that this is an improvement. The input expected in 'line'
> is supposed to look like: '<sha1> + <space> + <sha1> + <\n>'. So your
> 'chunk' would be a <sha1> plus one 'char' of some sort. Except that the
> caller of this function has already replaced the newline character with
> a '\0' char (so strlen(line) would return 81), but still passes the
> original line length! Also, note that this (and other functions in this
> file) actually test for 'isspace(char)' rather than for a ' ' char!
> 
> Hmm, maybe just:
> 
> if (len < (2 * GIT_SHA1_HEXSZ + 1) || line[GIT_SHA1_HEXSZ] != ' ' ||
>     get_sha1_hex(line + GIT_SHA1_HEXSZ + 1, oid.hash))
> 
> (or, perhaps, still call isspace() in this patch ...)

Well, I think it's strictly an improvement in that we have avoided
writing hardcoded constants[0].  I did intend it as a "hash plus one"
chunk, which is actually quite common throughout the code.

I'm wondering if parse_oid_hex could be useful here as well.

[0] If we change the hash size, all of the GIT_SHA1_HEXSZ constants can
be replaced with a variable that varies based on hash size, and the code
still works.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

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

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

* Re: [PATCH v3 16/19] sha1_file: introduce an nth_packed_object_oid function
  2017-02-18  1:24   ` Ramsay Jones
@ 2017-02-18  1:29     ` brian m. carlson
  0 siblings, 0 replies; 28+ messages in thread
From: brian m. carlson @ 2017-02-18  1:29 UTC (permalink / raw)
  To: Ramsay Jones; +Cc: git, Jeff King, Michael Haggerty, Junio C Hamano

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

On Sat, Feb 18, 2017 at 01:24:34AM +0000, Ramsay Jones wrote:
> 
> 
> On 18/02/17 00:06, brian m. carlson wrote:
> > There are places in the code where we would like to provide a struct
> > object_id *, yet read the hash directly from the pack.  Provide an
> > nth_packed_object_oid function that is similar to the
> > nth_packed_object_sha1 function.
> > 
> > In order to avoid a potentially invalid cast, nth_packed_object_oid
> > provides a variable into which to store the value, which it returns on
> > success; on error, it returns NULL, as nth_packed_object_sha1 does.
> > 
> > Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> > ---
> >  cache.h     |  6 ++++++
> >  sha1_file.c | 17 ++++++++++++++---
> >  2 files changed, 20 insertions(+), 3 deletions(-)
> > 
> > diff --git a/cache.h b/cache.h
> > index e03a672d15..4f3bfc5ee7 100644
> > --- a/cache.h
> > +++ b/cache.h
> > @@ -1608,6 +1608,12 @@ extern void check_pack_index_ptr(const struct packed_git *p, const void *ptr);
> >   * error.
> >   */
> >  extern const unsigned char *nth_packed_object_sha1(struct packed_git *, uint32_t n);
> > +/*
> > + * Like nth_packed_object_oid, but write the data into the object specified by
>                              ^^^
> ... Like nth_packed_object_sha1, but ...

Good catch.

> Having said that, if the intent is to eventually replace that function with
> the new nth_packed_object_oid(), then it is probably not a good idea to
> describe this function in terms of the function it will obsolete. ;-)

I've chosen to define them that way for now, in the hopes that it will
make them easier to use (as people are already familiar with the _sha1
version).  When I remove nth_packed_object_sha1 eventually, I'll update
the docstring so that the oid version is standalone.  I think I've
already done that somewhere else before (although it may be in a patch
that I haven't sent yet).
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

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

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

* Re: [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id
  2017-02-18  1:26     ` brian m. carlson
@ 2017-02-18  1:42       ` Jeff King
  2017-02-18  3:15         ` Jeff King
  0 siblings, 1 reply; 28+ messages in thread
From: Jeff King @ 2017-02-18  1:42 UTC (permalink / raw)
  To: brian m. carlson, Ramsay Jones, git, Michael Haggerty, Junio C Hamano

On Sat, Feb 18, 2017 at 01:26:07AM +0000, brian m. carlson wrote:

> > > +	struct object_id oid;
> > >  	struct tree *tree2;
> > > -	if (len != 82 || !isspace(line[40]) || get_sha1_hex(line + 41, sha1))
> > > +	const int chunksz = GIT_SHA1_HEXSZ + 1;
> > > +	if (len != 2 * chunksz || !isspace(line[chunksz-1]) ||
> > > +		get_sha1_hex(line + chunksz, oid.hash))
> > 
> > I'm not sure that this is an improvement. The input expected in 'line'
> > is supposed to look like: '<sha1> + <space> + <sha1> + <\n>'. So your
> > 'chunk' would be a <sha1> plus one 'char' of some sort. Except that the
> > caller of this function has already replaced the newline character with
> > a '\0' char (so strlen(line) would return 81), but still passes the
> > original line length! Also, note that this (and other functions in this
> > file) actually test for 'isspace(char)' rather than for a ' ' char!
> > 
> > Hmm, maybe just:
> > 
> > if (len < (2 * GIT_SHA1_HEXSZ + 1) || line[GIT_SHA1_HEXSZ] != ' ' ||
> >     get_sha1_hex(line + GIT_SHA1_HEXSZ + 1, oid.hash))
> > 
> > (or, perhaps, still call isspace() in this patch ...)
> 
> Well, I think it's strictly an improvement in that we have avoided
> writing hardcoded constants[0].  I did intend it as a "hash plus one"
> chunk, which is actually quite common throughout the code.
> 
> I'm wondering if parse_oid_hex could be useful here as well.

I know I haven't looked at this chunk nearly as carefully as you have,
but it seems somewhat crazy to me that these functions get the original
"line" in the first place. Shouldn't they get line+40 from the caller
(who in turn should be using parse_oid_hex to compute that)?

And then each function should subsequently parse left-to-right with
a mix of isspace() and parse_oid_hex(), and probably doesn't even need
to care about the original "len" at all (yes, you can quit early if you
know your len isn't long enough, but that's the unusual error case
anyway; it's not a big deal to find that out while parsing).

In general, I think this sort of left-to-right incremental pointer
movement is safe and simple. There may be a few cases where it doesn't
apply (i.e., where you need to look at the end of the string to know how
to parse the beginning), but that should be relatively rare.

> [0] If we change the hash size, all of the GIT_SHA1_HEXSZ constants can
> be replaced with a variable that varies based on hash size, and the code
> still works.

I am happy to see fewer magic numbers. But I think with incremental
pointer-movements, we don't even need to use the numeric constants,
either. If one day we can parse "sha256:1234abcd..." as an oid, then the
existing code would "just work".

-Peff

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

* Re: [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id
  2017-02-18  1:42       ` Jeff King
@ 2017-02-18  3:15         ` Jeff King
  2017-02-18 19:12           ` brian m. carlson
  0 siblings, 1 reply; 28+ messages in thread
From: Jeff King @ 2017-02-18  3:15 UTC (permalink / raw)
  To: brian m. carlson, Ramsay Jones, git, Michael Haggerty, Junio C Hamano

On Fri, Feb 17, 2017 at 08:42:17PM -0500, Jeff King wrote:

> > I'm wondering if parse_oid_hex could be useful here as well.
> 
> I know I haven't looked at this chunk nearly as carefully as you have,
> but it seems somewhat crazy to me that these functions get the original
> "line" in the first place. Shouldn't they get line+40 from the caller
> (who in turn should be using parse_oid_hex to compute that)?
> 
> And then each function should subsequently parse left-to-right with
> a mix of isspace() and parse_oid_hex(), and probably doesn't even need
> to care about the original "len" at all (yes, you can quit early if you
> know your len isn't long enough, but that's the unusual error case
> anyway; it's not a big deal to find that out while parsing).
> 
> In general, I think this sort of left-to-right incremental pointer
> movement is safe and simple. There may be a few cases where it doesn't
> apply (i.e., where you need to look at the end of the string to know how
> to parse the beginning), but that should be relatively rare.

So for this case, something like the patch below.

Incidentally, there's an off-by-one in the original loop of
stdin_diff_commit that reads past the end of the trailing NUL for the
final sha1 on the line. The problem is the:

  pos += GIT_SHA1_HEXSZ + 1;

which assumes we're slurping up the trailing space. This works in
practice because the caller will only permit a string which had a
newline (which it converted into a NUL).

I suspect that function could be more aggressive about complaining about
nonsense on the line, rather than silently ignoring it.

 builtin/diff-tree.c | 43 ++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 23 deletions(-)

diff --git a/builtin/diff-tree.c b/builtin/diff-tree.c
index 1f1573bb2..222c671f2 100644
--- a/builtin/diff-tree.c
+++ b/builtin/diff-tree.c
@@ -16,37 +16,33 @@ static int diff_tree_commit_sha1(const struct object_id *oid)
 }
 
 /* Diff one or more commits. */
-static int stdin_diff_commit(struct commit *commit, char *line, int len)
+static int stdin_diff_commit(struct commit *commit, const char *p)
 {
 	struct object_id oid;
-	if (isspace(line[GIT_SHA1_HEXSZ]) && !get_oid_hex(line+GIT_SHA1_HEXSZ+1, &oid)) {
-		/* Graft the fake parents locally to the commit */
-		int pos = GIT_SHA1_HEXSZ + 1;
-		struct commit_list **pptr;
-
-		/* Free the real parent list */
-		free_commit_list(commit->parents);
-		commit->parents = NULL;
-		pptr = &(commit->parents);
-		while (line[pos] && !get_oid_hex(line + pos, &oid)) {
-			struct commit *parent = lookup_commit(oid.hash);
-			if (parent) {
-				pptr = &commit_list_insert(parent, pptr)->next;
-			}
-			pos += GIT_SHA1_HEXSZ + 1;
+	struct commit_list **pptr = NULL;
+
+	/* Graft the fake parents locally to the commit */
+	while (isspace(*p++) && !parse_oid_hex(p, &oid, &p)) {
+		struct commit *parent = lookup_commit(oid.hash);
+		if (!pptr) {
+			/* Free the real parent list */
+			free_commit_list(commit->parents);
+			commit->parents = NULL;
+			pptr = &(commit->parents);
+		}
+		if (parent) {
+			pptr = &commit_list_insert(parent, pptr)->next;
 		}
 	}
 	return log_tree_commit(&log_tree_opt, commit);
 }
 
 /* Diff two trees. */
-static int stdin_diff_trees(struct tree *tree1, char *line, int len)
+static int stdin_diff_trees(struct tree *tree1, const char *p)
 {
 	struct object_id oid;
 	struct tree *tree2;
-	const int chunksz = GIT_SHA1_HEXSZ + 1;
-	if (len != 2 * chunksz || !isspace(line[chunksz-1]) ||
-		get_sha1_hex(line + chunksz, oid.hash))
+	if (!isspace(*p++) || parse_oid_hex(p, &oid, &p) || *p)
 		return error("Need exactly two trees, separated by a space");
 	tree2 = lookup_tree(oid.hash);
 	if (!tree2 || parse_tree(tree2))
@@ -64,19 +60,20 @@ static int diff_tree_stdin(char *line)
 	int len = strlen(line);
 	struct object_id oid;
 	struct object *obj;
+	const char *rest;
 
 	if (!len || line[len-1] != '\n')
 		return -1;
 	line[len-1] = 0;
-	if (get_oid_hex(line, &oid))
+	if (parse_oid_hex(line, &oid, &rest))
 		return -1;
 	obj = parse_object(oid.hash);
 	if (!obj)
 		return -1;
 	if (obj->type == OBJ_COMMIT)
-		return stdin_diff_commit((struct commit *)obj, line, len);
+		return stdin_diff_commit((struct commit *)obj, rest);
 	if (obj->type == OBJ_TREE)
-		return stdin_diff_trees((struct tree *)obj, line, len);
+		return stdin_diff_trees((struct tree *)obj, rest);
 	error("Object %s is a %s, not a commit or tree",
 	      oid_to_hex(&oid), typename(obj->type));
 	return -1;

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

* Re: [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id
  2017-02-18  3:15         ` Jeff King
@ 2017-02-18 19:12           ` brian m. carlson
  2017-02-18 20:01             ` Jeff King
  0 siblings, 1 reply; 28+ messages in thread
From: brian m. carlson @ 2017-02-18 19:12 UTC (permalink / raw)
  To: Jeff King; +Cc: Ramsay Jones, git, Michael Haggerty, Junio C Hamano

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

On Fri, Feb 17, 2017 at 10:15:31PM -0500, Jeff King wrote:
> So for this case, something like the patch below.
> 
> Incidentally, there's an off-by-one in the original loop of
> stdin_diff_commit that reads past the end of the trailing NUL for the
> final sha1 on the line. The problem is the:
> 
>   pos += GIT_SHA1_HEXSZ + 1;
> 
> which assumes we're slurping up the trailing space. This works in
> practice because the caller will only permit a string which had a
> newline (which it converted into a NUL).
> 
> I suspect that function could be more aggressive about complaining about
> nonsense on the line, rather than silently ignoring it.

I'd come to basically the same patch, but I did pick up a few niceties
from your patch, like avoiding the off-by-one issue you mentioned above.
Can I place your sign-off on the resulting change?
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

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

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

* Re: [PATCH v3 02/19] builtin/diff-tree: convert to struct object_id
  2017-02-18 19:12           ` brian m. carlson
@ 2017-02-18 20:01             ` Jeff King
  0 siblings, 0 replies; 28+ messages in thread
From: Jeff King @ 2017-02-18 20:01 UTC (permalink / raw)
  To: brian m. carlson, Ramsay Jones, git, Michael Haggerty, Junio C Hamano

On Sat, Feb 18, 2017 at 07:12:18PM +0000, brian m. carlson wrote:

> On Fri, Feb 17, 2017 at 10:15:31PM -0500, Jeff King wrote:
> > So for this case, something like the patch below.
> > 
> > Incidentally, there's an off-by-one in the original loop of
> > stdin_diff_commit that reads past the end of the trailing NUL for the
> > final sha1 on the line. The problem is the:
> > 
> >   pos += GIT_SHA1_HEXSZ + 1;
> > 
> > which assumes we're slurping up the trailing space. This works in
> > practice because the caller will only permit a string which had a
> > newline (which it converted into a NUL).
> > 
> > I suspect that function could be more aggressive about complaining about
> > nonsense on the line, rather than silently ignoring it.
> 
> I'd come to basically the same patch, but I did pick up a few niceties
> from your patch, like avoiding the off-by-one issue you mentioned above.
> Can I place your sign-off on the resulting change?

Absolutely. Thanks for taking a look.

-Peff

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

end of thread, other threads:[~2017-02-18 20:01 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-18  0:06 [PATCH v3 00/19] object_id part 6 brian m. carlson
2017-02-18  0:06 ` [PATCH v3 01/19] builtin/commit: convert to struct object_id brian m. carlson
2017-02-18  0:06 ` [PATCH v3 02/19] builtin/diff-tree: " brian m. carlson
2017-02-18  1:18   ` Ramsay Jones
2017-02-18  1:26     ` brian m. carlson
2017-02-18  1:42       ` Jeff King
2017-02-18  3:15         ` Jeff King
2017-02-18 19:12           ` brian m. carlson
2017-02-18 20:01             ` Jeff King
2017-02-18  0:06 ` [PATCH v3 03/19] builtin/describe: " brian m. carlson
2017-02-18  0:06 ` [PATCH v3 04/19] builtin/fast-export: " brian m. carlson
2017-02-18  0:06 ` [PATCH v3 05/19] builtin/fmt-merge-message: " brian m. carlson
2017-02-18  0:06 ` [PATCH v3 06/19] builtin/grep: " brian m. carlson
2017-02-18  0:06 ` [PATCH v3 07/19] builtin/branch: " brian m. carlson
2017-02-18  0:06 ` [PATCH v3 08/19] builtin/clone: " brian m. carlson
2017-02-18  0:06 ` [PATCH v3 09/19] builtin/merge: " brian m. carlson
2017-02-18  0:06 ` [PATCH v3 10/19] Convert remaining callers of resolve_refdup to object_id brian m. carlson
2017-02-18  0:06 ` [PATCH v3 11/19] builtin/replace: convert to struct object_id brian m. carlson
2017-02-18  0:06 ` [PATCH v3 12/19] reflog-walk: convert struct reflog_info " brian m. carlson
2017-02-18  0:06 ` [PATCH v3 13/19] refs: convert each_reflog_ent_fn " brian m. carlson
2017-02-18  0:06 ` [PATCH v3 14/19] hex: introduce parse_oid_hex brian m. carlson
2017-02-18  0:06 ` [PATCH v3 15/19] refs: simplify parsing of reflog entries brian m. carlson
2017-02-18  0:06 ` [PATCH v3 16/19] sha1_file: introduce an nth_packed_object_oid function brian m. carlson
2017-02-18  1:24   ` Ramsay Jones
2017-02-18  1:29     ` brian m. carlson
2017-02-18  0:06 ` [PATCH v3 17/19] Convert object iteration callbacks to struct object_id brian m. carlson
2017-02-18  0:06 ` [PATCH v3 18/19] builtin/merge-base: convert " brian m. carlson
2017-02-18  0:06 ` [PATCH v3 19/19] wt-status: " brian m. carlson

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