Derrick Stolee <stolee@xxxxxxxxx> writes:
>> + 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?
