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

Re: [PATCH] commit-reach: plug minor memory leak after using is_descendant_of()

From
Derrick Stolee <stolee@gmail.com>
Date
Jun 19, 2020, 14:07 UTC
Message-ID
<a7621cce-b2f9-d4ce-2fca-85e23cf185c0@gmail.com>
In-Reply-To
<20200619133159.GB2540897@coredump.intra.peff.net>
On 6/19/2020 9:31 AM, Jeff King wrote:
Show 7 quoted lines
> On Fri, Jun 19, 2020 at 03:13:46PM +0200, René Scharfe wrote:
> 
>> ref_newer() builds a commit_list to pass a single potential ancestor to
>> is_descendant_of().  The latter leaves the list intact.  Release the
>> allocated memory after the call.
> 
> Looks obviously correct.
I concur, thanks!
Show 17 quoted lines
>> ---
>> We could allocate the commit_list on the stack, which would simplify such
>> glue code quite a bit.  That would be dangerous in case is_descendant_of()
>> or some other function that is handed such a list tries to consume/free()
>> it.  How can we be tell a function is safe to be given a stack-allocated
>> list?  Perhaps by marking its argument as const.  Or by converting all
>> functions to arrays.
> 
> Yeah, if we're not worried about the performance implications of the
> extra allocation, I think it's better to err on the side of safety.
> 
> I do agree that if we consistently passed an array (and length), some of
> these functions would get less awkward. I tried a few years ago to
> convert many of the commit_list uses to arrays, but it was a bit of a
> yak shave, since often they get lists from callers, who get it from
> rev_info, etc. And some of those callers _do_ like having lists, because
> they want to do O(1) splicing, etc.

Yeah, this constant parameter-conversion between the different methods in commit-reach.c is certainly unfortunate, but persists due to historical reasons that Peff mentions.

Thanks, -Stolee

Previous: Jeff King
Message 3 of 3 in “commit-reach: plug minor memory leak after using is_descendant_of()”
  1. commit-reach: plug minor memory leak after using is_descendant_of()René Scharfe, Jun 19, 2020
  2. Jeff KingJun 19, 2020
  3. Derrick StoleeJun 19, 2020

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.