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>

diff --git a/notes.c b/notes.c
index d9f92d59f89392cd30cc2526c42e97eb675d9f09..5be570e398742b8c2a6ca53bfcc1c6cdc5a17e78 100644 (file)
--- a/notes.c
+++ b/notes.c
@@ -433,30 +433,40 @@ static void load_subtree(struct notes_tree *t, struct leaf_node *subtree,
        while (tree_entry(&desc, &entry)) {
                unsigned char type;
                struct leaf_node *l;
        while (tree_entry(&desc, &entry)) {
                unsigned char type;
                struct leaf_node *l;

-               int len, path_len = strlen(entry.path);
+               int path_len = strlen(entry.path);
+
+               if (path_len == 2 * (GIT_SHA1_RAWSZ - prefix_len)) {
+                       /* This is potentially the remainder of the SHA-1 */
+                       if (get_oid_hex_segment(entry.path, path_len,
+                                               object_oid.hash + prefix_len,
+                                               GIT_SHA1_RAWSZ - prefix_len) < 0)
+                               goto handle_non_note; /* entry.path is not a SHA1 */
+
+                       type = PTR_TYPE_NOTE;
+                       l = (struct leaf_node *)
+                               xcalloc(1, sizeof(struct leaf_node));
+                       oidcpy(&l->key_oid, &object_oid);
+                       oidcpy(&l->val_oid, entry.oid);
+               } else if (path_len == 2) {
+                       /* This is potentially an internal node */
+                       if (get_oid_hex_segment(entry.path, 2,
+                                               object_oid.hash + prefix_len,
+                                               GIT_SHA1_RAWSZ - prefix_len) < 0)
+                               goto handle_non_note; /* entry.path is not a SHA1 */

 
 

-               len = get_oid_hex_segment(entry.path, path_len,
-                               object_oid.hash + prefix_len, GIT_SHA1_RAWSZ - prefix_len);
-               if (len < 0)
-                       goto handle_non_note; /* entry.path is not a SHA1 */
-               len += prefix_len;
-
-               /*
-                * If object SHA1 is complete (len == 20), assume note object
-                * If object SHA1 is incomplete (len < 20), and current
-                * component consists of 2 hex chars, assume note subtree
-                */
-               type = PTR_TYPE_NOTE;
-               l = (struct leaf_node *)
-                       xcalloc(1, sizeof(struct leaf_node));
-               oidcpy(&l->key_oid, &object_oid);
-               oidcpy(&l->val_oid, entry.oid);
-               if (len < GIT_SHA1_RAWSZ) {
-                       if (!S_ISDIR(entry.mode) || path_len != 2)
-                               goto handle_non_note; /* not subtree */
-                       l->key_oid.hash[KEY_INDEX] = (unsigned char) len;

                        type = PTR_TYPE_SUBTREE;
                        type = PTR_TYPE_SUBTREE;

+                       l = (struct leaf_node *)
+                               xcalloc(1, sizeof(struct leaf_node));
+                       oidcpy(&l->key_oid, &object_oid);
+                       oidcpy(&l->val_oid, entry.oid);
+                       if (!S_ISDIR(entry.mode))
+                               goto handle_non_note; /* not subtree */
+                       l->key_oid.hash[KEY_INDEX] = (unsigned char) (prefix_len + 1);
+               } else {
+                       /* This can't be part of a note */
+                       goto handle_non_note;

                }
                }

+

                if (note_tree_insert(t, node, n, l, type,
                                     combine_notes_concatenate))
                        die("Failed to load %s %s into notes tree "
                if (note_tree_insert(t, node, n, l, type,
                                     combine_notes_concatenate))
                        die("Failed to load %s %s into notes tree "