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

Re: [PATCH 3/3] diff.c: use diff_free_queue()

From
SZEDER Gábor <szeder.dev@gmail.com>
Date
Nov 7, 2022, 16:13 UTC
Message-ID
<20221107161311.GC1951@szeder.dev>
In-Reply-To
<Y2MKKTz4nK0L8uW5@nand.local>
On Wed, Nov 02, 2022 at 08:24:09PM -0400, Taylor Blau wrote:
Show 23 quoted lines
> On Wed, Nov 02, 2022 at 11:01:42PM +0100, SZEDER Gábor wrote:
> >  int diff_flush_patch_id(struct diff_options *options, struct object_id *oid, int diff_header_only)
> >  {
> >  	struct diff_queue_struct *q = &diff_queued_diff;
> > -	int i;
> >  	int result = diff_get_patch_id(options, oid, diff_header_only);
> >
> > -	for (i = 0; i < q->nr; i++)
> > -		diff_free_filepair(q->queue[i]);
> > -
> > -	free(q->queue);
> > +	diff_free_queue(q);
> >  	DIFF_QUEUE_CLEAR(q);
> 
> So, this all looks fine to me. But I did a quick grep around for
> DIFF_QUEUE_CLEAR(), and this macro is used in quite a few places.
> Mostly, as far as I can tell, to "empty" out the diff-queue by setting
> its 'queue' pointer to NULL, and its 'nr' back to 0.
> 
> Should we be freeing the memory held by the queue there more
> aggressively? I.e., should we make sure that there is a
> diff_free_queue() call above each expansion of the DIFF_QUEUE_CLEAR()
> macro?

Definitely not. DIFF_QUEUE_CLEAR is often used to initialize a just created 'struct diff_queue_struct' instance; by adding a diff_free_queue() above those it would operate on uninitialized memory.

Previous: Taylor BlauNext: Taylor Blau
Message 15 of 17 in “Bug report: git -L requires excessive memory.”
  1. man dogOct 29, 2022
  2. SZEDER GáborOct 31, 2022
  3. Taylor BlauOct 31, 2022
  4. 0/3 line-log: plug some memory leaksSZEDER Gábor, Nov 2, 2022
  5. 1/3 line-log: free diff queue when processing non-merge commitsSZEDER Gábor, Nov 2, 2022
  6. Taylor BlauNov 3, 2022
  7. SZEDER GáborNov 7, 2022
  8. Ævar Arnfjörð BjarmasonNov 7, 2022
  9. SZEDER GáborNov 7, 2022
  10. Taylor BlauNov 8, 2022
  11. 2/3 line-log: free the diff queues' arrays when processing merge commitsSZEDER Gábor, Nov 2, 2022
  12. Taylor BlauNov 3, 2022
  13. 3/3 diff.c: use diff_free_queue()SZEDER Gábor, Nov 2, 2022
  14. Taylor BlauNov 3, 2022
  15. SZEDER GáborNov 7, 2022
  16. Taylor BlauNov 8, 2022
  17. Ævar Arnfjörð BjarmasonNov 3, 2022

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.