{"thread":{"id":"36577","subject":"[PATCH 1/9] Define a structure for object IDs.","startedAt":"2014-05-03T20:12:13Z","lastAt":"2014-05-06T15:08:35Z","messageCount":39,"participants":["brian m. carlson","Michael Haggerty","Johannes Sixt","David Kastrup","Andreas Schwab","Duy Nguyen","Felipe Contreras","James Denholm"],"isPatch":true,"patchVersion":1,"patchTotal":9},"messages":[{"id":"240643","messageId":"1399147942-165308-1-git-send-email-sandals@crustytoothpaste.net","threadId":"36577","inReplyTo":null,"subject":"[RFC PATCH 0/9] Use a structure for object IDs.","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-03T20:12:13Z","receivedAt":"2014-05-03T20:12:13Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"This is a preliminary RFC patch series to move all the relevant uses of\nunsigned char [20] to struct object_id. It should not be applied to any\nbranch yet.\n\nThe goal of this series to improve type-checking in the codebase and to\nmake it easier to move to a different hash function if the project\ndecides to do that. This series does not convert all of the codebase,\nbut only parts. I'm looking for feedback to see if there is consensus\nthat this is the right direction before investing a large amount of\ntime.\n\nCertain parts of the code have to be converted before others to keep the\npatch sizes small, maintainable, and bisectable, so functions and\nstructures that are used across the codebase (e.g. hashcmp and struct\nobject) will be converted later. Conversion has been done in a roughly\nalphabetical order by name of file.\n\nThe constants for raw and hex sizes of SHA-1 values are maintained.\nThese constants are used where the quantity is the size of an SHA-1\nvalue, and sizeof(struct object_id) is used wherever memory is to be\nallocated. This is done to permit the struct to turn into a union later\nif multiple hashes are supported. I left the names at GIT_OID_RAWSZ and\nGIT_OID_HEXSZ because that's what libgit2 uses and what Junio seemed to\nprefer, but they can be changed later if there's a desire to do that.\n\nI called the structure member \"oid\" because it was easily grepable and\ndistinct from the rest of the codebase. It, too, can be changed if we\ndecide on a better name. I specifically did not choose \"sha1\" since it\nlooks weird to have \"sha1->sha1\" and I didn't want to rename lots of\nvariables.\n\nComments?\n\nbrian m. carlson (9):\n Define a structure for object IDs.\n bisect.c: convert to use struct object_id\n archive.c: convert to use struct object_id\n zip: use GIT_OID_HEXSZ for trailers\n branch.c: convert to use struct object_id\n bulk-checkin.c: convert to use struct object_id\n bundle.c: convert leaf functions to struct object_id\n cache-tree: convert struct cache_tree to use object_id\n diff: convert struct combine_diff_path to object_id\n\n archive-zip.c | 4 ++--\n archive.c | 16 +++++++--------\n archive.h | 1 +\n bisect.c | 30 ++++++++++++++--------------\n branch.c | 16 +++++++--------\n builtin/commit.c | 2 +-\n builtin/fsck.c | 4 ++--\n bulk-checkin.c | 12 +++++------\n bundle.c | 38 +++++++++++++++++------------------\n cache-tree.c | 30 ++++++++++++++--------------\n cache-tree.h | 3 ++-\n combine-diff.c | 54 +++++++++++++++++++++++++-------------------------\n diff-lib.c | 10 +++++-----\n diff.h | 5 +++--\n merge-recursive.c | 2 +-\n object.h | 13 +++++++++++-\n reachable.c | 2 +-\n sequencer.c | 2 +-\n test-dump-cache-tree.c | 4 ++--\n 19 files changed, 131 insertions(+), 117 deletions(-)\n\n-- \n2.0.0.rc0\n"},{"id":"240636","messageId":"1399147942-165308-2-git-send-email-sandals@crustytoothpaste.net","threadId":"36577","inReplyTo":"1399147942-165308-1-git-send-email-sandals@crustytoothpaste.net","subject":"[PATCH 1/9] Define a structure for object IDs.","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-03T20:12:14Z","receivedAt":"2014-05-03T20:12:14Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Many places throughout the code use \"unsigned char [20]\" to store object IDs\n(SHA-1 values). This leads to lots of hardcoded numbers throughout the\ncodebase. It also leads to confusion about the purposes of a buffer.\n\nIntroduce a structure for object IDs. This allows us to obtain the benefits\nof compile-time checking for misuse. The structure is expected to remain\nthe same size and have the same alignment requirements on all known\nplatforms, compared to the array of unsigned char.\n\nSigned-off-by: brian m. carlson \n---\n object.h | 13 ++++++++++++-\n 1 file changed, 12 insertions(+), 1 deletion(-)\n\ndiff --git a/object.h b/object.h\nindex 6e12f2c..6a9680d 100644\n--- a/object.h\n+++ b/object.h\n@@ -1,6 +1,17 @@\n #ifndef OBJECT_H\n #define OBJECT_H\n \n+/*\n+ * The length in bytes and in hex digits of an object name (SHA-1 value).\n+ * These are the same names used by libgit2.\n+ */\n+#define GIT_OID_RAWSZ 20\n+#define GIT_OID_HEXSZ 40\n+\n+struct object_id {\n+\tunsigned char oid[GIT_OID_RAWSZ];\n+};\n+\n struct object_list {\n \tstruct object *item;\n \tstruct object_list *next;\n@@ -49,7 +60,7 @@ struct object {\n \tunsigned used : 1;\n \tunsigned type : TYPE_BITS;\n \tunsigned flags : FLAG_BITS;\n-\tunsigned char sha1[20];\n+\tunsigned char sha1[GIT_OID_RAWSZ];\n };\n \n extern const char *typename(unsigned int type);\n-- \n2.0.0.rc0\n"},{"id":"240645","messageId":"1399147942-165308-3-git-send-email-sandals@crustytoothpaste.net","threadId":"36577","inReplyTo":"1399147942-165308-1-git-send-email-sandals@crustytoothpaste.net","subject":"[PATCH 2/9] bisect.c: convert to use struct object_id","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-03T20:12:15Z","receivedAt":"2014-05-03T20:12:15Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Signed-off-by: brian m. carlson \n---\n bisect.c | 30 +++++++++++++++---------------\n 1 file changed, 15 insertions(+), 15 deletions(-)\n\ndiff --git a/bisect.c b/bisect.c\nindex d6e851d..fe53214 100644\n--- a/bisect.c\n+++ b/bisect.c\n@@ -15,7 +15,7 @@\n static struct sha1_array good_revs;\n static struct sha1_array skipped_revs;\n \n-static unsigned char *current_bad_sha1;\n+static struct object_id *current_bad_sha1;\n \n static const char *argv_checkout[] = {\"checkout\", \"-q\", NULL, \"--\", NULL};\n static const char *argv_show_branch[] = {\"show-branch\", NULL, NULL};\n@@ -403,8 +403,8 @@ static int register_ref(const char *refname, const unsigned char *sha1,\n \t\t\tint flags, void *cb_data)\n {\n \tif (!strcmp(refname, \"bad\")) {\n-\t\tcurrent_bad_sha1 = xmalloc(20);\n-\t\thashcpy(current_bad_sha1, sha1);\n+\t\tcurrent_bad_sha1 = xmalloc(sizeof(*current_bad_sha1));\n+\t\thashcpy(current_bad_sha1->oid, sha1);\n \t} else if (starts_with(refname, \"good-\")) {\n \t\tsha1_array_append(&good_revs, sha1);\n \t} else if (starts_with(refname, \"skip-\")) {\n@@ -563,7 +563,7 @@ static struct commit_list *skip_away(struct commit_list *list, int count)\n \n \tfor (i = 0; cur; cur = cur->next, i++) {\n \t\tif (i == index) {\n-\t\t\tif (hashcmp(cur->item->object.sha1, current_bad_sha1))\n+\t\t\tif (hashcmp(cur->item->object.sha1, current_bad_sha1->oid))\n \t\t\t\treturn cur;\n \t\t\tif (previous)\n \t\t\t\treturn previous;\n@@ -606,7 +606,7 @@ static void bisect_rev_setup(struct rev_info *revs, const char *prefix,\n \n \t/* rev_argv.argv[0] will be ignored by setup_revisions */\n \targv_array_push(&rev_argv, \"bisect_rev_setup\");\n-\targv_array_pushf(&rev_argv, bad_format, sha1_to_hex(current_bad_sha1));\n+\targv_array_pushf(&rev_argv, bad_format, sha1_to_hex(current_bad_sha1->oid));\n \tfor (i = 0; i < good_revs.nr; i++)\n \t\targv_array_pushf(&rev_argv, good_format,\n \t\t\t\t sha1_to_hex(good_revs.sha1[i]));\n@@ -627,7 +627,7 @@ static void bisect_common(struct rev_info *revs)\n }\n \n static void exit_if_skipped_commits(struct commit_list *tried,\n-\t\t\t\t const unsigned char *bad)\n+\t\t\t\t const struct object_id *bad)\n {\n \tif (!tried)\n \t\treturn;\n@@ -636,12 +636,12 @@ static void exit_if_skipped_commits(struct commit_list *tried,\n \t \"The first bad commit could be any of:\\n\");\n \tprint_commit_list(tried, \"%s\\n\", \"%s\\n\");\n \tif (bad)\n-\t\tprintf(\"%s\\n\", sha1_to_hex(bad));\n+\t\tprintf(\"%s\\n\", sha1_to_hex(bad->oid));\n \tprintf(\"We cannot bisect more!\\n\");\n \texit(2);\n }\n \n-static int is_expected_rev(const unsigned char *sha1)\n+static int is_expected_rev(const struct object_id *sha1)\n {\n \tconst char *filename = git_path(\"BISECT_EXPECTED_REV\");\n \tstruct stat st;\n@@ -657,7 +657,7 @@ static int is_expected_rev(const unsigned char *sha1)\n \t\treturn 0;\n \n \tif (strbuf_getline(&str, fp, '\\n') != EOF)\n-\t\tres = !strcmp(str.buf, sha1_to_hex(sha1));\n+\t\tres = !strcmp(str.buf, sha1_to_hex(sha1->oid));\n \n \tstrbuf_release(&str);\n \tfclose(fp);\n@@ -718,7 +718,7 @@ static struct commit **get_bad_and_good_commits(int *rev_nr)\n \tstruct commit **rev = xmalloc(len * sizeof(*rev));\n \tint i, n = 0;\n \n-\trev[n++] = get_commit_reference(current_bad_sha1);\n+\trev[n++] = get_commit_reference(current_bad_sha1->oid);\n \tfor (i = 0; i < good_revs.nr; i++)\n \t\trev[n++] = get_commit_reference(good_revs.sha1[i]);\n \t*rev_nr = n;\n@@ -729,7 +729,7 @@ static struct commit **get_bad_and_good_commits(int *rev_nr)\n static void handle_bad_merge_base(void)\n {\n \tif (is_expected_rev(current_bad_sha1)) {\n-\t\tchar *bad_hex = sha1_to_hex(current_bad_sha1);\n+\t\tchar *bad_hex = sha1_to_hex(current_bad_sha1->oid);\n \t\tchar *good_hex = join_sha1_array_hex(&good_revs, ' ');\n \n \t\tfprintf(stderr, \"The merge base %s is bad.\\n\"\n@@ -749,7 +749,7 @@ static void handle_bad_merge_base(void)\n static void handle_skipped_merge_base(const unsigned char *mb)\n {\n \tchar *mb_hex = sha1_to_hex(mb);\n-\tchar *bad_hex = sha1_to_hex(current_bad_sha1);\n+\tchar *bad_hex = sha1_to_hex(current_bad_sha1->oid);\n \tchar *good_hex = join_sha1_array_hex(&good_revs, ' ');\n \n \twarning(\"the merge base between %s and [%s] \"\n@@ -780,7 +780,7 @@ static void check_merge_bases(int no_checkout)\n \n \tfor (; result; result = result->next) {\n \t\tconst unsigned char *mb = result->item->object.sha1;\n-\t\tif (!hashcmp(mb, current_bad_sha1)) {\n+\t\tif (!hashcmp(mb, current_bad_sha1->oid)) {\n \t\t\thandle_bad_merge_base();\n \t\t} else if (0 <= sha1_array_lookup(&good_revs, mb)) {\n \t\t\tcontinue;\n@@ -926,7 +926,7 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \t\texit_if_skipped_commits(tried, NULL);\n \n \t\tprintf(\"%s was both good and bad\\n\",\n-\t\t sha1_to_hex(current_bad_sha1));\n+\t\t sha1_to_hex(current_bad_sha1->oid));\n \t\texit(1);\n \t}\n \n@@ -939,7 +939,7 @@ int bisect_next_all(const char *prefix, int no_checkout)\n \tbisect_rev = revs.commits->item->object.sha1;\n \tmemcpy(bisect_rev_hex, sha1_to_hex(bisect_rev), 41);\n \n-\tif (!hashcmp(bisect_rev, current_bad_sha1)) {\n+\tif (!hashcmp(bisect_rev, current_bad_sha1->oid)) {\n \t\texit_if_skipped_commits(tried, current_bad_sha1);\n \t\tprintf(\"%s is the first bad commit\\n\", bisect_rev_hex);\n \t\tshow_diff_tree(prefix, revs.commits->item);\n-- \n2.0.0.rc0\n"},{"id":"240644","messageId":"1399147942-165308-4-git-send-email-sandals@crustytoothpaste.net","threadId":"36577","inReplyTo":"1399147942-165308-1-git-send-email-sandals@crustytoothpaste.net","subject":"[PATCH 3/9] archive.c: convert to use struct object_id","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-03T20:12:16Z","receivedAt":"2014-05-03T20:12:16Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Signed-off-by: brian m. carlson \n---\n archive.c | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/archive.c b/archive.c\nindex 3fc0fb2..dba148a 100644\n--- a/archive.c\n+++ b/archive.c\n@@ -255,7 +255,7 @@ static void parse_treeish_arg(const char **argv,\n \ttime_t archive_time;\n \tstruct tree *tree;\n \tconst struct commit *commit;\n-\tunsigned char sha1[20];\n+\tstruct object_id sha1;\n \n \t/* Remotes are only allowed to fetch actual refs */\n \tif (remote && !remote_allow_unreachable) {\n@@ -263,15 +263,15 @@ static void parse_treeish_arg(const char **argv,\n \t\tconst char *colon = strchrnul(name, ':');\n \t\tint refnamelen = colon - name;\n \n-\t\tif (!dwim_ref(name, refnamelen, sha1, &ref))\n+\t\tif (!dwim_ref(name, refnamelen, sha1.oid, &ref))\n \t\t\tdie(\"no such ref: %.*s\", refnamelen, name);\n \t\tfree(ref);\n \t}\n \n-\tif (get_sha1(name, sha1))\n+\tif (get_sha1(name, sha1.oid))\n \t\tdie(\"Not a valid object name\");\n \n-\tcommit = lookup_commit_reference_gently(sha1, 1);\n+\tcommit = lookup_commit_reference_gently(sha1.oid, 1);\n \tif (commit) {\n \t\tcommit_sha1 = commit->object.sha1;\n \t\tarchive_time = commit->date;\n@@ -280,21 +280,21 @@ static void parse_treeish_arg(const char **argv,\n \t\tarchive_time = time(NULL);\n \t}\n \n-\ttree = parse_tree_indirect(sha1);\n+\ttree = parse_tree_indirect(sha1.oid);\n \tif (tree == NULL)\n \t\tdie(\"not a tree object\");\n \n \tif (prefix) {\n-\t\tunsigned char tree_sha1[20];\n+\t\tstruct object_id tree_sha1;\n \t\tunsigned int mode;\n \t\tint err;\n \n \t\terr = get_tree_entry(tree->object.sha1, prefix,\n-\t\t\t\t tree_sha1, &mode);\n+\t\t\t\t tree_sha1.oid, &mode);\n \t\tif (err || !S_ISDIR(mode))\n \t\t\tdie(\"current working directory is untracked\");\n \n-\t\ttree = parse_tree_indirect(tree_sha1);\n+\t\ttree = parse_tree_indirect(tree_sha1.oid);\n \t}\n \tar_args->tree = tree;\n \tar_args->commit_sha1 = commit_sha1;\n-- \n2.0.0.rc0\n"},{"id":"240637","messageId":"1399147942-165308-5-git-send-email-sandals@crustytoothpaste.net","threadId":"36577","inReplyTo":"1399147942-165308-1-git-send-email-sandals@crustytoothpaste.net","subject":"[PATCH 4/9] zip: use GIT_OID_HEXSZ for trailers","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-03T20:12:17Z","receivedAt":"2014-05-03T20:12:17Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"The object.h header is included in archive.h for this constant. It will be\nused by other parts of the archiving code in the future.\n\nSigned-off-by: brian m. carlson \n---\n archive-zip.c | 4 ++--\n archive.h | 1 +\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/archive-zip.c b/archive-zip.c\nindex 4bde019..5b9fe42 100644\n--- a/archive-zip.c\n+++ b/archive-zip.c\n@@ -427,12 +427,12 @@ static void write_zip_trailer(const unsigned char *sha1)\n \tcopy_le16(trailer.entries, zip_dir_entries);\n \tcopy_le32(trailer.size, zip_dir_offset);\n \tcopy_le32(trailer.offset, zip_offset);\n-\tcopy_le16(trailer.comment_length, sha1 ? 40 : 0);\n+\tcopy_le16(trailer.comment_length, sha1 ? GIT_OID_HEXSZ : 0);\n \n \twrite_or_die(1, zip_dir, zip_dir_offset);\n \twrite_or_die(1, &trailer, ZIP_DIR_TRAILER_SIZE);\n \tif (sha1)\n-\t\twrite_or_die(1, sha1_to_hex(sha1), 40);\n+\t\twrite_or_die(1, sha1_to_hex(sha1), GIT_OID_HEXSZ);\n }\n \n static void dos_time(time_t *time, int *dos_date, int *dos_time)\ndiff --git a/archive.h b/archive.h\nindex 4a791e1..fd21408 100644\n--- a/archive.h\n+++ b/archive.h\n@@ -2,6 +2,7 @@\n #define ARCHIVE_H\n \n #include \"pathspec.h\"\n+#include \"object.h\"\n \n struct archiver_args {\n \tconst char *base;\n-- \n2.0.0.rc0\n"},{"id":"240639","messageId":"1399147942-165308-6-git-send-email-sandals@crustytoothpaste.net","threadId":"36577","inReplyTo":"1399147942-165308-1-git-send-email-sandals@crustytoothpaste.net","subject":"[PATCH 5/9] branch.c: convert to use struct object_id","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-03T20:12:18Z","receivedAt":"2014-05-03T20:12:18Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Signed-off-by: brian m. carlson \n---\n branch.c | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git a/branch.c b/branch.c\nindex 660097b..8dc0d49 100644\n--- a/branch.c\n+++ b/branch.c\n@@ -184,9 +184,9 @@ int validate_new_branchname(const char *name, struct strbuf *ref,\n \n \tif (!attr_only) {\n \t\tconst char *head;\n-\t\tunsigned char sha1[20];\n+\t\tstruct object_id sha1;\n \n-\t\thead = resolve_ref_unsafe(\"HEAD\", sha1, 0, NULL);\n+\t\thead = resolve_ref_unsafe(\"HEAD\", sha1.oid, 0, NULL);\n \t\tif (!is_bare_repository() && head && !strcmp(head, ref->buf))\n \t\t\tdie(_(\"Cannot force update the current branch.\"));\n \t}\n@@ -228,7 +228,7 @@ void create_branch(const char *head,\n {\n \tstruct ref_lock *lock = NULL;\n \tstruct commit *commit;\n-\tunsigned char sha1[20];\n+\tstruct object_id sha1;\n \tchar *real_ref, msg[PATH_MAX + 20];\n \tstruct strbuf ref = STRBUF_INIT;\n \tint forcing = 0;\n@@ -248,7 +248,7 @@ void create_branch(const char *head,\n \t}\n \n \treal_ref = NULL;\n-\tif (get_sha1(start_name, sha1)) {\n+\tif (get_sha1(start_name, sha1.oid)) {\n \t\tif (explicit_tracking) {\n \t\t\tif (advice_set_upstream_failure) {\n \t\t\t\terror(_(upstream_missing), start_name);\n@@ -260,7 +260,7 @@ void create_branch(const char *head,\n \t\tdie(_(\"Not a valid object name: '%s'.\"), start_name);\n \t}\n \n-\tswitch (dwim_ref(start_name, strlen(start_name), sha1, &real_ref)) {\n+\tswitch (dwim_ref(start_name, strlen(start_name), sha1.oid, &real_ref)) {\n \tcase 0:\n \t\t/* Not branching from any existing branch */\n \t\tif (explicit_tracking)\n@@ -281,9 +281,9 @@ void create_branch(const char *head,\n \t\tbreak;\n \t}\n \n-\tif ((commit = lookup_commit_reference(sha1)) == NULL)\n+\tif ((commit = lookup_commit_reference(sha1.oid)) == NULL)\n \t\tdie(_(\"Not a valid branch point: '%s'.\"), start_name);\n-\thashcpy(sha1, commit->object.sha1);\n+\thashcpy(sha1.oid, commit->object.sha1);\n \n \tif (!dont_change_ref) {\n \t\tlock = lock_any_ref_for_update(ref.buf, NULL, 0, NULL);\n@@ -305,7 +305,7 @@ void create_branch(const char *head,\n \t\tsetup_tracking(ref.buf + 11, real_ref, track, quiet);\n \n \tif (!dont_change_ref)\n-\t\tif (write_ref_sha1(lock, sha1, msg) < 0)\n+\t\tif (write_ref_sha1(lock, sha1.oid, msg) < 0)\n \t\t\tdie_errno(_(\"Failed to write ref\"));\n \n \tstrbuf_release(&ref);\n-- \n2.0.0.rc0\n"},{"id":"240642","messageId":"1399147942-165308-7-git-send-email-sandals@crustytoothpaste.net","threadId":"36577","inReplyTo":"1399147942-165308-1-git-send-email-sandals@crustytoothpaste.net","subject":"[PATCH 6/9] bulk-checkin.c: convert to use struct object_id","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-03T20:12:19Z","receivedAt":"2014-05-03T20:12:19Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Signed-off-by: brian m. carlson \n---\n bulk-checkin.c | 12 ++++++------\n 1 file changed, 6 insertions(+), 6 deletions(-)\n\ndiff --git a/bulk-checkin.c b/bulk-checkin.c\nindex 98e651c..92c7b5e 100644\n--- a/bulk-checkin.c\n+++ b/bulk-checkin.c\n@@ -23,7 +23,7 @@ static struct bulk_checkin_state {\n \n static void finish_bulk_checkin(struct bulk_checkin_state *state)\n {\n-\tunsigned char sha1[20];\n+\tstruct object_id sha1;\n \tstruct strbuf packname = STRBUF_INIT;\n \tint i;\n \n@@ -35,11 +35,11 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state)\n \t\tunlink(state->pack_tmp_name);\n \t\tgoto clear_exit;\n \t} else if (state->nr_written == 1) {\n-\t\tsha1close(state->f, sha1, CSUM_FSYNC);\n+\t\tsha1close(state->f, sha1.oid, CSUM_FSYNC);\n \t} else {\n-\t\tint fd = sha1close(state->f, sha1, 0);\n-\t\tfixup_pack_header_footer(fd, sha1, state->pack_tmp_name,\n-\t\t\t\t\t state->nr_written, sha1,\n+\t\tint fd = sha1close(state->f, sha1.oid, 0);\n+\t\tfixup_pack_header_footer(fd, sha1.oid, state->pack_tmp_name,\n+\t\t\t\t\t state->nr_written, sha1.oid,\n \t\t\t\t\t state->offset);\n \t\tclose(fd);\n \t}\n@@ -47,7 +47,7 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state)\n \tstrbuf_addf(&packname, \"%s/pack/pack-\", get_object_directory());\n \tfinish_tmp_packfile(&packname, state->pack_tmp_name,\n \t\t\t state->written, state->nr_written,\n-\t\t\t &state->pack_idx_opts, sha1);\n+\t\t\t &state->pack_idx_opts, sha1.oid);\n \tfor (i = 0; i < state->nr_written; i++)\n \t\tfree(state->written[i]);\n \n-- \n2.0.0.rc0\n"},{"id":"240640","messageId":"1399147942-165308-8-git-send-email-sandals@crustytoothpaste.net","threadId":"36577","inReplyTo":"1399147942-165308-1-git-send-email-sandals@crustytoothpaste.net","subject":"[PATCH 7/9] bundle.c: convert leaf functions to struct object_id","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-03T20:12:20Z","receivedAt":"2014-05-03T20:12:20Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Signed-off-by: brian m. carlson \n---\n bundle.c | 38 +++++++++++++++++++-------------------\n 1 file changed, 19 insertions(+), 19 deletions(-)\n\ndiff --git a/bundle.c b/bundle.c\nindex 1222952..798ba28 100644\n--- a/bundle.c\n+++ b/bundle.c\n@@ -11,11 +11,11 @@\n \n static const char bundle_signature[] = \"# v2 git bundle\\n\";\n \n-static void add_to_ref_list(const unsigned char *sha1, const char *name,\n+static void add_to_ref_list(const struct object_id *sha1, const char *name,\n \t\tstruct ref_list *list)\n {\n \tALLOC_GROW(list->list, list->nr + 1, list->alloc);\n-\thashcpy(list->list[list->nr].sha1, sha1);\n+\thashcpy(list->list[list->nr].sha1, sha1->oid);\n \tlist->list[list->nr].name = xstrdup(name);\n \tlist->nr++;\n }\n@@ -39,7 +39,7 @@ static int parse_bundle_header(int fd, struct bundle_header *header,\n \t/* The bundle header ends with an empty line */\n \twhile (!strbuf_getwholeline_fd(&buf, fd, '\\n') &&\n \t buf.len && buf.buf[0] != '\\n') {\n-\t\tunsigned char sha1[20];\n+\t\tstruct object_id sha1;\n \t\tint is_prereq = 0;\n \n \t\tif (*buf.buf == '-') {\n@@ -53,9 +53,9 @@ static int parse_bundle_header(int fd, struct bundle_header *header,\n \t\t * Prerequisites have object name that is optionally\n \t\t * followed by SP and subject line.\n \t\t */\n-\t\tif (get_sha1_hex(buf.buf, sha1) ||\n-\t\t (buf.len > 40 && !isspace(buf.buf[40])) ||\n-\t\t (!is_prereq && buf.len <= 40)) {\n+\t\tif (get_sha1_hex(buf.buf, sha1.oid) ||\n+\t\t (buf.len > GIT_OID_HEXSZ && !isspace(buf.buf[GIT_OID_HEXSZ])) ||\n+\t\t (!is_prereq && buf.len <= GIT_OID_HEXSZ)) {\n \t\t\tif (report_path)\n \t\t\t\terror(_(\"unrecognized header: %s%s (%d)\"),\n \t\t\t\t (is_prereq ? \"-\" : \"\"), buf.buf, (int)buf.len);\n@@ -63,9 +63,9 @@ static int parse_bundle_header(int fd, struct bundle_header *header,\n \t\t\tbreak;\n \t\t} else {\n \t\t\tif (is_prereq)\n-\t\t\t\tadd_to_ref_list(sha1, \"\", &header->prerequisites);\n+\t\t\t\tadd_to_ref_list(&sha1, \"\", &header->prerequisites);\n \t\t\telse\n-\t\t\t\tadd_to_ref_list(sha1, buf.buf + 41, &header->references);\n+\t\t\t\tadd_to_ref_list(&sha1, buf.buf + 41, &header->references);\n \t\t}\n \t}\n \n@@ -274,16 +274,16 @@ int create_bundle(struct bundle_header *header, const char *path,\n \t\treturn -1;\n \trls_fout = xfdopen(rls.out, \"r\");\n \twhile (strbuf_getwholeline(&buf, rls_fout, '\\n') != EOF) {\n-\t\tunsigned char sha1[20];\n+\t\tstruct object_id sha1;\n \t\tif (buf.len > 0 && buf.buf[0] == '-') {\n \t\t\twrite_or_die(bundle_fd, buf.buf, buf.len);\n-\t\t\tif (!get_sha1_hex(buf.buf + 1, sha1)) {\n-\t\t\t\tstruct object *object = parse_object_or_die(sha1, buf.buf);\n+\t\t\tif (!get_sha1_hex(buf.buf + 1, sha1.oid)) {\n+\t\t\t\tstruct object *object = parse_object_or_die(sha1.oid, buf.buf);\n \t\t\t\tobject->flags |= UNINTERESTING;\n \t\t\t\tadd_pending_object(&revs, object, buf.buf);\n \t\t\t}\n-\t\t} else if (!get_sha1_hex(buf.buf, sha1)) {\n-\t\t\tstruct object *object = parse_object_or_die(sha1, buf.buf);\n+\t\t} else if (!get_sha1_hex(buf.buf, sha1.oid)) {\n+\t\t\tstruct object *object = parse_object_or_die(sha1.oid, buf.buf);\n \t\t\tobject->flags |= SHOWN;\n \t\t}\n \t}\n@@ -302,16 +302,16 @@ int create_bundle(struct bundle_header *header, const char *path,\n \n \tfor (i = 0; i < revs.pending.nr; i++) {\n \t\tstruct object_array_entry *e = revs.pending.objects + i;\n-\t\tunsigned char sha1[20];\n+\t\tstruct object_id sha1;\n \t\tchar *ref;\n \t\tconst char *display_ref;\n \t\tint flag;\n \n \t\tif (e->item->flags & UNINTERESTING)\n \t\t\tcontinue;\n-\t\tif (dwim_ref(e->name, strlen(e->name), sha1, &ref) != 1)\n+\t\tif (dwim_ref(e->name, strlen(e->name), sha1.oid, &ref) != 1)\n \t\t\tcontinue;\n-\t\tif (read_ref_full(e->name, sha1, 1, &flag))\n+\t\tif (read_ref_full(e->name, sha1.oid, 1, &flag))\n \t\t\tflag = 0;\n \t\tdisplay_ref = (flag & REF_ISSYMREF) ? e->name : ref;\n \n@@ -342,13 +342,13 @@ int create_bundle(struct bundle_header *header, const char *path,\n \t\t * commit that is referenced by the tag, and not the tag\n \t\t * itself.\n \t\t */\n-\t\tif (hashcmp(sha1, e->item->sha1)) {\n+\t\tif (hashcmp(sha1.oid, e->item->sha1)) {\n \t\t\t/*\n \t\t\t * Is this the positive end of a range expressed\n \t\t\t * in terms of a tag (e.g. v2.0 from the range\n \t\t\t * \"v1.0..v2.0\")?\n \t\t\t */\n-\t\t\tstruct commit *one = lookup_commit_reference(sha1);\n+\t\t\tstruct commit *one = lookup_commit_reference(sha1.oid);\n \t\t\tstruct object *obj;\n \n \t\t\tif (e->item == &(one->object)) {\n@@ -360,7 +360,7 @@ int create_bundle(struct bundle_header *header, const char *path,\n \t\t\t\t * end up triggering \"empty bundle\"\n \t\t\t\t * error.\n \t\t\t\t */\n-\t\t\t\tobj = parse_object_or_die(sha1, e->name);\n+\t\t\t\tobj = parse_object_or_die(sha1.oid, e->name);\n \t\t\t\tobj->flags |= SHOWN;\n \t\t\t\tadd_pending_object(&revs, obj, e->name);\n \t\t\t}\n-- \n2.0.0.rc0\n"},{"id":"240638","messageId":"1399147942-165308-9-git-send-email-sandals@crustytoothpaste.net","threadId":"36577","inReplyTo":"1399147942-165308-1-git-send-email-sandals@crustytoothpaste.net","subject":"[PATCH 8/9] cache-tree: convert struct cache_tree to use object_id","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-03T20:12:21Z","receivedAt":"2014-05-03T20:12:21Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Signed-off-by: brian m. carlson \n---\n builtin/commit.c | 2 +-\n builtin/fsck.c | 4 ++--\n cache-tree.c | 30 +++++++++++++++---------------\n cache-tree.h | 3 ++-\n merge-recursive.c | 2 +-\n reachable.c | 2 +-\n sequencer.c | 2 +-\n test-dump-cache-tree.c | 4 ++--\n 8 files changed, 25 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 9cfef6c..639f843 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1659,7 +1659,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tappend_merge_tag_headers(parents, &tail);\n \t}\n \n-\tif (commit_tree_extended(&sb, active_cache_tree->sha1, parents, sha1,\n+\tif (commit_tree_extended(&sb, active_cache_tree->sha1.oid, parents, sha1,\n \t\t\t\t author_ident.buf, sign_commit, extra)) {\n \t\trollback_index_files();\n \t\tdie(_(\"failed to write commit object\"));\ndiff --git a/builtin/fsck.c b/builtin/fsck.c\nindex fc150c8..6854c81 100644\n--- a/builtin/fsck.c\n+++ b/builtin/fsck.c\n@@ -587,10 +587,10 @@ static int fsck_cache_tree(struct cache_tree *it)\n \t\tfprintf(stderr, \"Checking cache tree\\n\");\n \n \tif (0 <= it->entry_count) {\n-\t\tstruct object *obj = parse_object(it->sha1);\n+\t\tstruct object *obj = parse_object(it->sha1.oid);\n \t\tif (!obj) {\n \t\t\terror(\"%s: invalid sha1 pointer in cache-tree\",\n-\t\t\t sha1_to_hex(it->sha1));\n+\t\t\t sha1_to_hex(it->sha1.oid));\n \t\t\treturn 1;\n \t\t}\n \t\tobj->used = 1;\ndiff --git a/cache-tree.c b/cache-tree.c\nindex 7fa524a..b7b2d06 100644\n--- a/cache-tree.c\n+++ b/cache-tree.c\n@@ -219,7 +219,7 @@ int cache_tree_fully_valid(struct cache_tree *it)\n \tint i;\n \tif (!it)\n \t\treturn 0;\n-\tif (it->entry_count < 0 || !has_sha1_file(it->sha1))\n+\tif (it->entry_count < 0 || !has_sha1_file(it->sha1.oid))\n \t\treturn 0;\n \tfor (i = 0; i < it->subtree_nr; i++) {\n \t\tif (!cache_tree_fully_valid(it->down[i]->cache_tree))\n@@ -244,7 +244,7 @@ static int update_one(struct cache_tree *it,\n \n \t*skip_count = 0;\n \n-\tif (0 <= it->entry_count && has_sha1_file(it->sha1))\n+\tif (0 <= it->entry_count && has_sha1_file(it->sha1.oid))\n \t\treturn it->entry_count;\n \n \t/*\n@@ -311,7 +311,7 @@ static int update_one(struct cache_tree *it,\n \t\tstruct cache_tree_sub *sub;\n \t\tconst char *path, *slash;\n \t\tint pathlen, entlen;\n-\t\tconst unsigned char *sha1;\n+\t\tconst struct object_id *sha1;\n \t\tunsigned mode;\n \n \t\tpath = ce->name;\n@@ -327,21 +327,21 @@ static int update_one(struct cache_tree *it,\n \t\t\t\tdie(\"cache-tree.c: '%.*s' in '%s' not found\",\n \t\t\t\t entlen, path + baselen, path);\n \t\t\ti += sub->count;\n-\t\t\tsha1 = sub->cache_tree->sha1;\n+\t\t\tsha1 = &sub->cache_tree->sha1;\n \t\t\tmode = S_IFDIR;\n \t\t\tif (sub->cache_tree->entry_count < 0)\n \t\t\t\tto_invalidate = 1;\n \t\t}\n \t\telse {\n-\t\t\tsha1 = ce->sha1;\n+\t\t\tsha1 = (struct object_id *)ce->sha1;\n \t\t\tmode = ce->ce_mode;\n \t\t\tentlen = pathlen - baselen;\n \t\t\ti++;\n \t\t}\n-\t\tif (mode != S_IFGITLINK && !missing_ok && !has_sha1_file(sha1)) {\n+\t\tif (mode != S_IFGITLINK && !missing_ok && !has_sha1_file(sha1->oid)) {\n \t\t\tstrbuf_release(&buffer);\n \t\t\treturn error(\"invalid object %06o %s for '%.*s'\",\n-\t\t\t\tmode, sha1_to_hex(sha1), entlen+baselen, path);\n+\t\t\t\tmode, sha1_to_hex(sha1->oid), entlen+baselen, path);\n \t\t}\n \n \t\t/*\n@@ -375,8 +375,8 @@ static int update_one(struct cache_tree *it,\n \t}\n \n \tif (dryrun)\n-\t\thash_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1);\n-\telse if (write_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1)) {\n+\t\thash_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1.oid);\n+\telse if (write_sha1_file(buffer.buf, buffer.len, tree_type, it->sha1.oid)) {\n \t\tstrbuf_release(&buffer);\n \t\treturn -1;\n \t}\n@@ -432,7 +432,7 @@ static void write_one(struct strbuf *buffer, struct cache_tree *it,\n #endif\n \n \tif (0 <= it->entry_count) {\n-\t\tstrbuf_add(buffer, it->sha1, 20);\n+\t\tstrbuf_add(buffer, it->sha1.oid, GIT_OID_RAWSZ);\n \t}\n \tfor (i = 0; i < it->subtree_nr; i++) {\n \t\tstruct cache_tree_sub *down = it->down[i];\n@@ -489,7 +489,7 @@ static struct cache_tree *read_one(const char **buffer, unsigned long *size_p)\n \tif (0 <= it->entry_count) {\n \t\tif (size < 20)\n \t\t\tgoto free_return;\n-\t\thashcpy(it->sha1, (const unsigned char*)buf);\n+\t\thashcpy(it->sha1.oid, (const unsigned char*)buf);\n \t\tbuf += 20;\n \t\tsize -= 20;\n \t}\n@@ -612,10 +612,10 @@ int write_cache_as_tree(unsigned char *sha1, int flags, const char *prefix)\n \t\t\tcache_tree_find(active_cache_tree, prefix);\n \t\tif (!subtree)\n \t\t\treturn WRITE_TREE_PREFIX_ERROR;\n-\t\thashcpy(sha1, subtree->sha1);\n+\t\thashcpy(sha1, subtree->sha1.oid);\n \t}\n \telse\n-\t\thashcpy(sha1, active_cache_tree->sha1);\n+\t\thashcpy(sha1, active_cache_tree->sha1.oid);\n \n \tif (0 <= newfd)\n \t\trollback_lock_file(lock_file);\n@@ -629,7 +629,7 @@ static void prime_cache_tree_rec(struct cache_tree *it, struct tree *tree)\n \tstruct name_entry entry;\n \tint cnt;\n \n-\thashcpy(it->sha1, tree->object.sha1);\n+\thashcpy(it->sha1.oid, tree->object.sha1);\n \tinit_tree_desc(&desc, tree->buffer, tree->size);\n \tcnt = 0;\n \twhile (tree_entry(&desc, &entry)) {\n@@ -683,7 +683,7 @@ int cache_tree_matches_traversal(struct cache_tree *root,\n \n \tit = find_cache_tree_from_traversal(root, info);\n \tit = cache_tree_find(it, ent->path);\n-\tif (it && it->entry_count > 0 && !hashcmp(ent->sha1, it->sha1))\n+\tif (it && it->entry_count > 0 && !hashcmp(ent->sha1, it->sha1.oid))\n \t\treturn it->entry_count;\n \treturn 0;\n }\ndiff --git a/cache-tree.h b/cache-tree.h\nindex f1923ad..a65231e 100644\n--- a/cache-tree.h\n+++ b/cache-tree.h\n@@ -3,6 +3,7 @@\n \n #include \"tree.h\"\n #include \"tree-walk.h\"\n+#include \"object.h\"\n \n struct cache_tree;\n struct cache_tree_sub {\n@@ -15,7 +16,7 @@ struct cache_tree_sub {\n \n struct cache_tree {\n \tint entry_count; /* negative means \"invalid\" */\n-\tunsigned char sha1[20];\n+\tstruct object_id sha1;\n \tint subtree_nr;\n \tint subtree_alloc;\n \tstruct cache_tree_sub **down;\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex 4177092..7db772d 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -270,7 +270,7 @@ struct tree *write_tree_from_memory(struct merge_options *o)\n \t\t\t active_nr, 0) < 0)\n \t\tdie(_(\"error building trees\"));\n \n-\tresult = lookup_tree(active_cache_tree->sha1);\n+\tresult = lookup_tree(active_cache_tree->sha1.oid);\n \n \treturn result;\n }\ndiff --git a/reachable.c b/reachable.c\nindex 654a8c5..464c5ef 100644\n--- a/reachable.c\n+++ b/reachable.c\n@@ -177,7 +177,7 @@ static void add_cache_tree(struct cache_tree *it, struct rev_info *revs)\n \tint i;\n \n \tif (it->entry_count >= 0)\n-\t\tadd_one_tree(it->sha1, revs);\n+\t\tadd_one_tree(it->sha1.oid, revs);\n \tfor (i = 0; i < it->subtree_nr; i++)\n \t\tadd_cache_tree(it->down[i]->cache_tree, revs);\n }\ndiff --git a/sequencer.c b/sequencer.c\nindex bde5f04..fe48518 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -377,7 +377,7 @@ static int is_index_unchanged(void)\n \t\t\t\t active_nr, 0))\n \t\t\treturn error(_(\"Unable to update cache tree\\n\"));\n \n-\treturn !hashcmp(active_cache_tree->sha1, head_commit->tree->object.sha1);\n+\treturn !hashcmp(active_cache_tree->sha1.oid, head_commit->tree->object.sha1);\n }\n \n /*\ndiff --git a/test-dump-cache-tree.c b/test-dump-cache-tree.c\nindex 47eab97..9d97908 100644\n--- a/test-dump-cache-tree.c\n+++ b/test-dump-cache-tree.c\n@@ -10,7 +10,7 @@ static void dump_one(struct cache_tree *it, const char *pfx, const char *x)\n \t\t \"invalid\", x, pfx, it->subtree_nr);\n \telse\n \t\tprintf(\"%s %s%s (%d entries, %d subtrees)\\n\",\n-\t\t sha1_to_hex(it->sha1), x, pfx,\n+\t\t sha1_to_hex(it->sha1.oid), x, pfx,\n \t\t it->entry_count, it->subtree_nr);\n }\n \n@@ -33,7 +33,7 @@ static int dump_cache_tree(struct cache_tree *it,\n \t}\n \telse {\n \t\tdump_one(it, pfx, \"\");\n-\t\tif (hashcmp(it->sha1, ref->sha1) ||\n+\t\tif (hashcmp(it->sha1.oid, ref->sha1.oid) ||\n \t\t ref->entry_count != it->entry_count ||\n \t\t ref->subtree_nr != it->subtree_nr) {\n \t\t\tdump_one(ref, pfx, \"#(ref) \");\n-- \n2.0.0.rc0\n"},{"id":"240641","messageId":"1399147942-165308-10-git-send-email-sandals@crustytoothpaste.net","threadId":"36577","inReplyTo":"1399147942-165308-1-git-send-email-sandals@crustytoothpaste.net","subject":"[PATCH 9/9] diff: convert struct combine_diff_path to object_id","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-03T20:12:22Z","receivedAt":"2014-05-03T20:12:22Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"Signed-off-by: brian m. carlson \n---\n combine-diff.c | 54 +++++++++++++++++++++++++++---------------------------\n diff-lib.c | 10 +++++-----\n diff.h | 5 +++--\n 3 files changed, 35 insertions(+), 34 deletions(-)\n\ndiff --git a/combine-diff.c b/combine-diff.c\nindex 24ca7e2..f97eb3a 100644\n--- a/combine-diff.c\n+++ b/combine-diff.c\n@@ -34,9 +34,9 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\t\tmemset(p->parent, 0,\n \t\t\t sizeof(p->parent[0]) * num_parent);\n \n-\t\t\thashcpy(p->sha1, q->queue[i]->two->sha1);\n+\t\t\thashcpy(p->sha1.oid, q->queue[i]->two->sha1);\n \t\t\tp->mode = q->queue[i]->two->mode;\n-\t\t\thashcpy(p->parent[n].sha1, q->queue[i]->one->sha1);\n+\t\t\thashcpy(p->parent[n].sha1.oid, q->queue[i]->one->sha1);\n \t\t\tp->parent[n].mode = q->queue[i]->one->mode;\n \t\t\tp->parent[n].status = q->queue[i]->status;\n \t\t\t*tail = p;\n@@ -67,7 +67,7 @@ static struct combine_diff_path *intersect_paths(struct combine_diff_path *curr,\n \t\t\tcontinue;\n \t\t}\n \n-\t\thashcpy(p->parent[n].sha1, q->queue[i]->one->sha1);\n+\t\thashcpy(p->parent[n].sha1.oid, q->queue[i]->one->sha1);\n \t\tp->parent[n].mode = q->queue[i]->one->mode;\n \t\tp->parent[n].status = q->queue[i]->status;\n \n@@ -274,7 +274,7 @@ static struct lline *coalesce_lines(struct lline *base, int *lenbase,\n \treturn base;\n }\n \n-static char *grab_blob(const unsigned char *sha1, unsigned int mode,\n+static char *grab_blob(const struct object_id *sha1, unsigned int mode,\n \t\t unsigned long *size, struct userdiff_driver *textconv,\n \t\t const char *path)\n {\n@@ -284,20 +284,20 @@ static char *grab_blob(const unsigned char *sha1, unsigned int mode,\n \tif (S_ISGITLINK(mode)) {\n \t\tblob = xmalloc(100);\n \t\t*size = snprintf(blob, 100,\n-\t\t\t\t \"Subproject commit %s\\n\", sha1_to_hex(sha1));\n-\t} else if (is_null_sha1(sha1)) {\n+\t\t\t\t \"Subproject commit %s\\n\", sha1_to_hex(sha1->oid));\n+\t} else if (is_null_sha1(sha1->oid)) {\n \t\t/* deleted blob */\n \t\t*size = 0;\n \t\treturn xcalloc(1, 1);\n \t} else if (textconv) {\n \t\tstruct diff_filespec *df = alloc_filespec(path);\n-\t\tfill_filespec(df, sha1, 1, mode);\n+\t\tfill_filespec(df, sha1->oid, 1, mode);\n \t\t*size = fill_textconv(textconv, df, &blob);\n \t\tfree_filespec(df);\n \t} else {\n-\t\tblob = read_sha1_file(sha1, &type, size);\n+\t\tblob = read_sha1_file(sha1->oid, &type, size);\n \t\tif (type != OBJ_BLOB)\n-\t\t\tdie(\"object '%s' is not a blob!\", sha1_to_hex(sha1));\n+\t\t\tdie(\"object '%s' is not a blob!\", sha1_to_hex(sha1->oid));\n \t}\n \treturn blob;\n }\n@@ -379,7 +379,7 @@ static void consume_line(void *state_, char *line, unsigned long len)\n \t}\n }\n \n-static void combine_diff(const unsigned char *parent, unsigned int mode,\n+static void combine_diff(const struct object_id *parent, unsigned int mode,\n \t\t\t mmfile_t *result_file,\n \t\t\t struct sline *sline, unsigned int cnt, int n,\n \t\t\t int num_parent, int result_deleted,\n@@ -904,11 +904,11 @@ static void show_combined_header(struct combine_diff_path *elem,\n \t\t\t \"\", elem->path, line_prefix, c_meta, c_reset);\n \tprintf(\"%s%sindex \", line_prefix, c_meta);\n \tfor (i = 0; i < num_parent; i++) {\n-\t\tabb = find_unique_abbrev(elem->parent[i].sha1,\n+\t\tabb = find_unique_abbrev(elem->parent[i].sha1.oid,\n \t\t\t\t\t abbrev);\n \t\tprintf(\"%s%s\", i ? \",\" : \"\", abb);\n \t}\n-\tabb = find_unique_abbrev(elem->sha1, abbrev);\n+\tabb = find_unique_abbrev(elem->sha1.oid, abbrev);\n \tprintf(\"..%s%s\\n\", abb, c_reset);\n \n \tif (mode_differs) {\n@@ -981,7 +981,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \n \t/* Read the result of merge first */\n \tif (!working_tree_file)\n-\t\tresult = grab_blob(elem->sha1, elem->mode, &result_size,\n+\t\tresult = grab_blob(&elem->sha1, elem->mode, &result_size,\n \t\t\t\t textconv, elem->path);\n \telse {\n \t\t/* Used by diff-tree to read from the working tree */\n@@ -1003,12 +1003,12 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \t\t\tresult = strbuf_detach(&buf, NULL);\n \t\t\telem->mode = canon_mode(st.st_mode);\n \t\t} else if (S_ISDIR(st.st_mode)) {\n-\t\t\tunsigned char sha1[20];\n-\t\t\tif (resolve_gitlink_ref(elem->path, \"HEAD\", sha1) < 0)\n-\t\t\t\tresult = grab_blob(elem->sha1, elem->mode,\n+\t\t\tstruct object_id sha1;\n+\t\t\tif (resolve_gitlink_ref(elem->path, \"HEAD\", sha1.oid) < 0)\n+\t\t\t\tresult = grab_blob(&elem->sha1, elem->mode,\n \t\t\t\t\t\t &result_size, NULL, NULL);\n \t\t\telse\n-\t\t\t\tresult = grab_blob(sha1, elem->mode,\n+\t\t\t\tresult = grab_blob(&sha1, elem->mode,\n \t\t\t\t\t\t &result_size, NULL, NULL);\n \t\t} else if (textconv) {\n \t\t\tstruct diff_filespec *df = alloc_filespec(elem->path);\n@@ -1080,7 +1080,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \t\tfor (i = 0; !is_binary && i < num_parent; i++) {\n \t\t\tchar *buf;\n \t\t\tunsigned long size;\n-\t\t\tbuf = grab_blob(elem->parent[i].sha1,\n+\t\t\tbuf = grab_blob(&elem->parent[i].sha1,\n \t\t\t\t\telem->parent[i].mode,\n \t\t\t\t\t&size, NULL, NULL);\n \t\t\tif (buffer_is_binary(buf, size))\n@@ -1129,14 +1129,14 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,\n \tfor (i = 0; i < num_parent; i++) {\n \t\tint j;\n \t\tfor (j = 0; j < i; j++) {\n-\t\t\tif (!hashcmp(elem->parent[i].sha1,\n-\t\t\t\t elem->parent[j].sha1)) {\n+\t\t\tif (!hashcmp(elem->parent[i].sha1.oid,\n+\t\t\t\t elem->parent[j].sha1.oid)) {\n \t\t\t\treuse_combine_diff(sline, cnt, i, j);\n \t\t\t\tbreak;\n \t\t\t}\n \t\t}\n \t\tif (i <= j)\n-\t\t\tcombine_diff(elem->parent[i].sha1,\n+\t\t\tcombine_diff(&elem->parent[i].sha1,\n \t\t\t\t elem->parent[i].mode,\n \t\t\t\t &result_file, sline,\n \t\t\t\t cnt, i, num_parent, result_deleted,\n@@ -1196,9 +1196,9 @@ static void show_raw_diff(struct combine_diff_path *p, int num_parent, struct re\n \n \t\t/* Show sha1's */\n \t\tfor (i = 0; i < num_parent; i++)\n-\t\t\tprintf(\" %s\", diff_unique_abbrev(p->parent[i].sha1,\n+\t\t\tprintf(\" %s\", diff_unique_abbrev(p->parent[i].sha1.oid,\n \t\t\t\t\t\t\t opt->abbrev));\n-\t\tprintf(\" %s \", diff_unique_abbrev(p->sha1, opt->abbrev));\n+\t\tprintf(\" %s \", diff_unique_abbrev(p->sha1.oid, opt->abbrev));\n \t}\n \n \tif (opt->output_format & (DIFF_FORMAT_RAW | DIFF_FORMAT_NAME_STATUS)) {\n@@ -1261,16 +1261,16 @@ static struct diff_filepair *combined_pair(struct combine_diff_path *p,\n \tfor (i = 0; i < num_parent; i++) {\n \t\tpair->one[i].path = p->path;\n \t\tpair->one[i].mode = p->parent[i].mode;\n-\t\thashcpy(pair->one[i].sha1, p->parent[i].sha1);\n-\t\tpair->one[i].sha1_valid = !is_null_sha1(p->parent[i].sha1);\n+\t\thashcpy(pair->one[i].sha1, p->parent[i].sha1.oid);\n+\t\tpair->one[i].sha1_valid = !is_null_sha1(p->parent[i].sha1.oid);\n \t\tpair->one[i].has_more_entries = 1;\n \t}\n \tpair->one[num_parent - 1].has_more_entries = 0;\n \n \tpair->two->path = p->path;\n \tpair->two->mode = p->mode;\n-\thashcpy(pair->two->sha1, p->sha1);\n-\tpair->two->sha1_valid = !is_null_sha1(p->sha1);\n+\thashcpy(pair->two->sha1, p->sha1.oid);\n+\tpair->two->sha1_valid = !is_null_sha1(p->sha1.oid);\n \treturn pair;\n }\n \ndiff --git a/diff-lib.c b/diff-lib.c\nindex 0448729..4b74a02 100644\n--- a/diff-lib.c\n+++ b/diff-lib.c\n@@ -124,7 +124,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\tdpath->next = NULL;\n \t\t\tmemcpy(dpath->path, ce->name, path_len);\n \t\t\tdpath->path[path_len] = '\\0';\n-\t\t\thashclr(dpath->sha1);\n+\t\t\thashclr(dpath->sha1.oid);\n \t\t\tmemset(&(dpath->parent[0]), 0,\n \t\t\t sizeof(struct combine_diff_parent)*5);\n \n@@ -154,7 +154,7 @@ int run_diff_files(struct rev_info *revs, unsigned int option)\n \t\t\t\tif (2 <= stage) {\n \t\t\t\t\tint mode = nce->ce_mode;\n \t\t\t\t\tnum_compare_stages++;\n-\t\t\t\t\thashcpy(dpath->parent[stage-2].sha1, nce->sha1);\n+\t\t\t\t\thashcpy(dpath->parent[stage-2].sha1.oid, nce->sha1);\n \t\t\t\t\tdpath->parent[stage-2].mode = ce_mode_from_stat(nce, mode);\n \t\t\t\t\tdpath->parent[stage-2].status =\n \t\t\t\t\t\tDIFF_STATUS_MODIFIED;\n@@ -326,14 +326,14 @@ static int show_modified(struct rev_info *revs,\n \t\tmemcpy(p->path, new->name, pathlen);\n \t\tp->path[pathlen] = 0;\n \t\tp->mode = mode;\n-\t\thashclr(p->sha1);\n+\t\thashclr(p->sha1.oid);\n \t\tmemset(p->parent, 0, 2 * sizeof(struct combine_diff_parent));\n \t\tp->parent[0].status = DIFF_STATUS_MODIFIED;\n \t\tp->parent[0].mode = new->ce_mode;\n-\t\thashcpy(p->parent[0].sha1, new->sha1);\n+\t\thashcpy(p->parent[0].sha1.oid, new->sha1);\n \t\tp->parent[1].status = DIFF_STATUS_MODIFIED;\n \t\tp->parent[1].mode = old->ce_mode;\n-\t\thashcpy(p->parent[1].sha1, old->sha1);\n+\t\thashcpy(p->parent[1].sha1.oid, old->sha1);\n \t\tshow_combined_diff(p, 2, revs->dense_combined_merges, revs);\n \t\tfree(p);\n \t\treturn 0;\ndiff --git a/diff.h b/diff.h\nindex a24a767..38bb1ed 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -6,6 +6,7 @@\n \n #include \"tree-walk.h\"\n #include \"pathspec.h\"\n+#include \"object.h\"\n \n struct rev_info;\n struct diff_options;\n@@ -200,11 +201,11 @@ struct combine_diff_path {\n \tstruct combine_diff_path *next;\n \tchar *path;\n \tunsigned int mode;\n-\tunsigned char sha1[20];\n+\tstruct object_id sha1;\n \tstruct combine_diff_parent {\n \t\tchar status;\n \t\tunsigned int mode;\n-\t\tunsigned char sha1[20];\n+\t\tstruct object_id sha1;\n \t} parent[FLEX_ARRAY];\n };\n #define combine_diff_path_size(n, l) \\\n-- \n2.0.0.rc0\n"},{"id":"240650","messageId":"20140503224957.GL75770@vauxhall.crustytoothpaste.net","threadId":"36577","inReplyTo":"1399147942-165308-1-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [RFC PATCH 0/9] Use a structure for object IDs.","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-03T22:49:58Z","receivedAt":"2014-05-03T22:49:58Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sat, May 03, 2014 at 08:12:13PM +0000, brian m. carlson wrote:\n> This is a preliminary RFC patch series to move all the relevant uses of\n> unsigned char [20] to struct object_id. It should not be applied to any\n> branch yet.\n> \n> The goal of this series to improve type-checking in the codebase and to\n> make it easier to move to a different hash function if the project\n> decides to do that. This series does not convert all of the codebase,\n> but only parts. I'm looking for feedback to see if there is consensus\n> that this is the right direction before investing a large amount of\n> time.\n\nI would like to point out that to get something with as few calls as\nhashclr converted to use struct object_id requires an insane amount of\nwork, because often major parts of several files have to be converted\nfirst. So the list should be aware that this will likely be an\nextensive series, although it is bisectable, so it could theoretically\nbe done in batches.\n\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"240661","messageId":"5365D91E.70207@alum.mit.edu","threadId":"36577","inReplyTo":"1399147942-165308-2-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-05-04T06:07:26Z","receivedAt":"2014-05-04T06:07:26Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 05/03/2014 10:12 PM, brian m. carlson wrote:\n> Many places throughout the code use \"unsigned char [20]\" to store object IDs\n> (SHA-1 values). This leads to lots of hardcoded numbers throughout the\n> codebase. It also leads to confusion about the purposes of a buffer.\n> \n> Introduce a structure for object IDs. This allows us to obtain the benefits\n> of compile-time checking for misuse. The structure is expected to remain\n> the same size and have the same alignment requirements on all known\n> platforms, compared to the array of unsigned char.\n\nPlease clarify whether you plan to rely on all platforms having \"the\nsame size and alignment constraints\" for correctness, or whether that\nobservation of the status quo is only meant to reassure us that this\nchange won't cause memory to be wasted on padding.\n\nIf the former then I would feel very uncomfortable about the change.\nOtherwise I think it will be a nice improvement in code clarity (and I\nadmire your ambition in taking on this project!)\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"240668","messageId":"5365DF94.9060707@alum.mit.edu","threadId":"36577","inReplyTo":"1399147942-165308-1-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [RFC PATCH 0/9] Use a structure for object IDs.","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-05-04T06:35:00Z","receivedAt":"2014-05-04T06:35:00Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 05/03/2014 10:12 PM, brian m. carlson wrote:\n> This is a preliminary RFC patch series to move all the relevant uses of\n> unsigned char [20] to struct object_id. It should not be applied to any\n> branch yet.\n> \n> The goal of this series to improve type-checking in the codebase and to\n> make it easier to move to a different hash function if the project\n> decides to do that. This series does not convert all of the codebase,\n> but only parts. I'm looking for feedback to see if there is consensus\n> that this is the right direction before investing a large amount of\n> time.\n> \n> Certain parts of the code have to be converted before others to keep the\n> patch sizes small, maintainable, and bisectable, so functions and\n> structures that are used across the codebase (e.g. hashcmp and struct\n> object) will be converted later. Conversion has been done in a roughly\n> alphabetical order by name of file.\n> \n> The constants for raw and hex sizes of SHA-1 values are maintained.\n> These constants are used where the quantity is the size of an SHA-1\n> value, and sizeof(struct object_id) is used wherever memory is to be\n> allocated. This is done to permit the struct to turn into a union later\n> if multiple hashes are supported. I left the names at GIT_OID_RAWSZ and\n> GIT_OID_HEXSZ because that's what libgit2 uses and what Junio seemed to\n> prefer, but they can be changed later if there's a desire to do that.\n> \n> I called the structure member \"oid\" because it was easily grepable and\n> distinct from the rest of the codebase. It, too, can be changed if we\n> decide on a better name. I specifically did not choose \"sha1\" since it\n> looks weird to have \"sha1->sha1\" and I didn't want to rename lots of\n> variables.\n\nThat means that we will have sha1->oid all over the place, right?\nThat's unfortunate, because it is exactly backwards from what we would\nwant in a hypothetical future where OIDs are not necessarily SHA-1s. In\nthat future we would certainly have to support SHA-1s in parallel with\nthe new hash. So (in that hypothetical future) we will probably want\nthese expressions to look like oid->sha1, to allow, say, a second struct\nor union field oid->sha256 [1].\n\nIf that future would come to pass, then we would also want to have\ndistinct constants like GIT_SHA1_RAWSZ and GIT_SHA256_RAWSZ rather than\nthe generically-named GIT_OID_RAWSZ.\n\nI think that this patch series will improve the code clarity and type\nsafety independent of thoughts about supporting different hash\nalgorithms, so I'm not objecting to your naming decision. But *if* such\nsupport is part of your long-term hope, then you might ease the future\ntransition by choosing different names now.\n\n(Maybe renaming local variables \"sha1 -> oid\" might be a handy way of\nmaking clear which code has been converted to the new style.)\n\nJust to be clear, the above are just some random thoughts for your\nconsideration, but feel free to disregard them.\n\nIn any case, it sure will be a lot of code churn. If you succeed in\nthis project, then \"git blame\" will probably consider you the author of\nabout 2/3 of git :-)\n\nMichael\n\n[1] I'm certainly not advocating that we want to support a different\nhash, let alone that that hash should be SHA-256; these examples are\njust for illustration.\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"240673","messageId":"5366060B.4000301@kdbg.org","threadId":"36577","inReplyTo":"5365DF94.9060707@alum.mit.edu","subject":"Re: [RFC PATCH 0/9] Use a structure for object IDs.","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2014-05-04T09:19:07Z","receivedAt":"2014-05-04T09:19:07Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 04.05.2014 08:35, schrieb Michael Haggerty:\n> On 05/03/2014 10:12 PM, brian m. carlson wrote:\n>> I specifically did not choose \"sha1\" since it\n>> looks weird to have \"sha1->sha1\" and I didn't want to rename lots of\n>> variables.\n>\n> That means that we will have sha1->oid all over the place, right?\n\nOnly during the transition period. When all functions that currently take \nunsigned char[20] are converted to struct object_id *, this additional \ndereferences go away again.\n\n-- Hannes\n"},{"id":"240674","messageId":"536606AB.1020803@kdbg.org","threadId":"36577","inReplyTo":"5365D91E.70207@alum.mit.edu","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2014-05-04T09:21:47Z","receivedAt":"2014-05-04T09:21:47Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 04.05.2014 08:07, schrieb Michael Haggerty:\n> On 05/03/2014 10:12 PM, brian m. carlson wrote:\n>> Introduce a structure for object IDs. This allows us to obtain the benefits\n>> of compile-time checking for misuse. The structure is expected to remain\n>> the same size and have the same alignment requirements on all known\n>> platforms, compared to the array of unsigned char.\n>\n> Please clarify whether you plan to rely on all platforms having \"the\n> same size and alignment constraints\" for correctness, or whether that\n> observation of the status quo is only meant to reassure us that this\n> change won't cause memory to be wasted on padding.\n\nI think that a compiler that has different size and alignment requirements \nfor the proposed struct object_id and an unsigned char[20] would, strictly \nspeaking, not be a \"C\" compiler.\n\n-- Hannes\n"},{"id":"240677","messageId":"878uqhvpzb.fsf@fencepost.gnu.org","threadId":"36577","inReplyTo":"536606AB.1020803@kdbg.org","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-05-04T09:43:20Z","receivedAt":"2014-05-04T09:43:20Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Johannes Sixt writes:\n\n> Am 04.05.2014 08:07, schrieb Michael Haggerty:\n>> On 05/03/2014 10:12 PM, brian m. carlson wrote:\n>>> Introduce a structure for object IDs. This allows us to obtain the benefits\n>>> of compile-time checking for misuse. The structure is expected to remain\n>>> the same size and have the same alignment requirements on all known\n>>> platforms, compared to the array of unsigned char.\n>>\n>> Please clarify whether you plan to rely on all platforms having \"the\n>> same size and alignment constraints\" for correctness, or whether that\n>> observation of the status quo is only meant to reassure us that this\n>> change won't cause memory to be wasted on padding.\n>\n> I think that a compiler that has different size and alignment\n> requirements for the proposed struct object_id and an unsigned\n> char[20] would, strictly speaking, not be a \"C\" compiler.\n\nHuh? How so? There is no warranty as far as I know that a structure\nwith only a single member has the same size and alignment requirements\nas the single member would have. There is also no guarantee as far as I\nknow that anything but element dereference is a valid means of\nconverting access to a struct to access to a sole element.\n\n-- \nDavid Kastrup\n"},{"id":"240681","messageId":"m2mwexke34.fsf@linux-m68k.org","threadId":"36577","inReplyTo":"536606AB.1020803@kdbg.org","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2014-05-04T10:55:43Z","receivedAt":"2014-05-04T10:55:43Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Johannes Sixt writes:\n\n> I think that a compiler that has different size and alignment requirements\n> for the proposed struct object_id and an unsigned char[20] would, strictly\n> speaking, not be a \"C\" compiler.\n\nUnlike arrays, a struct can have arbitrary internal padding. It is\nperfectly compliant (and even reasonable) to make struct object_id\nrequire 8 byte alignment, adding 4 bytes of padding at the end.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"240684","messageId":"CACsJy8Bb31FNN+aZpX7LgMQkPwC5_p_BMKthUKKPbRJomGwJSA@mail.gmail.com","threadId":"36577","inReplyTo":"5365D91E.70207@alum.mit.edu","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2014-05-04T12:29:21Z","receivedAt":"2014-05-04T12:29:21Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, May 4, 2014 at 1:07 PM, Michael Haggerty wrote:\n> On 05/03/2014 10:12 PM, brian m. carlson wrote:\n>> Many places throughout the code use \"unsigned char [20]\" to store object IDs\n>> (SHA-1 values). This leads to lots of hardcoded numbers throughout the\n>> codebase. It also leads to confusion about the purposes of a buffer.\n>>\n>> Introduce a structure for object IDs. This allows us to obtain the benefits\n>> of compile-time checking for misuse. The structure is expected to remain\n>> the same size and have the same alignment requirements on all known\n>> platforms, compared to the array of unsigned char.\n>\n> Please clarify whether you plan to rely on all platforms having \"the\n> same size and alignment constraints\" for correctness, or whether that\n> observation of the status quo is only meant to reassure us that this\n> change won't cause memory to be wasted on padding.\n\nIt's not just about wasted padding. Some structs, like\nondisk_cache_entry, reflect on-disk format. Padding means breakage.\nBut I don't think this will be a big issue because we can detect if\npadding happens, abort to force the user to complain, and deal with it\non case-by-case basis.\n-- \nDuy\n"},{"id":"240685","messageId":"20140504160728.GN75770@vauxhall.crustytoothpaste.net","threadId":"36577","inReplyTo":"5365D91E.70207@alum.mit.edu","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-04T16:07:28Z","receivedAt":"2014-05-04T16:07:28Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, May 04, 2014 at 08:07:26AM +0200, Michael Haggerty wrote:\n> Please clarify whether you plan to rely on all platforms having \"the\n> same size and alignment constraints\" for correctness, or whether that\n> observation of the status quo is only meant to reassure us that this\n> change won't cause memory to be wasted on padding.\n\nI plan to write the code portably. My statement was basically that I\ndon't expect this to result in more memory being used. I don't even\nplan to write the code assuming that offsetof(struct object_id, oid) is\n0.\n\nI have owned SPARC systems, and I have experienced plenty of aggravation\nwith code that makes unportable alignment assumptions. I don't want to\nmake that mistake myself.\n\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"240686","messageId":"87bnvd8p7h.fsf@igel.home","threadId":"36577","inReplyTo":"20140504160728.GN75770@vauxhall.crustytoothpaste.net","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2014-05-04T16:48:34Z","receivedAt":"2014-05-04T16:48:34Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"\"brian m. carlson\" writes:\n\n> I don't even plan to write the code assuming that offsetof(struct\n> object_id, oid) is 0.\n\nThis is guaranteed by the C standard, though.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"240687","messageId":"87wqe1tqu3.fsf@fencepost.gnu.org","threadId":"36577","inReplyTo":"87bnvd8p7h.fsf@igel.home","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-05-04T17:07:48Z","receivedAt":"2014-05-04T17:07:48Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Andreas Schwab writes:\n\n> \"brian m. carlson\" writes:\n>\n>> I don't even plan to write the code assuming that offsetof(struct\n>> object_id, oid) is 0.\n>\n> This is guaranteed by the C standard, though.\n\nAny reference?\n\n-- \nDavid Kastrup\n"},{"id":"240689","messageId":"877g618njf.fsf@igel.home","threadId":"36577","inReplyTo":"87wqe1tqu3.fsf@fencepost.gnu.org","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2014-05-04T17:24:36Z","receivedAt":"2014-05-04T17:24:36Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"David Kastrup writes:\n\n> Andreas Schwab writes:\n>\n>> \"brian m. carlson\" writes:\n>>\n>>> I don't even plan to write the code assuming that offsetof(struct\n>>> object_id, oid) is 0.\n>>\n>> This is guaranteed by the C standard, though.\n>\n> Any reference?\n\n§6.7.2.1#15\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"240690","messageId":"87ppjttp4b.fsf@fencepost.gnu.org","threadId":"36577","inReplyTo":"877g618njf.fsf@igel.home","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-05-04T17:44:52Z","receivedAt":"2014-05-04T17:44:52Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Andreas Schwab writes:\n\n> David Kastrup writes:\n>\n>> Andreas Schwab writes:\n>>\n>>> \"brian m. carlson\" writes:\n>>>\n>>>> I don't even plan to write the code assuming that offsetof(struct\n>>>> object_id, oid) is 0.\n>>>\n>>> This is guaranteed by the C standard, though.\n>>\n>> Any reference?\n>\n> §6.7.2.1#15\n\nMore like #13. I am pretty sure, however, that this has not always been\nthe case.\n\n-- \nDavid Kastrup\n"},{"id":"240692","messageId":"20140504175459.GO75770@vauxhall.crustytoothpaste.net","threadId":"36577","inReplyTo":"5365DF94.9060707@alum.mit.edu","subject":"Re: [RFC PATCH 0/9] Use a structure for object IDs.","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2014-05-04T17:54:59Z","receivedAt":"2014-05-04T17:54:59Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sun, May 04, 2014 at 08:35:00AM +0200, Michael Haggerty wrote:\n> On 05/03/2014 10:12 PM, brian m. carlson wrote:\n> > I called the structure member \"oid\" because it was easily grepable and\n> > distinct from the rest of the codebase. It, too, can be changed if we\n> > decide on a better name. I specifically did not choose \"sha1\" since it\n> > looks weird to have \"sha1->sha1\" and I didn't want to rename lots of\n> > variables.\n> \n> That means that we will have sha1->oid all over the place, right?\n> That's unfortunate, because it is exactly backwards from what we would\n> want in a hypothetical future where OIDs are not necessarily SHA-1s. In\n> that future we would certainly have to support SHA-1s in parallel with\n> the new hash. So (in that hypothetical future) we will probably want\n> these expressions to look like oid->sha1, to allow, say, a second struct\n> or union field oid->sha256 [1].\n\nAs Johannes pointed out, only during the transition period.\n\n> If that future would come to pass, then we would also want to have\n> distinct constants like GIT_SHA1_RAWSZ and GIT_SHA256_RAWSZ rather than\n> the generically-named GIT_OID_RAWSZ.\n\nYou have a point. I'll make the change.\n\n> I think that this patch series will improve the code clarity and type\n> safety independent of thoughts about supporting different hash\n> algorithms, so I'm not objecting to your naming decision. But *if* such\n> support is part of your long-term hope, then you might ease the future\n> transition by choosing different names now.\n\nIt is an eventual goal, but without this series, it's not even worth\ndiscussing since it's too hard to implement. Even if that doesn't\nhappen, my hope is that we'll at least improve the safety of the code\nand hopefully avoid a bug or two out of it.\n\n> (Maybe renaming local variables \"sha1 -> oid\" might be a handy way of\n> making clear which code has been converted to the new style.)\n\nThis is a good idea as well. I'll walk through the patches and fix\nthat.\n\n> Just to be clear, the above are just some random thoughts for your\n> consideration, but feel free to disregard them.\n\nI appreciate the well-thought-out response.\n\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\n+1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187\n"},{"id":"240693","messageId":"8738gp8lta.fsf@igel.home","threadId":"36577","inReplyTo":"87ppjttp4b.fsf@fencepost.gnu.org","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2014-05-04T18:01:53Z","receivedAt":"2014-05-04T18:01:53Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"David Kastrup writes:\n\n> Andreas Schwab writes:\n>\n>> David Kastrup writes:\n>>\n>>> Andreas Schwab writes:\n>>>\n>>>> \"brian m. carlson\" writes:\n>>>>\n>>>>> I don't even plan to write the code assuming that offsetof(struct\n>>>>> object_id, oid) is 0.\n>>>>\n>>>> This is guaranteed by the C standard, though.\n>>>\n>>> Any reference?\n>>\n>> §6.7.2.1#15\n>\n> More like #13.\n\nI don't know what you mean, but this is a C11 reference.\n\n> I am pretty sure, however, that this has not always been the case.\n\nYou are wrong.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"240698","messageId":"5366A09E.6030802@kdbg.org","threadId":"36577","inReplyTo":"m2mwexke34.fsf@linux-m68k.org","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2014-05-04T20:18:38Z","receivedAt":"2014-05-04T20:18:38Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 04.05.2014 12:55, schrieb Andreas Schwab:\n> Johannes Sixt writes:\n> \n>> I think that a compiler that has different size and alignment requirements\n>> for the proposed struct object_id and an unsigned char[20] would, strictly\n>> speaking, not be a \"C\" compiler.\n> \n> Unlike arrays, a struct can have arbitrary internal padding. It is\n> perfectly compliant (and even reasonable) to make struct object_id\n> require 8 byte alignment, adding 4 bytes of padding at the end.\n\nIsn't internal padding only allowed between members to achieve correct\nalignment of later members, and at the end only sufficient padding so\nthat members are aligned correctly when the struct is part of an array?\nThe former would not be the case because there is only one member, and\nthe latter is not the case because a char or array of char does not have\nalignment requirement?\n\n-- Hannes\n"},{"id":"240704","messageId":"87ppjt6xjv.fsf@igel.home","threadId":"36577","inReplyTo":"5366A09E.6030802@kdbg.org","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2014-05-04T21:31:16Z","receivedAt":"2014-05-04T21:31:16Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Johannes Sixt writes:\n\n> Isn't internal padding only allowed between members to achieve correct\n> alignment of later members, and at the end only sufficient padding so\n> that members are aligned correctly when the struct is part of an array?\n\nThe standard allows arbitrary internal padding, it doesn't have to be\nminimal.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"240728","messageId":"87lhugu7iw.fsf@fencepost.gnu.org","threadId":"36577","inReplyTo":"87ppjt6xjv.fsf@igel.home","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-05-05T05:19:35Z","receivedAt":"2014-05-05T05:19:35Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Andreas Schwab writes:\n\n> Johannes Sixt writes:\n>\n>> Isn't internal padding only allowed between members to achieve correct\n>> alignment of later members, and at the end only sufficient padding so\n>> that members are aligned correctly when the struct is part of an array?\n>\n> The standard allows arbitrary internal padding, it doesn't have to be\n> minimal.\n\nWhat the standard does guarantee is that a pointer to a struct can be\ncast to a pointer to its first member and vice versa. It does not as\nfar as I can see guarantee that a pointer to something of the same type\nof its first member can be converted to a pointer to a struct even if\nthe struct only contains a member of such type.\n\n-- \nDavid Kastrup\n"},{"id":"240712","messageId":"87vbtk60lh.fsf@igel.home","threadId":"36577","inReplyTo":"87lhugu7iw.fsf@fencepost.gnu.org","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2014-05-05T09:23:06Z","receivedAt":"2014-05-05T09:23:06Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"David Kastrup writes:\n\n> It does not as far as I can see guarantee that a pointer to something\n> of the same type of its first member can be converted to a pointer to\n> a struct even if the struct only contains a member of such type.\n\nThis sentence doesn't make any sense. If you have an object of struct\ntype then any pointer to the first member of the object can only be a\npointer to the one and same object.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"240756","messageId":"fd1b0343-d92f-4e51-a54e-a7629ea31028@email.android.com","threadId":"36577","inReplyTo":"87vbtk60lh.fsf@igel.home","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"James Denholm","fromEmail":"nod.helm@gmail.com","sentAt":"2014-05-05T09:33:44Z","receivedAt":"2014-05-05T09:33:44Z","isPatch":true,"sender":{"key":"nod.helm@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1721189?v=4"},"body":"On 5 May 2014 19:23:06 GMT+10:00, Andreas Schwab wrote:\n>David Kastrup writes:\n>\n>> It does not as far as I can see guarantee that a pointer to something\n>> of the same type of its first member can be converted to a pointer to\n>> a struct even if the struct only contains a member of such type.\n>\n>This sentence doesn't make any sense. If you have an object of struct\n>type then any pointer to the first member of the object can only be a\n>pointer to the one and same object.\n\nI think what David means is that a pointer to a wrapper\ncan be derefed into its internal, sure, but an object of\nthat internal type can't necessarily pretend to be a\nwrapper.\n\nThat said, obviously I'm not David, so I could be wrong.\nThat's what I got from his statement, though.\n\nRegards,\nJames Denholm.\n"},{"id":"240771","messageId":"87d2fstuzw.fsf@fencepost.gnu.org","threadId":"36577","inReplyTo":"87vbtk60lh.fsf@igel.home","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-05-05T09:50:11Z","receivedAt":"2014-05-05T09:50:11Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Andreas Schwab writes:\n\n> David Kastrup writes:\n>\n>> It does not as far as I can see guarantee that a pointer to something\n>> of the same type of its first member can be converted to a pointer to\n>> a struct even if the struct only contains a member of such type.\n>\n> This sentence doesn't make any sense.\n\nI disagree.\n\n> If you have an object of struct type\n\nYour premise is _not_ assumed in my statement. My premise was \"a\npointer to something of the same type of [the struct's] first member\".\nThat does quite explicitly _not_ state that an object of struct type is\nin existence.\n\n> then any pointer to the first member of the object can only be a\n> pointer to the one and same object.\n\nThe case we are talking about is basically passing a pointer to some\nactual bonafide toplevel unsigned char [20] object to a routine that\nexpects a pointer to a struct _only_ containing one such\nunsigned char [20] element.\n\nThis is the situation we have to deal with if a caller has not been\nconverted to using such a struct, but the called function does.\n\nMore seriously, this is the situation we have to deal with when our SHA1\nis actually embedded in some header or whatever else that is actually\navailable only inside of a larger byte buffer.\n\nIn that case, the standard does not permit us converting the address\nwhere that SHA1 is into a pointer to struct. It may well be that this\nwill fall under the \"let's ignore the standard and write for \"sensible\"\ncompilers/architectures\" dictum, but if it doesn't, it might be\nnecessary to first copy the data to a struct before passing it to\nroutines expecting a pointer to struct.\n\n-- \nDavid Kastrup\n"},{"id":"240720","messageId":"53676D6C.4010009@alum.mit.edu","threadId":"36577","inReplyTo":"87d2fstuzw.fsf@fencepost.gnu.org","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-05-05T10:52:28Z","receivedAt":"2014-05-05T10:52:28Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 05/05/2014 11:50 AM, David Kastrup wrote:\n> The case we are talking about is basically passing a pointer to some\n> actual bonafide toplevel unsigned char [20] object to a routine that\n> expects a pointer to a struct _only_ containing one such\n> unsigned char [20] element.\n> \n> This is the situation we have to deal with if a caller has not been\n> converted to using such a struct, but the called function does.\n\nIf the rewrite is done by first changing data structures and then\nchanging functions in caller -> callee order then (1) the deltas can be\npretty small, and (2) such illegal casting should be unnecessary.\n\n> More seriously, this is the situation we have to deal with when our SHA1\n> is actually embedded in some header or whatever else that is actually\n> available only inside of a larger byte buffer.\n> \n> In that case, the standard does not permit us converting the address\n> where that SHA1 is into a pointer to struct. It may well be that this\n> will fall under the \"let's ignore the standard and write for \"sensible\"\n> compilers/architectures\" dictum, but if it doesn't, it might be\n> necessary to first copy the data to a struct before passing it to\n> routines expecting a pointer to struct.\n\nThis sounds dangerous even for a \"sensible\" compiler. For example, I\ncan imagine that a sensible compiler might make the assumption that a\nsha1 field that it knows was obtained from oid->sha1 is word-aligned,\nand generate optimized code based on that assumption, even though it\notherwise wouldn't have had trouble working with unaligned (unsigned\nchar *) pointers.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"240751","messageId":"87r4485vve.fsf@igel.home","threadId":"36577","inReplyTo":"87d2fstuzw.fsf@fencepost.gnu.org","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2014-05-05T11:05:09Z","receivedAt":"2014-05-05T11:05:09Z","isPatch":true,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"David Kastrup writes:\n\n> Your premise is _not_ assumed in my statement. My premise was \"a\n> pointer to something of the same type of [the struct's] first member\".\n> That does quite explicitly _not_ state that an object of struct type is\n> in existence.\n\nSo you are not taking about struct object_id, and it's irrelevant to\nthis thread.\n\nThis thread is about objects of type struct object_id, and their address\nis always the same as the address of its first member. Nothing else is\nrelevant.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"240748","messageId":"87zjiwsc4a.fsf@fencepost.gnu.org","threadId":"36577","inReplyTo":"87r4485vve.fsf@igel.home","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-05-05T11:23:17Z","receivedAt":"2014-05-05T11:23:17Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Andreas Schwab writes:\n\n> David Kastrup writes:\n>\n>> Your premise is _not_ assumed in my statement. My premise was \"a\n>> pointer to something of the same type of [the struct's] first member\".\n>> That does quite explicitly _not_ state that an object of struct type is\n>> in existence.\n>\n> So you are not taking about struct object_id, and it's irrelevant to\n> this thread.\n>\n> This thread is about objects of type struct object_id, and their address\n> is always the same as the address of its first member. Nothing else is\n> relevant.\n\nHave it your way. I am too old for selective quotation games.\n\n-- \nDavid Kastrup\n"},{"id":"240743","messageId":"5367d595c04ed_25278db2ec8b@nysa.notmuch","threadId":"36577","inReplyTo":"87r4485vve.fsf@igel.home","subject":"Re: [PATCH 1/9] Define a structure for object IDs.","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2014-05-05T18:16:53Z","receivedAt":"2014-05-05T18:16:53Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Andreas Schwab wrote:\n> This thread is about objects of type struct object_id, and their\n> address is always the same as the address of its first member.\n> Nothing else is relevant.\n\nIndeed. I suggest you ingore that guy, he will only derail the\ndiscussion.\n\n-- \nFelipe Contreras\n"},{"id":"240800","messageId":"5368F4C8.2060604@alum.mit.edu","threadId":"36577","inReplyTo":"1399147942-165308-8-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH 7/9] bundle.c: convert leaf functions to struct object_id","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-05-06T14:42:16Z","receivedAt":"2014-05-06T14:42:16Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 05/03/2014 10:12 PM, brian m. carlson wrote:\n> Signed-off-by: brian m. carlson \n> ---\n> bundle.c | 38 +++++++++++++++++++-------------------\n> 1 file changed, 19 insertions(+), 19 deletions(-)\n> \n> diff --git a/bundle.c b/bundle.c\n> index 1222952..798ba28 100644\n> --- a/bundle.c\n> +++ b/bundle.c\n> @@ -11,11 +11,11 @@\n> \n> static const char bundle_signature[] = \"# v2 git bundle\\n\";\n> \n> -static void add_to_ref_list(const unsigned char *sha1, const char *name,\n> +static void add_to_ref_list(const struct object_id *sha1, const char *name,\n> \t\tstruct ref_list *list)\n> {\n> \tALLOC_GROW(list->list, list->nr + 1, list->alloc);\n> -\thashcpy(list->list[list->nr].sha1, sha1);\n> +\thashcpy(list->list[list->nr].sha1, sha1->oid);\n> \tlist->list[list->nr].name = xstrdup(name);\n> \tlist->nr++;\n> }\n> @@ -39,7 +39,7 @@ static int parse_bundle_header(int fd, struct bundle_header *header,\n> \t/* The bundle header ends with an empty line */\n> \twhile (!strbuf_getwholeline_fd(&buf, fd, '\\n') &&\n> \t buf.len && buf.buf[0] != '\\n') {\n> -\t\tunsigned char sha1[20];\n> +\t\tstruct object_id sha1;\n> \t\tint is_prereq = 0;\n> \n> \t\tif (*buf.buf == '-') {\n> @@ -53,9 +53,9 @@ static int parse_bundle_header(int fd, struct bundle_header *header,\n> \t\t * Prerequisites have object name that is optionally\n> \t\t * followed by SP and subject line.\n> \t\t */\n> -\t\tif (get_sha1_hex(buf.buf, sha1) ||\n> -\t\t (buf.len > 40 && !isspace(buf.buf[40])) ||\n> -\t\t (!is_prereq && buf.len <= 40)) {\n> +\t\tif (get_sha1_hex(buf.buf, sha1.oid) ||\n> +\t\t (buf.len > GIT_OID_HEXSZ && !isspace(buf.buf[GIT_OID_HEXSZ])) ||\n> +\t\t (!is_prereq && buf.len <= GIT_OID_HEXSZ)) {\n> \t\t\tif (report_path)\n> \t\t\t\terror(_(\"unrecognized header: %s%s (%d)\"),\n> \t\t\t\t (is_prereq ? \"-\" : \"\"), buf.buf, (int)buf.len);\n> @@ -63,9 +63,9 @@ static int parse_bundle_header(int fd, struct bundle_header *header,\n> \t\t\tbreak;\n> \t\t} else {\n> \t\t\tif (is_prereq)\n> -\t\t\t\tadd_to_ref_list(sha1, \"\", &header->prerequisites);\n> +\t\t\t\tadd_to_ref_list(&sha1, \"\", &header->prerequisites);\n> \t\t\telse\n> -\t\t\t\tadd_to_ref_list(sha1, buf.buf + 41, &header->references);\n> +\t\t\t\tadd_to_ref_list(&sha1, buf.buf + 41, &header->references);\n\nI think that 41 here is GIT_OID_HEXSZ + 1.\n\n> [...]\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"240798","messageId":"5368F77B.8090409@alum.mit.edu","threadId":"36577","inReplyTo":"1399147942-165308-9-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH 8/9] cache-tree: convert struct cache_tree to use object_id","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-05-06T14:53:47Z","receivedAt":"2014-05-06T14:53:47Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 05/03/2014 10:12 PM, brian m. carlson wrote:\n> Signed-off-by: brian m. carlson \n> ---\n> builtin/commit.c | 2 +-\n> builtin/fsck.c | 4 ++--\n> cache-tree.c | 30 +++++++++++++++---------------\n> cache-tree.h | 3 ++-\n> merge-recursive.c | 2 +-\n> reachable.c | 2 +-\n> sequencer.c | 2 +-\n> test-dump-cache-tree.c | 4 ++--\n> 8 files changed, 25 insertions(+), 24 deletions(-)\n> \n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 9cfef6c..639f843 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -1659,7 +1659,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n> \t\tappend_merge_tag_headers(parents, &tail);\n> \t}\n> \n> -\tif (commit_tree_extended(&sb, active_cache_tree->sha1, parents, sha1,\n> +\tif (commit_tree_extended(&sb, active_cache_tree->sha1.oid, parents, sha1,\n> \t\t\t\t author_ident.buf, sign_commit, extra)) {\n> \t\trollback_index_files();\n> \t\tdie(_(\"failed to write commit object\"));\n> diff --git a/builtin/fsck.c b/builtin/fsck.c\n> index fc150c8..6854c81 100644\n> --- a/builtin/fsck.c\n> +++ b/builtin/fsck.c\n> @@ -587,10 +587,10 @@ static int fsck_cache_tree(struct cache_tree *it)\n> \t\tfprintf(stderr, \"Checking cache tree\\n\");\n> \n> \tif (0 <= it->entry_count) {\n> -\t\tstruct object *obj = parse_object(it->sha1);\n> +\t\tstruct object *obj = parse_object(it->sha1.oid);\n> \t\tif (!obj) {\n> \t\t\terror(\"%s: invalid sha1 pointer in cache-tree\",\n> -\t\t\t sha1_to_hex(it->sha1));\n> +\t\t\t sha1_to_hex(it->sha1.oid));\n> \t\t\treturn 1;\n> \t\t}\n> \t\tobj->used = 1;\n> diff --git a/cache-tree.c b/cache-tree.c\n> index 7fa524a..b7b2d06 100644\n> --- a/cache-tree.c\n> +++ b/cache-tree.c\n> @@ -219,7 +219,7 @@ int cache_tree_fully_valid(struct cache_tree *it)\n> \tint i;\n> \tif (!it)\n> \t\treturn 0;\n> -\tif (it->entry_count < 0 || !has_sha1_file(it->sha1))\n> +\tif (it->entry_count < 0 || !has_sha1_file(it->sha1.oid))\n> \t\treturn 0;\n> \tfor (i = 0; i < it->subtree_nr; i++) {\n> \t\tif (!cache_tree_fully_valid(it->down[i]->cache_tree))\n> @@ -244,7 +244,7 @@ static int update_one(struct cache_tree *it,\n> \n> \t*skip_count = 0;\n> \n> -\tif (0 <= it->entry_count && has_sha1_file(it->sha1))\n> +\tif (0 <= it->entry_count && has_sha1_file(it->sha1.oid))\n> \t\treturn it->entry_count;\n> \n> \t/*\n> @@ -311,7 +311,7 @@ static int update_one(struct cache_tree *it,\n> \t\tstruct cache_tree_sub *sub;\n> \t\tconst char *path, *slash;\n> \t\tint pathlen, entlen;\n> -\t\tconst unsigned char *sha1;\n> +\t\tconst struct object_id *sha1;\n> \t\tunsigned mode;\n> \n> \t\tpath = ce->name;\n> @@ -327,21 +327,21 @@ static int update_one(struct cache_tree *it,\n> \t\t\t\tdie(\"cache-tree.c: '%.*s' in '%s' not found\",\n> \t\t\t\t entlen, path + baselen, path);\n> \t\t\ti += sub->count;\n> -\t\t\tsha1 = sub->cache_tree->sha1;\n> +\t\t\tsha1 = &sub->cache_tree->sha1;\n> \t\t\tmode = S_IFDIR;\n> \t\t\tif (sub->cache_tree->entry_count < 0)\n> \t\t\t\tto_invalidate = 1;\n> \t\t}\n> \t\telse {\n> -\t\t\tsha1 = ce->sha1;\n> +\t\t\tsha1 = (struct object_id *)ce->sha1;\n\nThis topic was discussed on the mailing list in the abstract. Here is a\nconcrete example.\n\nThis cast is undefined, because you can't make the assumption that\ncache_entry::sha1 has the same alignment and padding as (struct object_id).\n\nI think the transition will be more tractable if you rewrite the data\nstructures *first*; in this case changing cache_entry::sha1 to be\n(struct object_id) *before* rewriting code that works with it.\n\n> [...]\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"240776","messageId":"5368F990.2040702@alum.mit.edu","threadId":"36577","inReplyTo":"1399147942-165308-9-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH 8/9] cache-tree: convert struct cache_tree to use object_id","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-05-06T15:02:40Z","receivedAt":"2014-05-06T15:02:40Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 05/03/2014 10:12 PM, brian m. carlson wrote:\n> [...]\n> diff --git a/cache-tree.c b/cache-tree.c\n> index 7fa524a..b7b2d06 100644\n> --- a/cache-tree.c\n> +++ b/cache-tree.c\n\nIn this file I also found a couple other \"20\" that could be converted to\nGIT_OID_RAWSZ:\n\nAround line 369:\n\t\tstrbuf_add(&buffer, sha1, 20);\n\nAnd around line 490 (three instances):\n\t\tif (size < 20)\n\t\t\tgoto free_return;\n\t\thashcpy(it->sha1, (const unsigned char*)buf);\n\t\tbuf += 20;\n\t\tsize -= 20;\n\nI guess a search for \"\\<[24][0-9]\\>\" will find most (but not all!) of\nthe literal constants that are derived from GIT_OID_RAWSZ and GIT_OID_HEXSZ.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"240791","messageId":"5368FAF3.6000909@alum.mit.edu","threadId":"36577","inReplyTo":"1399147942-165308-10-git-send-email-sandals@crustytoothpaste.net","subject":"Re: [PATCH 9/9] diff: convert struct combine_diff_path to object_id","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2014-05-06T15:08:35Z","receivedAt":"2014-05-06T15:08:35Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 05/03/2014 10:12 PM, brian m. carlson wrote:\n> Signed-off-by: brian m. carlson \n> ---\n> combine-diff.c | 54 +++++++++++++++++++++++++++---------------------------\n> diff-lib.c | 10 +++++-----\n> diff.h | 5 +++--\n> 3 files changed, 35 insertions(+), 34 deletions(-)\n> \n> diff --git a/combine-diff.c b/combine-diff.c\n> index 24ca7e2..f97eb3a 100644\n> --- a/combine-diff.c\n> +++ b/combine-diff.c\n> [...]\n\nThis file also has two literal \"40\" constants in it that are probably\nGIT_OID_HEXSZ.\n\nFWIW, I glanced over all of the patches in this series (though without\nsystematically looking for other literal constants that should be\nderived from GIT_OID_RAWSZ and GIT_OID_HEXSZ) and, aside from the\nproblems that I already noted, they looked OK to me.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"}]}