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>
Junio C Hamano committed
Dec 12, 2010 at 22:19 UTC
28042dbcd60f3190d25cfeae0bf81d58a16f4771
1 file changed
+13
-11
sha1_name.c
+13
-11
@@ -693,8 +693,7 @@ static int handle_one_ref(const char *path,
693
static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
694
{
695
struct commit_list *list = NULL, *backup = NULL, *l;
696
- int retval = -1;
697
- char *temp_commit_buffer = NULL;
696
+ int found = 0;
697
regex_t regex;
698
699
if (prefix[0] == '!') {
@@ -710,37 +709,40 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
709
for (l = list; l; l = l->next)
710
commit_list_insert(l->item, &backup);
711
while (list) {
713
- char *p;
712
+ char *p, *to_free = NULL;
713
struct commit *commit;
714
enum object_type type;
715
unsigned long size;
716
+ int matches;
717
718
commit = pop_most_recent_commit(&list, ONELINE_SEEN);
719
if (!parse_object(commit->object.sha1))
720
continue;
721
- free(temp_commit_buffer);
721
if (commit->buffer)
722
p = commit->buffer;
723
else {
724
p = read_sha1_file(commit->object.sha1, &type, &size);
725
if (!p)
726
continue;
728
- temp_commit_buffer = p;
727
+ to_free = p;
728
}
730
- if (!(p = strstr(p, "\n\n")))
731
- continue;
732
- if (!regexec(®ex, p + 2, 0, NULL, 0)) {
729
+
730
+ p = strstr(p, "\n\n");
731
+ matches = p && !regexec(®ex, p + 2, 0, NULL, 0);
732
+ free(to_free);
733
+
734
+ if (matches) {
735
hashcpy(sha1, commit->object.sha1);
734
- retval = 0;
736
+ found = 1;
737
break;
738
}
739
}
740
regfree(®ex);
739
- free(temp_commit_buffer);
741
free_commit_list(list);
742
for (l = backup; l; l = l->next)
743
clear_commit_marks(l->item, ONELINE_SEEN);
743
- return retval;
744
+ free_commit_list(backup);
745
+ return found ? 0 : -1;
746
}
747
748
struct grab_nth_branch_switch_cbdata {