git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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?
Previous: Derrick StoleeNext: Derrick Stolee via GitGitGadget
Message 9 of 25 in “Properly peel tags in can_all_from_reach_with_flags()”
  1. 0/1 Properly peel tags in can_all_from_reach_with_flags()Derrick Stolee via GitGitGadget, Sep 12, 2018
  2. 1/1 commit-reach: properly peel tagsDerrick Stolee via GitGitGadget, Sep 12, 2018
  3. Jeff KingSep 12, 2018
  4. Junio C HamanoSep 12, 2018
  5. Jeff KingSep 12, 2018
  6. 0/1 Properly peel tags in can_all_from_reach_with_flags()Derrick Stolee via GitGitGadget, Sep 13, 2018
  7. 1/1 commit-reach: properly peel tagsDerrick Stolee via GitGitGadget, Sep 13, 2018
  8. Derrick StoleeSep 13, 2018
  9. Junio C HamanoSep 13, 2018
  10. 0/2 Properly peel tags in can_all_from_reach_with_flags()Derrick Stolee via GitGitGadget, Sep 21, 2018
  11. 1/2 commit-reach: properly peel tagsDerrick Stolee via GitGitGadget, Sep 21, 2018
  12. Jeff KingSep 21, 2018
  13. Derrick StoleeSep 24, 2018
  14. 2/2 commit-reach: fix memory and flag leaksDerrick Stolee via GitGitGadget, Sep 21, 2018
  15. Jeff KingSep 21, 2018
  16. Derrick StoleeSep 24, 2018
  17. Jeff KingSep 24, 2018
  18. 0/1 Properly peel tags in can_all_from_reach_with_flags()Derrick Stolee via GitGitGadget, Sep 24, 2018
  19. 1/1 commit-reach: properly peel tags and clear flagsDerrick Stolee via GitGitGadget, Sep 24, 2018
  20. Jeff KingSep 24, 2018
  21. Eric SunshineSep 25, 2018
  22. commit-reach: cleanups in can_all_from_reach...Derrick Stolee, Sep 25, 2018
  23. Junio C HamanoSep 25, 2018
  24. Derrick StoleeSep 25, 2018
  25. Junio C HamanoSep 25, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.