From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from zimbra.cs.ucla.edu (zimbra.cs.ucla.edu [131.179.128.68]) by sourceware.org (Postfix) with ESMTPS id 2CCD83858D1E for ; Fri, 10 Mar 2023 21:41:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2CCD83858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=cs.ucla.edu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=cs.ucla.edu Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 6995216004F; Fri, 10 Mar 2023 13:41:16 -0800 (PST) Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10032) with ESMTP id nGVXVLQcDGzS; Fri, 10 Mar 2023 13:41:15 -0800 (PST) Received: from localhost (localhost [127.0.0.1]) by zimbra.cs.ucla.edu (Postfix) with ESMTP id 92BB3160090; Fri, 10 Mar 2023 13:41:15 -0800 (PST) DKIM-Filter: OpenDKIM Filter v2.9.2 zimbra.cs.ucla.edu 92BB3160090 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cs.ucla.edu; s=78364E5A-2AF3-11ED-87FA-8298ECA2D365; t=1678484475; bh=jiCL/VBUwps5e3W5PwYpquCAtPSXav74HplerYocYA0=; h=Message-ID:Date:MIME-Version:To:From:Subject:Content-Type: Content-Transfer-Encoding; b=KREZ3aW4vtamAB6KGgyc/zbgRgXCcPr4H5ytf3V7jZLEJDBC+Z5ZuhESQL2Xy67nt xC43Nstb6hFvHtumBY2byGX5DjbJD0ENSmH5QqHDw0zLx94DSdzAP20Kcc+UAY3wXX ZRcP2x6jcVBeveB+evtH2VB7z/vAUXhLZJv8PD1A= X-Virus-Scanned: amavisd-new at zimbra.cs.ucla.edu Received: from zimbra.cs.ucla.edu ([127.0.0.1]) by localhost (zimbra.cs.ucla.edu [127.0.0.1]) (amavisd-new, port 10026) with ESMTP id 2eaGiggo6jjB; Fri, 10 Mar 2023 13:41:15 -0800 (PST) Received: from [192.168.1.9] (cpe-172-91-119-151.socal.res.rr.com [172.91.119.151]) by zimbra.cs.ucla.edu (Postfix) with ESMTPSA id 5C41C16004F; Fri, 10 Mar 2023 13:41:15 -0800 (PST) Message-ID: <75761f49-474e-6c3c-7029-03b30e83da3e@cs.ucla.edu> Date: Fri, 10 Mar 2023 13:41:15 -0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Content-Language: en-US To: Adhemerval Zanella , libc-alpha@sourceware.org, "Andreas K . Huettel" , Florian Weimer References: <20230302145732.2293756-1-adhemerval.zanella@linaro.org> <20230302145732.2293756-4-adhemerval.zanella@linaro.org> From: Paul Eggert Organization: UCLA Computer Science Department Subject: Re: [PATCH v6 3/3] linux: Set internal DIR filepos as off64_t (BZ #23960, BZ #24050) In-Reply-To: <20230302145732.2293756-4-adhemerval.zanella@linaro.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,JMQ_SPF_NEUTRAL,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 2023-03-02 06:57, Adhemerval Zanella wrote: > + for (i = 0; i < dirstream_loc_size (&dirp->locs); i++) > + if (*dirstream_loc_at (&dirp->locs, i) == dirp->filepos) > + break; > + /* It should be pre-allocated on readdir. */ > + assert (i != dirstream_loc_size (&dirp->locs)); This should be something like the following, to avoid unnecessary work when assertions are disabled: for (long int i = 0; ; i++) { assert (i < dirstream_loc_size (&dirp->locs)); if (*dirstream_loc_at (&dirp->locs, i) == dirp->filepos) break; } > + /* This assignment might overflow, however most likely ENOME would > + happen long before. */ > + dsp.p.info = i; This doesn't sound right. The allocator should never create a table with more than LONG_MAX entries because the upper part of any such table would be useless. If that is done right, the assignment cannot overflow. > +_Static_assert (sizeof (long int) == sizeof (off64_t), > + "sizeof (long int) != sizeof (off64_t)"); This is confusing. First, we need require only that long int be at least as wide as off64_t; it doesn't have to be exactly the same width. Second, why both "==" and "!="? Third, why not use plain "static_assert" with one arg instead of the old-fashioned "_Static_assert" with two? We can support this form of static_assert on older compilers - see how Gnulib does it. > +static __always_inline bool > +telldir_need_dirstream (__off64_t d_off) > +{ > + return d_off >= 1UL << 31; > +} Safer would be '! (TYPE_MINIMUM (off_t) <= d_off && d_off <= TYPE_MAXIMUM (off_t))', in case d_off is negative (or off_t isn't 32-bit :-).