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

Re: [PATCH] merge-ort: only do pointer arithmetic for non-empty lists

From
Junio C Hamano <gitster@pobox.com>
Date
Apr 10, 2021, 22:56 UTC
Message-ID
<xmqq8s5pkav2.fsf@gitster.g>
In-Reply-To
<1866b90b-fe07-18df-0d60-e2350d935375@web.de>
René Scharfe <l.s.r@web.de> writes:
Show 29 quoted lines
>> diff --git a/merge-ort.c b/merge-ort.c
>> index 5e118a85ee04..4da4b4688336 100644
>> --- a/merge-ort.c
>> +++ b/merge-ort.c
>> @@ -2504,8 +2504,10 @@ static void write_tree(struct object_id *result_oid,
>>  	 * We won't use relevant_entries again and will let it just pop off the
>>  	 * stack, so there won't be allocation worries or anything.
>>  	 */
>> -	relevant_entries.items = versions->items + offset;
>> -	relevant_entries.nr = versions->nr - offset;
>> +	if (versions->nr) {
>> +		relevant_entries.items = versions->items + offset;
>> +		relevant_entries.nr = versions->nr - offset;
>> +	}
>>  	QSORT(relevant_entries.items, relevant_entries.nr, tree_entry_order);
>
> Reading the diff I was wondering if QSORT now gets handed uninitialized
> values if version-nr is 0.  The answer is no -- relevant_entries is
> initialized at declaration.  Otherwise the compiler would have probably
> warned, but sometimes it gets confused.
>
> I wonder why relevant_entries is introduced at all, though.  It's not
> referenced later.  So how about this instead?
>
> 	if (versions->nr)
> 		QSORT(versions->items + offset, nr, tree_entry_order);
>
> The intent to sort the last versions->nr-offset entries of versions,
> but only if it's not empty, is easier to see like this, I think.

That does make sense. I wonder if there needs some assertion to ensure that "offset" does not exceed "versions.nr", though.

Previous: René ScharfeNext: Andrzej Hunt
Message 3 of 9 in “merge-ort: only do pointer arithmetic for non-empty lists”
  1. merge-ort: only do pointer arithmetic for non-empty listsAndrzej Hunt via GitGitGadget, Apr 10, 2021
  2. René ScharfeApr 10, 2021
  3. Junio C HamanoApr 10, 2021
  4. Andrzej HuntApr 11, 2021
  5. Andrzej HuntApr 11, 2021
  6. merge-ort: only do pointer arithmetic for non-empty listsAndrzej Hunt via GitGitGadget, Apr 11, 2021
  7. Elijah NewrenApr 12, 2021
  8. Junio C HamanoApr 12, 2021
  9. Junio C HamanoApr 12, 2021

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.