Re: [PATCH v2 1/1] commit-reach: properly peel tags
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 13, 2018, 21:06 UTC
- Message-ID
- <xmqqy3c5ds48.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <8d6061de-1654-577c-40c6-211dbd03aa36@gmail.com>
Derrick Stolee <stolee@gmail.com> writes:
Show 49 quoted lines
>> + if (!from_one || from_one->type != OBJ_COMMIT) {
>> + /* no way to tell if this is reachable by
>> + * looking at the ancestry chain alone, so
>> + * leave a note to ourselves not to worry about
>> + * this object anymore.
>> + */
>> + from->objects[i].item->flags |= assign_flag;
>> + continue;
>> + }
>> +
>> + list[nr_commits] = (struct commit *)from_one;
>> + if (parse_commit(list[nr_commits]) ||
>> + list[nr_commits]->generation < min_generation)
>> + return 0; /* is this a leak? */
>
> Of course, after sending v2, I see this comment. This is a leak of
> 'list' and should be fixed.
>
> Not only is it a leak here, it is also a leak in the 'cleanup'
> section. I'll squash the following into v3, but I'll let v2 simmer for
> review before rerolling.
>
> diff --git a/commit-reach.c b/commit-reach.c
> index 4048a2132a..c457d8d85f 100644
> --- a/commit-reach.c
> +++ b/commit-reach.c
> @@ -569,8 +569,11 @@ int can_all_from_reach_with_flag(struct
> object_array *from,
>
> list[nr_commits] = (struct commit *)from_one;
> if (parse_commit(list[nr_commits]) ||
> - list[nr_commits]->generation < min_generation)
> - return 0; /* is this a leak? */
> + list[nr_commits]->generation < min_generation) {
> + result = 0;
> + goto cleanup;
> + }
> +
> nr_commits++;
> }
>
> @@ -623,6 +626,7 @@ int can_all_from_reach_with_flag(struct
> object_array *from,
> clear_commit_marks(list[i], RESULT);
> clear_commit_marks(list[i], assign_flag);
> }
> + free(list);
> return result;
> }With this, commit marks are cleared even when we do the "early return", but only for the objects that appear in the resulting list[]. Because the for() loop in the last hunk interates over list[], those non-commit objects that are smudged with assign_flag but never made to list[] will be left smudged when this function returns (this is true when there is no early return).
Is that intended?