get_sha1_oneline: fix lifespan rule of temp_commit_buffer variable

This is trying to free only what we ourselves read (as opposed to what
we borrowed from commit->buffer) but do so lazily only to work around
the fact that the code has many irregular exit points, and doing it right
makes it necessary to call free() from many different places in the loop.

Rewrite the structure of the code inside the loop so that the variable
has to live within a single iteration, ever.  This should make the logic
easier to follow as well.

Also we didn't free a temporary commit list we kept to hold the original
set of commits.  Free it.

Noticed-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sha1_name.c b/sha1_name.c
index 2c3a5fb363ff9b11a971b45e85819b2c0aaad157..2cc7a42390c246e8ea97c50b4b12d7109d35ba6c 100644 (file)
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -693,8 +693,7 @@ static int handle_one_ref(const char *path,
 static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 {
        struct commit_list *list = NULL, *backup = NULL, *l;
-       int retval = -1;
-       char *temp_commit_buffer = NULL;
+       int found = 0;
        regex_t regex;
 
        if (prefix[0] == '!') {
@@ -710,37 +709,40 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
        for (l = list; l; l = l->next)
                commit_list_insert(l->item, &backup);
        while (list) {
-               char *p;
+               char *p, *to_free = NULL;
                struct commit *commit;
                enum object_type type;
                unsigned long size;
+               int matches;
 
                commit = pop_most_recent_commit(&list, ONELINE_SEEN);
                if (!parse_object(commit->object.sha1))
                        continue;
-               free(temp_commit_buffer);
                if (commit->buffer)
                        p = commit->buffer;
                else {
                        p = read_sha1_file(commit->object.sha1, &type, &size);
                        if (!p)
                                continue;
-                       temp_commit_buffer = p;
+                       to_free = p;
                }
-               if (!(p = strstr(p, "\n\n")))
-                       continue;
-               if (!regexec(&regex, p + 2, 0, NULL, 0)) {
+
+               p = strstr(p, "\n\n");
+               matches = p && !regexec(&regex, p + 2, 0, NULL, 0);
+               free(to_free);
+
+               if (matches) {
                        hashcpy(sha1, commit->object.sha1);
-                       retval = 0;
+                       found = 1;
                        break;
                }
        }
        regfree(&regex);
-       free(temp_commit_buffer);
        free_commit_list(list);
        for (l = backup; l; l = l->next)
                clear_commit_marks(l->item, ONELINE_SEEN);
-       return retval;
+       free_commit_list(backup);
+       return found ? 0 : -1;
 }
 
 struct grab_nth_branch_switch_cbdata {