From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS53758 23.128.96.0/24 X-Spam-Status: No, score=-4.0 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id BB38E1F934 for ; Sat, 10 Apr 2021 22:56:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235106AbhDJW4U (ORCPT ); Sat, 10 Apr 2021 18:56:20 -0400 Received: from pb-smtp1.pobox.com ([64.147.108.70]:64612 "EHLO pb-smtp1.pobox.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234992AbhDJW4U (ORCPT ); Sat, 10 Apr 2021 18:56:20 -0400 Received: from pb-smtp1.pobox.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id 1F8DFC5036; Sat, 10 Apr 2021 18:56:04 -0400 (EDT) (envelope-from junio@pobox.com) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type:content-transfer-encoding; s=sasl; bh=yYtATD9updAa 13c0zGg+VpH3icc=; b=yE/XlU4mb+4fTatew79gfhoIVZT0luC+5XjM7nON4kOq buxBmLpvU0yM482VUo+pOcdqonwRkpCNIk+lZ4OcUp1FTggGeXO5Sz21HXzsJlFG UbByRdl+V5FanMhS4uY7zHkb+5x5NrWAlA3sBMweQKH8ZAPPvzEp6aPBrq+wjnc= DomainKey-Signature: a=rsa-sha1; c=nofws; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type:content-transfer-encoding; q=dns; s=sasl; b=P+TgkZ MHwUmnKe2Bi7AEs1n6l644i1HTgjf/atUf7aO2d2rG3WXzC+eoGOHdhXlNgT7xpp k8LaKJMqaEqItfV4SuAg60sE+4IG/AbAYFHWg5oGMNJJwfZf18B6G8dbcY/XFasr ssRfdnLfL+MKVTPsxOOsJw/2LhYtzJb/KAlDE= Received: from pb-smtp1.nyi.icgroup.com (unknown [127.0.0.1]) by pb-smtp1.pobox.com (Postfix) with ESMTP id C266BC5034; Sat, 10 Apr 2021 18:56:03 -0400 (EDT) (envelope-from junio@pobox.com) Received: from pobox.com (unknown [34.74.119.39]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp1.pobox.com (Postfix) with ESMTPSA id 5508EC5032; Sat, 10 Apr 2021 18:56:02 -0400 (EDT) (envelope-from junio@pobox.com) From: Junio C Hamano To: =?utf-8?Q?Ren=C3=A9?= Scharfe Cc: Andrzej Hunt via GitGitGadget , git@vger.kernel.org, Elijah Newren , Andrzej Hunt , Andrzej Hunt Subject: Re: [PATCH] merge-ort: only do pointer arithmetic for non-empty lists References: <1866b90b-fe07-18df-0d60-e2350d935375@web.de> Date: Sat, 10 Apr 2021 15:56:01 -0700 In-Reply-To: <1866b90b-fe07-18df-0d60-e2350d935375@web.de> (=?utf-8?Q?=22R?= =?utf-8?Q?en=C3=A9?= Scharfe"'s message of "Sat, 10 Apr 2021 13:48:18 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 X-Pobox-Relay-ID: EC5F6748-9A4F-11EB-9B10-D152C8D8090B-77302942!pb-smtp1.pobox.com Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Ren=C3=A9 Scharfe writes: >> 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 =3D versions->items + offset; >> - relevant_entries.nr =3D versions->nr - offset; >> + if (versions->nr) { >> + relevant_entries.items =3D versions->items + offset; >> + relevant_entries.nr =3D 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.