load_subtree(): separate logic for internal vs. terminal entries

There are only two legitimate notes path components: * A hexadecimal string that fills the rest of the SHA-1 * A two-digit hexadecimal string that constitutes another internal node. So handle those two cases at the top level, and reject others as non-notes without trying to parse them. The logic separation also simplifies upcoming changes. This prevents us from leaking memory for a leaf_node in the case of wrong-sized paths. There are still memory leaks in this code; they will be fixed in upcoming commits. Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Michael Haggerty committed Aug 26, 2017 at 10:28 UTC 98c9897d9e40f49cfb416801851656bac494ae40
1 file changed +31 -21
notes.c
+31 -21
@@ -433,30 +433,40 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
433 while (tree_entry(&desc, &entry)) {
434 unsigned char type;
435 struct leaf_node *l;
436 - int len, path_len = strlen(entry.path);
436 + int path_len = strlen(entry.path);
437 +
438 + if (path_len == 2 * (GIT_SHA1_RAWSZ - prefix_len)) {
439 + /* This is potentially the remainder of the SHA-1 */
440 + if (get_oid_hex_segment(entry.path, path_len,
441 + object_oid.hash + prefix_len,
442 + GIT_SHA1_RAWSZ - prefix_len) < 0)
443 + goto handle_non_note; /* entry.path is not a SHA1 */
444 +
445 + type = PTR_TYPE_NOTE;
446 + l = (struct leaf_node *)
447 + xcalloc(1, sizeof(struct leaf_node));
448 + oidcpy(&l->key_oid, &object_oid);
449 + oidcpy(&l->val_oid, entry.oid);
450 + } else if (path_len == 2) {
451 + /* This is potentially an internal node */
452 + if (get_oid_hex_segment(entry.path, 2,
453 + object_oid.hash + prefix_len,
454 + GIT_SHA1_RAWSZ - prefix_len) < 0)
455 + goto handle_non_note; /* entry.path is not a SHA1 */
456
438 - len = get_oid_hex_segment(entry.path, path_len,
439 - object_oid.hash + prefix_len, GIT_SHA1_RAWSZ - prefix_len);
440 - if (len < 0)
441 - goto handle_non_note; /* entry.path is not a SHA1 */
442 - len += prefix_len;
443 -
444 - /*
445 - * If object SHA1 is complete (len == 20), assume note object
446 - * If object SHA1 is incomplete (len < 20), and current
447 - * component consists of 2 hex chars, assume note subtree
448 - */
449 - type = PTR_TYPE_NOTE;
450 - l = (struct leaf_node *)
451 - xcalloc(1, sizeof(struct leaf_node));
452 - oidcpy(&l->key_oid, &object_oid);
453 - oidcpy(&l->val_oid, entry.oid);
454 - if (len < GIT_SHA1_RAWSZ) {
455 - if (!S_ISDIR(entry.mode) || path_len != 2)
456 - goto handle_non_note; /* not subtree */
457 - l->key_oid.hash[KEY_INDEX] = (unsigned char) len;
457 type = PTR_TYPE_SUBTREE;
458 + l = (struct leaf_node *)
459 + xcalloc(1, sizeof(struct leaf_node));
460 + oidcpy(&l->key_oid, &object_oid);
461 + oidcpy(&l->val_oid, entry.oid);
462 + if (!S_ISDIR(entry.mode))
463 + goto handle_non_note; /* not subtree */
464 + l->key_oid.hash[KEY_INDEX] = (unsigned char) (prefix_len + 1);
465 + } else {
466 + /* This can't be part of a note */
467 + goto handle_non_note;
468 }
469 +
470 if (note_tree_insert(t, node, n, l, type,
471 combine_notes_concatenate))
472 die("Failed to load %s %s into notes tree "