builtin/diff-tree: convert to struct object_id

Convert most leaf functions to struct object_id. Change several hardcoded numbers to uses of parse_oid_hex. In doing so, verify that we when we want two trees, we have exactly two trees. Finally, in stdin_diff_commit, avoid accessing the byte after the NUL. This will be a NUL as well, since the first NUL was a newline we overwrote. However, with parse_oid_hex, we no longer need to increment the pointer directly, and can simply increment it as part of our check for the space character. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

brian m. carlson committed Feb 21, 2017 at 23:47 UTC 5f5e936d4a2d9a9093e8e6a2501fad74bdc6c31c
1 file changed +31 -32
builtin/diff-tree.c
+31 -32
@@ -7,46 +7,44 @@
7
8 static struct rev_info log_tree_opt;
9
10 -static int diff_tree_commit_sha1(const unsigned char *sha1)
10 +static int diff_tree_commit_sha1(const struct object_id *oid)
11 {
12 - struct commit *commit = lookup_commit_reference(sha1);
12 + struct commit *commit = lookup_commit_reference(oid->hash);
13 if (!commit)
14 return -1;
15 return log_tree_commit(&log_tree_opt, commit);
16 }
17
18 /* Diff one or more commits. */
19 -static int stdin_diff_commit(struct commit *commit, char *line, int len)
19 +static int stdin_diff_commit(struct commit *commit, const char *p)
20 {
21 - unsigned char sha1[20];
22 - if (isspace(line[40]) && !get_sha1_hex(line+41, sha1)) {
23 - /* Graft the fake parents locally to the commit */
24 - int pos = 41;
25 - struct commit_list **pptr;
26 -
27 - /* Free the real parent list */
28 - free_commit_list(commit->parents);
29 - commit->parents = NULL;
30 - pptr = &(commit->parents);
31 - while (line[pos] && !get_sha1_hex(line + pos, sha1)) {
32 - struct commit *parent = lookup_commit(sha1);
33 - if (parent) {
34 - pptr = &commit_list_insert(parent, pptr)->next;
35 - }
36 - pos += 41;
21 + struct object_id oid;
22 + struct commit_list **pptr = NULL;
23 +
24 + /* Graft the fake parents locally to the commit */
25 + while (isspace(*p++) && !parse_oid_hex(p, &oid, &p)) {
26 + struct commit *parent = lookup_commit(oid.hash);
27 + if (!pptr) {
28 + /* Free the real parent list */
29 + free_commit_list(commit->parents);
30 + commit->parents = NULL;
31 + pptr = &(commit->parents);
32 + }
33 + if (parent) {
34 + pptr = &commit_list_insert(parent, pptr)->next;
35 }
36 }
37 return log_tree_commit(&log_tree_opt, commit);
38 }
39
40 /* Diff two trees. */
43 -static int stdin_diff_trees(struct tree *tree1, char *line, int len)
41 +static int stdin_diff_trees(struct tree *tree1, const char *p)
42 {
45 - unsigned char sha1[20];
43 + struct object_id oid;
44 struct tree *tree2;
47 - if (len != 82 || !isspace(line[40]) || get_sha1_hex(line + 41, sha1))
45 + if (!isspace(*p++) || parse_oid_hex(p, &oid, &p) || *p)
46 return error("Need exactly two trees, separated by a space");
49 - tree2 = lookup_tree(sha1);
47 + tree2 = lookup_tree(oid.hash);
48 if (!tree2 || parse_tree(tree2))
49 return -1;
50 printf("%s %s\n", oid_to_hex(&tree1->object.oid),
@@ -60,23 +58,24 @@ static int stdin_diff_trees(struct tree *tree1, char *line, int len)
58 static int diff_tree_stdin(char *line)
59 {
60 int len = strlen(line);
63 - unsigned char sha1[20];
61 + struct object_id oid;
62 struct object *obj;
63 + const char *p;
64
65 if (!len || line[len-1] != '\n')
66 return -1;
67 line[len-1] = 0;
69 - if (get_sha1_hex(line, sha1))
68 + if (parse_oid_hex(line, &oid, &p))
69 return -1;
71 - obj = parse_object(sha1);
70 + obj = parse_object(oid.hash);
71 if (!obj)
72 return -1;
73 if (obj->type == OBJ_COMMIT)
75 - return stdin_diff_commit((struct commit *)obj, line, len);
74 + return stdin_diff_commit((struct commit *)obj, p);
75 if (obj->type == OBJ_TREE)
77 - return stdin_diff_trees((struct tree *)obj, line, len);
76 + return stdin_diff_trees((struct tree *)obj, p);
77 error("Object %s is a %s, not a commit or tree",
79 - sha1_to_hex(sha1), typename(obj->type));
78 + oid_to_hex(&oid), typename(obj->type));
79 return -1;
80 }
81
@@ -141,7 +140,7 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
140 break;
141 case 1:
142 tree1 = opt->pending.objects[0].item;
144 - diff_tree_commit_sha1(tree1->oid.hash);
143 + diff_tree_commit_sha1(&tree1->oid);
144 break;
145 case 2:
146 tree1 = opt->pending.objects[0].item;
@@ -166,9 +165,9 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
165 opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
166 DIFF_SETUP_USE_CACHE);
167 while (fgets(line, sizeof(line), stdin)) {
169 - unsigned char sha1[20];
168 + struct object_id oid;
169
171 - if (get_sha1_hex(line, sha1)) {
170 + if (get_oid_hex(line, &oid)) {
171 fputs(line, stdout);
172 fflush(stdout);
173 }