From 28042dbcd60f3190d25cfeae0bf81d58a16f4771 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Sun, 12 Dec 2010 22:19:00 -0800 Subject: [PATCH] get_sha1_oneline: fix lifespan rule of temp_commit_buffer variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- sha1_name.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/sha1_name.c b/sha1_name.c index 2c3a5fb363..2cc7a42390 100644 --- 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(®ex, p + 2, 0, NULL, 0)) { + + p = strstr(p, "\n\n"); + matches = p && !regexec(®ex, p + 2, 0, NULL, 0); + free(to_free); + + if (matches) { hashcpy(sha1, commit->object.sha1); - retval = 0; + found = 1; break; } } regfree(®ex); - 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 {