{"thread":{"id":"46572","subject":"[PATCH] tree-walk: convert fill_tree_descriptor() to object_id","startedAt":"2017-08-12T07:15:12Z","lastAt":"2017-08-12T14:59:37Z","messageCount":4,"participants":["René Scharfe","Johannes Sixt","brian m. carlson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"326192","messageId":"d1e4b270-4f8b-85e4-b6fd-09657720bed2@web.de","threadId":"46572","inReplyTo":null,"subject":"[PATCH] tree-walk: convert fill_tree_descriptor() to object_id","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-08-12T07:14:41Z","receivedAt":"2017-08-12T07:15:12Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"All callers of fill_tree_descriptor() have been converted to object_id\nalready, so convert that function as well.  As a nice side-effect we get\nrid of NULL checks in tree-diff.c, as fill_tree_descriptor() already\ndoes them for us.\n\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\n Documentation/technical/api-tree-walking.txt |  6 +++---\n builtin/merge-tree.c                         | 10 +++++-----\n builtin/reset.c                              |  4 ++--\n notes.c                                      |  2 +-\n tree-diff.c                                  |  5 ++---\n tree-walk.c                                  |  9 +++++----\n tree-walk.h                                  |  2 +-\n unpack-trees.c                               |  6 +++---\n 8 files changed, 22 insertions(+), 22 deletions(-)\n\ndiff --git a/Documentation/technical/api-tree-walking.txt b/Documentation/technical/api-tree-walking.txt\nindex 14af37c3f1..a5334a472e 100644\n--- a/Documentation/technical/api-tree-walking.txt\n+++ b/Documentation/technical/api-tree-walking.txt\n@@ -55,9 +55,9 @@ Initializing\n \n `fill_tree_descriptor`::\n \n-\tInitialize a `tree_desc` and decode its first entry given the sha1 of\n-\ta tree. Returns the `buffer` member if the sha1 is a valid tree\n-\tidentifier and NULL otherwise.\n+\tInitialize a `tree_desc` and decode its first entry given the\n+\tobject ID of a tree. Returns the `buffer` member if the sha1 is\n+\ta valid tree identifier and NULL otherwise.\n \n `setup_traverse_info`::\n \ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex f12da292cf..6ea246a200 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -213,10 +213,10 @@ static void unresolved_directory(const struct traverse_info *info,\n \n \tnewbase = traverse_path(info, p);\n \n-#define ENTRY_SHA1(e) (((e)->mode && S_ISDIR((e)->mode)) ? (e)->oid->hash : NULL)\n-\tbuf0 = fill_tree_descriptor(t+0, ENTRY_SHA1(n + 0));\n-\tbuf1 = fill_tree_descriptor(t+1, ENTRY_SHA1(n + 1));\n-\tbuf2 = fill_tree_descriptor(t+2, ENTRY_SHA1(n + 2));\n+#define ENTRY_OID(e) (((e)->mode && S_ISDIR((e)->mode)) ? (e)->oid : NULL)\n+\tbuf0 = fill_tree_descriptor(t + 0, ENTRY_OID(n + 0));\n+\tbuf1 = fill_tree_descriptor(t + 1, ENTRY_OID(n + 1));\n+\tbuf2 = fill_tree_descriptor(t + 2, ENTRY_OID(n + 2));\n #undef ENTRY_SHA1\n \n \tmerge_trees(t, newbase);\n@@ -352,7 +352,7 @@ static void *get_tree_descriptor(struct tree_desc *desc, const char *rev)\n \n \tif (get_oid(rev, &oid))\n \t\tdie(\"unknown rev %s\", rev);\n-\tbuf = fill_tree_descriptor(desc, oid.hash);\n+\tbuf = fill_tree_descriptor(desc, &oid);\n \tif (!buf)\n \t\tdie(\"%s is not a tree\", rev);\n \treturn buf;\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 046403ed68..4a02d74073 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -75,13 +75,13 @@ static int reset_index(const struct object_id *oid, int reset_type, int quiet)\n \t\tstruct object_id head_oid;\n \t\tif (get_oid(\"HEAD\", &head_oid))\n \t\t\treturn error(_(\"You do not have a valid HEAD.\"));\n-\t\tif (!fill_tree_descriptor(desc, head_oid.hash))\n+\t\tif (!fill_tree_descriptor(desc, &head_oid))\n \t\t\treturn error(_(\"Failed to find tree of HEAD.\"));\n \t\tnr++;\n \t\topts.fn = twoway_merge;\n \t}\n \n-\tif (!fill_tree_descriptor(desc + nr - 1, oid->hash))\n+\tif (!fill_tree_descriptor(desc + nr - 1, oid))\n \t\treturn error(_(\"Failed to find tree of %s.\"), oid_to_hex(oid));\n \tif (unpack_trees(nr, desc, &opts))\n \t\treturn -1;\ndiff --git a/notes.c b/notes.c\nindex 503754d79e..f090c88363 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -425,7 +425,7 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,\n \tunsigned char type;\n \tstruct leaf_node *l;\n \n-\tbuf = fill_tree_descriptor(&desc, subtree->val_oid.hash);\n+\tbuf = fill_tree_descriptor(&desc, &subtree->val_oid);\n \tif (!buf)\n \t\tdie(\"Could not read %s for notes-index\",\n \t\t     oid_to_hex(&subtree->val_oid));\ndiff --git a/tree-diff.c b/tree-diff.c\nindex 2357f72899..4bb93155bc 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -421,9 +421,8 @@ static struct combine_diff_path *ll_diff_tree_paths(\n \t *   diff_tree_oid(parent, commit) )\n \t */\n \tfor (i = 0; i < nparent; ++i)\n-\t\ttptree[i] = fill_tree_descriptor(&tp[i],\n-\t\t\t\tparents_oid[i] ? parents_oid[i]->hash : NULL);\n-\tttree = fill_tree_descriptor(&t, oid ? oid->hash : NULL);\n+\t\ttptree[i] = fill_tree_descriptor(&tp[i], parents_oid[i]);\n+\tttree = fill_tree_descriptor(&t, oid);\n \n \t/* Enable recursion indefinitely */\n \topt->pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);\ndiff --git a/tree-walk.c b/tree-walk.c\nindex 6a42e402b0..c99309069a 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -78,15 +78,16 @@ int init_tree_desc_gently(struct tree_desc *desc, const void *buffer, unsigned l\n \treturn result;\n }\n \n-void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1)\n+void *fill_tree_descriptor(struct tree_desc *desc, const struct object_id *oid)\n {\n \tunsigned long size = 0;\n \tvoid *buf = NULL;\n \n-\tif (sha1) {\n-\t\tbuf = read_object_with_reference(sha1, tree_type, &size, NULL);\n+\tif (oid) {\n+\t\tbuf = read_object_with_reference(oid->hash, tree_type, &size,\n+\t\t\t\t\t\t NULL);\n \t\tif (!buf)\n-\t\t\tdie(\"unable to read tree %s\", sha1_to_hex(sha1));\n+\t\t\tdie(\"unable to read tree %s\", oid_to_hex(oid));\n \t}\n \tinit_tree_desc(desc, buf, size);\n \treturn buf;\ndiff --git a/tree-walk.h b/tree-walk.h\nindex 68bb78b928..b6bd1b4ccf 100644\n--- a/tree-walk.h\n+++ b/tree-walk.h\n@@ -42,7 +42,7 @@ int init_tree_desc_gently(struct tree_desc *desc, const void *buf, unsigned long\n int tree_entry(struct tree_desc *, struct name_entry *);\n int tree_entry_gently(struct tree_desc *, struct name_entry *);\n \n-void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1);\n+void *fill_tree_descriptor(struct tree_desc *desc, const struct object_id *oid);\n \n struct traverse_info;\n typedef int (*traverse_callback_t)(int n, unsigned long mask, unsigned long dirmask, struct name_entry *entry, struct traverse_info *);\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 862cfce661..233ec5eb3d 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -662,10 +662,10 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,\n \t\telse if (i > 1 && are_same_oid(&names[i], &names[i - 2]))\n \t\t\tt[i] = t[i - 2];\n \t\telse {\n-\t\t\tconst unsigned char *sha1 = NULL;\n+\t\t\tconst struct object_id *oid = NULL;\n \t\t\tif (dirmask & 1)\n-\t\t\t\tsha1 = names[i].oid->hash;\n-\t\t\tbuf[nr_buf++] = fill_tree_descriptor(t+i, sha1);\n+\t\t\t\toid = names[i].oid;\n+\t\t\tbuf[nr_buf++] = fill_tree_descriptor(t + i, oid);\n \t\t}\n \t}\n \n-- \n2.14.1\n\n"},{"id":"326193","messageId":"fb99b685-327a-1c01-52d6-f0a1d106544c@kdbg.org","threadId":"46572","inReplyTo":"d1e4b270-4f8b-85e4-b6fd-09657720bed2@web.de","subject":"Re: [PATCH] tree-walk: convert fill_tree_descriptor() to object_id","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2017-08-12T08:04:24Z","receivedAt":"2017-08-12T08:04:34Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 12.08.2017 um 09:14 schrieb René Scharfe:\n> -\tInitialize a `tree_desc` and decode its first entry given the sha1 of\n> -\ta tree. Returns the `buffer` member if the sha1 is a valid tree\n> -\tidentifier and NULL otherwise.\n> +\tInitialize a `tree_desc` and decode its first entry given the\n> +\tobject ID of a tree. Returns the `buffer` member if the sha1 is\n> +\ta valid tree identifier and NULL otherwise.\n\nThere is another mention of \"sha1\" in the rewritten text. Intended or an \noversight?\n\n> -#define ENTRY_SHA1(e) (((e)->mode && S_ISDIR((e)->mode)) ? (e)->oid->hash : NULL)\n> -\tbuf0 = fill_tree_descriptor(t+0, ENTRY_SHA1(n + 0));\n> -\tbuf1 = fill_tree_descriptor(t+1, ENTRY_SHA1(n + 1));\n> -\tbuf2 = fill_tree_descriptor(t+2, ENTRY_SHA1(n + 2));\n> +#define ENTRY_OID(e) (((e)->mode && S_ISDIR((e)->mode)) ? (e)->oid : NULL)\n> +\tbuf0 = fill_tree_descriptor(t + 0, ENTRY_OID(n + 0));\n> +\tbuf1 = fill_tree_descriptor(t + 1, ENTRY_OID(n + 1));\n> +\tbuf2 = fill_tree_descriptor(t + 2, ENTRY_OID(n + 2));\n>   #undef ENTRY_SHA1\n\nThis whould be #undef ENTRY_OID.\n\n-- Hannes\n"},{"id":"326195","messageId":"9534aa87-def3-5105-652d-f136c72902c2@web.de","threadId":"46572","inReplyTo":"d1e4b270-4f8b-85e4-b6fd-09657720bed2@web.de","subject":"[PATCH v2] tree-walk: convert fill_tree_descriptor() to object_id","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2017-08-12T08:32:59Z","receivedAt":"2017-08-12T08:33:27Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"All callers of fill_tree_descriptor() have been converted to object_id\nalready, so convert that function as well.  As a nice side-effect we get\nrid of NULL checks in tree-diff.c, as fill_tree_descriptor() already\ndoes them for us.\n\nHelped-by: Johannes Sixt <j6t@kdbg.org>\nSigned-off-by: Rene Scharfe <l.s.r@web.de>\n---\nChanges from v1: Don't mention sha1 in documentation anymore, #undef\nENTRY_OID.  Thanks for spotting, Johannes!\n\n Documentation/technical/api-tree-walking.txt |  6 +++---\n builtin/merge-tree.c                         | 12 ++++++------\n builtin/reset.c                              |  4 ++--\n notes.c                                      |  2 +-\n tree-diff.c                                  |  5 ++---\n tree-walk.c                                  |  9 +++++----\n tree-walk.h                                  |  2 +-\n unpack-trees.c                               |  6 +++---\n 8 files changed, 23 insertions(+), 23 deletions(-)\n\ndiff --git a/Documentation/technical/api-tree-walking.txt b/Documentation/technical/api-tree-walking.txt\nindex 14af37c3f1..bde18622a8 100644\n--- a/Documentation/technical/api-tree-walking.txt\n+++ b/Documentation/technical/api-tree-walking.txt\n@@ -55,9 +55,9 @@ Initializing\n \n `fill_tree_descriptor`::\n \n-\tInitialize a `tree_desc` and decode its first entry given the sha1 of\n-\ta tree. Returns the `buffer` member if the sha1 is a valid tree\n-\tidentifier and NULL otherwise.\n+\tInitialize a `tree_desc` and decode its first entry given the\n+\tobject ID of a tree. Returns the `buffer` member if the latter\n+\tis a valid tree identifier and NULL otherwise.\n \n `setup_traverse_info`::\n \ndiff --git a/builtin/merge-tree.c b/builtin/merge-tree.c\nindex f12da292cf..d01ddecf66 100644\n--- a/builtin/merge-tree.c\n+++ b/builtin/merge-tree.c\n@@ -213,11 +213,11 @@ static void unresolved_directory(const struct traverse_info *info,\n \n \tnewbase = traverse_path(info, p);\n \n-#define ENTRY_SHA1(e) (((e)->mode && S_ISDIR((e)->mode)) ? (e)->oid->hash : NULL)\n-\tbuf0 = fill_tree_descriptor(t+0, ENTRY_SHA1(n + 0));\n-\tbuf1 = fill_tree_descriptor(t+1, ENTRY_SHA1(n + 1));\n-\tbuf2 = fill_tree_descriptor(t+2, ENTRY_SHA1(n + 2));\n-#undef ENTRY_SHA1\n+#define ENTRY_OID(e) (((e)->mode && S_ISDIR((e)->mode)) ? (e)->oid : NULL)\n+\tbuf0 = fill_tree_descriptor(t + 0, ENTRY_OID(n + 0));\n+\tbuf1 = fill_tree_descriptor(t + 1, ENTRY_OID(n + 1));\n+\tbuf2 = fill_tree_descriptor(t + 2, ENTRY_OID(n + 2));\n+#undef ENTRY_OID\n \n \tmerge_trees(t, newbase);\n \n@@ -352,7 +352,7 @@ static void *get_tree_descriptor(struct tree_desc *desc, const char *rev)\n \n \tif (get_oid(rev, &oid))\n \t\tdie(\"unknown rev %s\", rev);\n-\tbuf = fill_tree_descriptor(desc, oid.hash);\n+\tbuf = fill_tree_descriptor(desc, &oid);\n \tif (!buf)\n \t\tdie(\"%s is not a tree\", rev);\n \treturn buf;\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex 046403ed68..4a02d74073 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -75,13 +75,13 @@ static int reset_index(const struct object_id *oid, int reset_type, int quiet)\n \t\tstruct object_id head_oid;\n \t\tif (get_oid(\"HEAD\", &head_oid))\n \t\t\treturn error(_(\"You do not have a valid HEAD.\"));\n-\t\tif (!fill_tree_descriptor(desc, head_oid.hash))\n+\t\tif (!fill_tree_descriptor(desc, &head_oid))\n \t\t\treturn error(_(\"Failed to find tree of HEAD.\"));\n \t\tnr++;\n \t\topts.fn = twoway_merge;\n \t}\n \n-\tif (!fill_tree_descriptor(desc + nr - 1, oid->hash))\n+\tif (!fill_tree_descriptor(desc + nr - 1, oid))\n \t\treturn error(_(\"Failed to find tree of %s.\"), oid_to_hex(oid));\n \tif (unpack_trees(nr, desc, &opts))\n \t\treturn -1;\ndiff --git a/notes.c b/notes.c\nindex 503754d79e..f090c88363 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -425,7 +425,7 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,\n \tunsigned char type;\n \tstruct leaf_node *l;\n \n-\tbuf = fill_tree_descriptor(&desc, subtree->val_oid.hash);\n+\tbuf = fill_tree_descriptor(&desc, &subtree->val_oid);\n \tif (!buf)\n \t\tdie(\"Could not read %s for notes-index\",\n \t\t     oid_to_hex(&subtree->val_oid));\ndiff --git a/tree-diff.c b/tree-diff.c\nindex 2357f72899..4bb93155bc 100644\n--- a/tree-diff.c\n+++ b/tree-diff.c\n@@ -421,9 +421,8 @@ static struct combine_diff_path *ll_diff_tree_paths(\n \t *   diff_tree_oid(parent, commit) )\n \t */\n \tfor (i = 0; i < nparent; ++i)\n-\t\ttptree[i] = fill_tree_descriptor(&tp[i],\n-\t\t\t\tparents_oid[i] ? parents_oid[i]->hash : NULL);\n-\tttree = fill_tree_descriptor(&t, oid ? oid->hash : NULL);\n+\t\ttptree[i] = fill_tree_descriptor(&tp[i], parents_oid[i]);\n+\tttree = fill_tree_descriptor(&t, oid);\n \n \t/* Enable recursion indefinitely */\n \topt->pathspec.recursive = DIFF_OPT_TST(opt, RECURSIVE);\ndiff --git a/tree-walk.c b/tree-walk.c\nindex 6a42e402b0..c99309069a 100644\n--- a/tree-walk.c\n+++ b/tree-walk.c\n@@ -78,15 +78,16 @@ int init_tree_desc_gently(struct tree_desc *desc, const void *buffer, unsigned l\n \treturn result;\n }\n \n-void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1)\n+void *fill_tree_descriptor(struct tree_desc *desc, const struct object_id *oid)\n {\n \tunsigned long size = 0;\n \tvoid *buf = NULL;\n \n-\tif (sha1) {\n-\t\tbuf = read_object_with_reference(sha1, tree_type, &size, NULL);\n+\tif (oid) {\n+\t\tbuf = read_object_with_reference(oid->hash, tree_type, &size,\n+\t\t\t\t\t\t NULL);\n \t\tif (!buf)\n-\t\t\tdie(\"unable to read tree %s\", sha1_to_hex(sha1));\n+\t\t\tdie(\"unable to read tree %s\", oid_to_hex(oid));\n \t}\n \tinit_tree_desc(desc, buf, size);\n \treturn buf;\ndiff --git a/tree-walk.h b/tree-walk.h\nindex 68bb78b928..b6bd1b4ccf 100644\n--- a/tree-walk.h\n+++ b/tree-walk.h\n@@ -42,7 +42,7 @@ int init_tree_desc_gently(struct tree_desc *desc, const void *buf, unsigned long\n int tree_entry(struct tree_desc *, struct name_entry *);\n int tree_entry_gently(struct tree_desc *, struct name_entry *);\n \n-void *fill_tree_descriptor(struct tree_desc *desc, const unsigned char *sha1);\n+void *fill_tree_descriptor(struct tree_desc *desc, const struct object_id *oid);\n \n struct traverse_info;\n typedef int (*traverse_callback_t)(int n, unsigned long mask, unsigned long dirmask, struct name_entry *entry, struct traverse_info *);\ndiff --git a/unpack-trees.c b/unpack-trees.c\nindex 862cfce661..233ec5eb3d 100644\n--- a/unpack-trees.c\n+++ b/unpack-trees.c\n@@ -662,10 +662,10 @@ static int traverse_trees_recursive(int n, unsigned long dirmask,\n \t\telse if (i > 1 && are_same_oid(&names[i], &names[i - 2]))\n \t\t\tt[i] = t[i - 2];\n \t\telse {\n-\t\t\tconst unsigned char *sha1 = NULL;\n+\t\t\tconst struct object_id *oid = NULL;\n \t\t\tif (dirmask & 1)\n-\t\t\t\tsha1 = names[i].oid->hash;\n-\t\t\tbuf[nr_buf++] = fill_tree_descriptor(t+i, sha1);\n+\t\t\t\toid = names[i].oid;\n+\t\t\tbuf[nr_buf++] = fill_tree_descriptor(t + i, oid);\n \t\t}\n \t}\n \n-- \n2.14.1\n"},{"id":"326222","messageId":"20170812145928.jadxm3digocyejzg@genre.crustytoothpaste.net","threadId":"46572","inReplyTo":"9534aa87-def3-5105-652d-f136c72902c2@web.de","subject":"Re: [PATCH v2] tree-walk: convert fill_tree_descriptor() to object_id","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2017-08-12T14:59:28Z","receivedAt":"2017-08-12T14:59:37Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On Sat, Aug 12, 2017 at 10:32:59AM +0200, René Scharfe wrote:\n> All callers of fill_tree_descriptor() have been converted to object_id\n> already, so convert that function as well.  As a nice side-effect we get\n> rid of NULL checks in tree-diff.c, as fill_tree_descriptor() already\n> does them for us.\n> \n> Helped-by: Johannes Sixt <j6t@kdbg.org>\n> Signed-off-by: Rene Scharfe <l.s.r@web.de>\n\nThis looks good to me.  I had written a very similar patch for a later\nseries, and they're pretty much identical.\n-- \nbrian m. carlson / brian with sandals: Houston, Texas, US\nhttps://www.crustytoothpaste.net/~bmc | My opinion only\nOpenPGP: https://keybase.io/bk2204\n"}]}