From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from hall.aurel32.net (hall.aurel32.net [IPv6:2001:bc8:30d7:100::1]) by sourceware.org (Postfix) with ESMTPS id 881663858C55 for ; Tue, 1 Nov 2022 11:57:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 881663858C55 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=aurel32.net Authentication-Results: sourceware.org; spf=none smtp.mailfrom=aurel32.net DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=aurel32.net ; s=202004.hall; h=In-Reply-To:Content-Type:MIME-Version:References: Message-ID:Subject:To:From:Date:Content-Transfer-Encoding:Cc:From:Reply-To: Subject:Content-ID:Content-Description:X-Debbugs-Cc; bh=0QV2s+dIzLrQLiewfrFC2AC/zkQ3gdksnPd5yCpYi9Y=; b=I9G8zDopdlibg4H22Zdljsz1gv 9l9KRImV/Gq9JWa0S1eRKQv+BPAekYlLLyWqEjrheUjUUpTKmqOZg3fMNy+KjdL+grHe48B44+8jh CWJS+jhHMFAxzTniFwXNPY5R4B32+mITEevC3L+hM2WwRONfZIOdacKdY0vYDFiwUsAJUIB9PHoCd BKU7WI4Y0Op597AWjxoqiWiCiMqm5INyyUIc7HneQjIvZOibgpFvvgPVnfHI+3ihQ62YDkdpC69k/ zzEDBKIRrCYS3O/EoPqg9Fx1T3kwkx2jmbl+WFg/3PdKWEokIZWzg5UDh3zqLr/gJ385wS//SKg9A PyQBzD2A==; Received: from [2a01:e34:ec5d:a741:8a4c:7c4e:dc4c:1787] (helo=ohm.rr44.fr) by hall.aurel32.net with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1opptl-002CHe-Oq; Tue, 01 Nov 2022 12:57:41 +0100 Received: from aurel32 by ohm.rr44.fr with local (Exim 4.96) (envelope-from ) id 1opptl-006XUw-1H; Tue, 01 Nov 2022 12:57:41 +0100 Date: Tue, 1 Nov 2022 12:57:41 +0100 From: Aurelien Jarno To: Adhemerval Zanella Netto , YunQiang Su , libc-alpha@sourceware.org Subject: Re: [PATCH] linux: Fix fstatat with !XSTAT_IS_XSTAT64 and __TIMESIZE=64 (BZ #29730) Message-ID: Mail-Followup-To: Adhemerval Zanella Netto , YunQiang Su , libc-alpha@sourceware.org References: <20221028205440.1091298-1-aurelien@aurel32.net> <85bbb5fa-735e-254e-4467-e2726fd0ecc3@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/2.2.7 (2022-08-07) X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,SPF_HELO_PASS,SPF_NONE,TXREP autolearn=ham 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 2022-11-01 11:55, Aurelien Jarno wrote: > On 2022-10-31 23:20, Aurelien Jarno wrote: > > On 2022-10-31 14:25, Adhemerval Zanella Netto via Libc-alpha wrote: > > > > > > > > > On 31/10/22 13:03, Adhemerval Zanella Netto wrote: > > > > > > > > > > > > On 28/10/22 17:54, Aurelien Jarno wrote: > > > >> Commit 6e8a0aac2f883 ("time: Fix overflow itimer tests on 32-bit > > > >> systems") changed in_time_t_range to assume a 32-bit time_t. Therefore > > > >> stop calling it from __fstatat for systems with 64-bit time_t. > > > >> > > > >> Resolves: BZ #29730 > > > > > > > > Although this change is not incorrect, I think the fix I just sent [1] is > > > > slight better because it just alias non-LFS stat to LFS stat for mips > > > > (which is also a small runtime and code optimization), while avoiding > > > > touching generic code (so we need to check if trigger another issue on > > > > different architecture). > > > > > > > > [1] https://sourceware.org/pipermail/libc-alpha/2022-October/143089.html > > > > > > So I was wrong that we can assume stat and stat64 are equal on mips. Maybe > > > instead of adding the fix on stat we can make it more generic and move it > > > to is_time_t_range (so it can potentially fix usage not only on stat code) > > > as my comment on https://sourceware.org/bugzilla/show_bug.cgi?id=29730 ? > > > > Yes, we can move it to is_time_t_range, but then it gets trickier to get > > the condition. Your comment in BZ #29730 uses TIME64_NON_DEFAULT, but it > > is not defined for OpenRISC, so it means it will cast to time_t. However > > this has been explicitly changed that way in commit 6e8a0aac2f883 to > > unbreak OpenRISC. > > > > We probably want a condition that is "kernel ABI uses 32-bit time_t". Do > > we have something like that already defined? > > I guess this can be checked with: > (__WORDSIZE == 32 && (!defined __SYSCALL_WORDSIZE || __SYSCALL_WORDSIZE == 32)) > > And then we can rename the function to is_ktime_t_range to better > reflect its behaviour. Looking at it more in details, I am not sure it's a good idea, as this function is also used for non-syscall cases, like in mktime or timegm. While that works because there is a __TIMESIZE != 64 guard, this still seems fragile. In that regard, I think the approach from YunQiang to explicitly call either one or the other function might be better. An hybrid solution would be to define both in_time_t_range and in_ktime_t_range, the later being the one called in_int32_t_range in YunQiang's patch. What do you think? -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net