Message ID | 20211208144757.37641-2-alx.manpages@gmail.com |
---|---|
State | Not applicable |
Headers |
Return-Path: <libc-alpha-bounces+patchwork=sourceware.org@sourceware.org> X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id AF11D385842F for <patchwork@sourceware.org>; Wed, 8 Dec 2021 14:50:40 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org AF11D385842F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1638975040; bh=tV+pDHiqXhAZ0jLJ1ybmwZtDdnfF3HO17CDvFBRnnHA=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=caAd5Fl96zL0y4/53V6dwsuDYOziE3psWUkyUisOYjHlB6eJTPTe5e4G/jit5YYje 4lQcfAyMQ3uR0xjvtwr/itNLEbRjtrey5jXv4ZjnKtv1NwMacfnczAC+QZj7fdwgXB i/p9RQ8ObA2IzODKsLgRwHlBTVQxgc5e4WWBJOHs= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) by sourceware.org (Postfix) with ESMTPS id 9C1073858434 for <libc-alpha@sourceware.org>; Wed, 8 Dec 2021 14:50:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 9C1073858434 Received: by mail-wm1-x32b.google.com with SMTP id p3-20020a05600c1d8300b003334fab53afso4332838wms.3 for <libc-alpha@sourceware.org>; Wed, 08 Dec 2021 06:50:19 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=tV+pDHiqXhAZ0jLJ1ybmwZtDdnfF3HO17CDvFBRnnHA=; b=ce7mj36igcYfReAGMJFZrraNp+vaphf43uc5seTO3rh+wvGCOi7WF6FbdnnNXb1pGq OWfn7uH3Thxym3htKYJdy7s3XYlX82Rj5N5JtgoWc8SQVFXaL37ScF4nciyjKOJ1/P+g zg5SzOsDSR2GKg2irLYf+xbm3NSf5x4FQPF2Ur8EHUtBZB9dO3zP7LoRCYHwuZsZ5NqI zLvVKLlODlQge47KPI0eu7khItMx4aqK9A3Pe0TwfeqePK9RZTmNzwXStQd1aQQCn5RV WRX0yyIX4LemDkKp7375zX0oubmwgqp/dnRWcWBTrYbBVqw8fVWDIMZ/wnYX04mEi543 I7XQ== X-Gm-Message-State: AOAM530VsjEuZdof4IPg2bY9mxLVYEeyASr2K8eAQc0zcwi084fv7qiN x5TYl5o80cGftlqCbi0KBcgM13nf2fw= X-Google-Smtp-Source: ABdhPJyiHqaI7kjKnAqcCvcX8xUEJWyR0xhYLwOsMT8+SqionnV4z6PpcPJUGR6BMteUwTrp9gKHRg== X-Received: by 2002:a7b:cbd8:: with SMTP id n24mr16233958wmi.150.1638975018810; Wed, 08 Dec 2021 06:50:18 -0800 (PST) Received: from sqli.sqli.com ([195.53.121.100]) by smtp.googlemail.com with ESMTPSA id a9sm2990660wrt.66.2021.12.08.06.50.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 08 Dec 2021 06:50:18 -0800 (PST) To: libc-alpha@sourceware.org Subject: [RFC v3 2/3] sys/types.h: struct timespec: Use __snseconds_t for tv_nsec Date: Wed, 8 Dec 2021 15:47:58 +0100 Message-Id: <20211208144757.37641-2-alx.manpages@gmail.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20211207111957.8087-1-alx.manpages@gmail.com> References: <20211207111957.8087-1-alx.manpages@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list <libc-alpha.sourceware.org> List-Unsubscribe: <https://sourceware.org/mailman/options/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=unsubscribe> List-Archive: <https://sourceware.org/pipermail/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-request@sourceware.org?subject=help> List-Subscribe: <https://sourceware.org/mailman/listinfo/libc-alpha>, <mailto:libc-alpha-request@sourceware.org?subject=subscribe> From: Alejandro Colomar via Libc-alpha <libc-alpha@sourceware.org> Reply-To: Alejandro Colomar <alx.manpages@gmail.com> Cc: Alejandro Colomar <alx.manpages@gmail.com>, Rich Felker <dalias@libc.org>, Stefan Puiu <stefan.puiu@gmail.com>, Andreas Schwab <schwab@linux-m68k.org>, Michael Kerrisk <mtk.manpages@gmail.com>, =?utf-8?b?0L3QsNCx?= <nabijaczleweli@nabijaczleweli.xyz>, Jakub Wilk <jwilk@jwilk.net>, Joseph Myers <joseph@codesourcery.com> Errors-To: libc-alpha-bounces+patchwork=sourceware.org@sourceware.org Sender: "Libc-alpha" <libc-alpha-bounces+patchwork=sourceware.org@sourceware.org> |
Series |
None
|
|
Commit Message
Alejandro Colomar
Dec. 8, 2021, 2:47 p.m. UTC
The timespec(3) structure uses long for tv_nsec,
except if __x86_64__ && __ILP32__,
in which case it uses long long.
Use __snseconds_t to simplify the definition of struct timespec.
Link: linux-man <https://lore.kernel.org/linux-man/ec1dcc655184f6cdaae40ff8b7970b750434e4ef.1638123425.git.nabijaczleweli@nabijaczleweli.xyz/T/>
Link: glibc <https://sourceware.org/pipermail/libc-alpha/2021-December/133702.html>
Cc: наб <nabijaczleweli@nabijaczleweli.xyz>
Cc: Jakub Wilk <jwilk@jwilk.net>
Cc: Zack Weinberg <zackw@panix.com>
Cc: Stefan Puiu <stefan.puiu@gmail.com>
Cc: Michael Kerrisk (man-pages) <mtk.manpages@gmail.com>
Cc: H.J. Lu <hjl.tools@gmail.com>
Cc: Joseph Myers <joseph@codesourcery.com>
Cc: Rich Felker <dalias@libc.org>
Cc: Andreas Schwab <schwab@linux-m68k.org>
Cc: Paul Eggert <eggert@cs.ucla.edu>
Signed-off-by: Alejandro Colomar <alx.manpages@gmail.com>
---
time/bits/types/struct_timespec.h | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
Comments
On Wed, Dec 8, 2021, at 9:47 AM, Alejandro Colomar wrote: > > Use __snseconds_t to simplify the definition of struct timespec. ... > #if __WORDSIZE == 64 \ > || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \ > || (__TIMESIZE == 32 && !defined __USE_TIME_BITS64) > - __syscall_slong_t tv_nsec; /* Nanoseconds. */ > + __snseconds_t tv_nsec; /* Nanoseconds. */ All my grousing about spec bugs aside, the _point_ of having tv_nsec's type be a typedef is that it ought to be possible to make this preprocessor conditional (and the rest of the cases in the same if-else chain) less hairy. Can you look into that please? zw
On 12/8/21 15:53, Zack Weinberg wrote: > On Wed, Dec 8, 2021, at 9:47 AM, Alejandro Colomar wrote: >> >> Use __snseconds_t to simplify the definition of struct timespec. > ... >> #if __WORDSIZE == 64 \ >> || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \ >> || (__TIMESIZE == 32 && !defined __USE_TIME_BITS64) >> - __syscall_slong_t tv_nsec; /* Nanoseconds. */ >> + __snseconds_t tv_nsec; /* Nanoseconds. */ > > All my grousing about spec bugs aside, the _point_ of having tv_nsec's type be a typedef is that it ought to be possible to make this preprocessor conditional (and the rest of the cases in the same if-else chain) less hairy. Can you look into that please? Yes, that's what I'd like to achieve in the end too. However, I first wanted to make sure that I get the definition of snseconds_t right. I'd remove those ifdefs in a 4th patch (definitely in a separate patch from this one changing the type), so that each one is clearly understandable. Can you please confirm if this change by itself is correct for all cases? Thanks, Alex
On Wed, 8 Dec 2021, Zack Weinberg wrote: > On Wed, Dec 8, 2021, at 9:47 AM, Alejandro Colomar wrote: > > > > Use __snseconds_t to simplify the definition of struct timespec. > ... > > #if __WORDSIZE == 64 \ > > || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \ > > || (__TIMESIZE == 32 && !defined __USE_TIME_BITS64) > > - __syscall_slong_t tv_nsec; /* Nanoseconds. */ > > + __snseconds_t tv_nsec; /* Nanoseconds. */ > > All my grousing about spec bugs aside, the _point_ of having tv_nsec's > type be a typedef is that it ought to be possible to make this > preprocessor conditional (and the rest of the cases in the same if-else > chain) less hairy. Can you look into that please? The question about whether there is padding around tv_nsec is a separate one to what the type of tv_nsec is. I don't see how you can avoid the conditionals related to padding, which is what these conditionals are about. And the preprocessor doesn't readily lend itself to saying "if tv_sec is 64-bit and tv_nsec is 32-bit" to describe when padding is involved.
On 12/8/21 16:24, Joseph Myers wrote: > On Wed, 8 Dec 2021, Zack Weinberg wrote: > >> On Wed, Dec 8, 2021, at 9:47 AM, Alejandro Colomar wrote: >>> >>> Use __snseconds_t to simplify the definition of struct timespec. >> ... >>> #if __WORDSIZE == 64 \ >>> || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \ >>> || (__TIMESIZE == 32 && !defined __USE_TIME_BITS64) >>> - __syscall_slong_t tv_nsec; /* Nanoseconds. */ >>> + __snseconds_t tv_nsec; /* Nanoseconds. */ >> >> All my grousing about spec bugs aside, the _point_ of having tv_nsec's >> type be a typedef is that it ought to be possible to make this >> preprocessor conditional (and the rest of the cases in the same if-else >> chain) less hairy. Can you look into that please? > > The question about whether there is padding around tv_nsec is a separate > one to what the type of tv_nsec is. I don't see how you can avoid the > conditionals related to padding, which is what these conditionals are > about. And the preprocessor doesn't readily lend itself to saying "if > tv_sec is 64-bit and tv_nsec is 32-bit" to describe when padding is > involved. > How about defining _WIDTH macros for integral types (mirroring C2X _WIDTH macros)? That way we could make use of them to make these ifdefs simpler. I guess it would be something like #if __SNSECONDS_WIDTH == 32 && __TIME_WIDTH == 64 ... Would this make sense? Thanks, Alex
On Wed, Dec 8, 2021, at 10:24 AM, Joseph Myers wrote: > On Wed, 8 Dec 2021, Zack Weinberg wrote: >> All my grousing about spec bugs aside, the _point_ of having tv_nsec's >> type be a typedef is that it ought to be possible to make this >> preprocessor conditional (and the rest of the cases in the same if-else >> chain) less hairy. Can you look into that please? > > The question about whether there is padding around tv_nsec is a separate > one to what the type of tv_nsec is. I don't see how you can avoid the > conditionals related to padding, which is what these conditionals are > about. And the preprocessor doesn't readily lend itself to saying "if > tv_sec is 64-bit and tv_nsec is 32-bit" to describe when padding is > involved. I'm hoping we can get to something like this struct timespec { __time_t tv_sec; #if __TIMESPEC_PAD_BEFORE_TV_NSEC int : __TIMESPEC_PAD_BEFORE_TV_NSEC; #endif __snseconds_t tv_nsec; }; where __TIMESPEC_PAD_BEFORE_TV_NSEC is defined in some appropriate sysdeps header (bits/wordsize.h is the first thing to come to mind but I'm not sure it's optimal). Maybe we'd also need __TIMESPEC_PAD_AFTER_TV_NSEC, if the alignment requirement of __time_t is insufficient to make the compiler insert tail padding on some architectures. zw
On Wed, Dec 08, 2021 at 10:59:04AM -0500, Zack Weinberg wrote: > On Wed, Dec 8, 2021, at 10:24 AM, Joseph Myers wrote: > > On Wed, 8 Dec 2021, Zack Weinberg wrote: > >> All my grousing about spec bugs aside, the _point_ of having tv_nsec's > >> type be a typedef is that it ought to be possible to make this > >> preprocessor conditional (and the rest of the cases in the same if-else > >> chain) less hairy. Can you look into that please? > > > > The question about whether there is padding around tv_nsec is a separate > > one to what the type of tv_nsec is. I don't see how you can avoid the > > conditionals related to padding, which is what these conditionals are > > about. And the preprocessor doesn't readily lend itself to saying "if > > tv_sec is 64-bit and tv_nsec is 32-bit" to describe when padding is > > involved. > > I'm hoping we can get to something like this > > struct timespec { > __time_t tv_sec; > #if __TIMESPEC_PAD_BEFORE_TV_NSEC > int : __TIMESPEC_PAD_BEFORE_TV_NSEC; > #endif > __snseconds_t tv_nsec; > }; > > where __TIMESPEC_PAD_BEFORE_TV_NSEC is defined in some appropriate > sysdeps header (bits/wordsize.h is the first thing to come to mind > but I'm not sure it's optimal). As we did in musl, you can write it as an integer constant expression assuming you have __BYTE_ORDER, in terms of sizeof(time_t) and sizeof(long). You don't even need a preprocessor conditional around it because :0 is valid. > Maybe we'd also need __TIMESPEC_PAD_AFTER_TV_NSEC, if the alignment > requirement of __time_t is insufficient to make the compiler insert > tail padding on some architectures. Yes, you do, for that reason. At least x86 and m68k are affected. Maybe some other obscure 32-bit archs too. Rich
diff --git a/time/bits/types/struct_timespec.h b/time/bits/types/struct_timespec.h index 489e81136d..cc366590d9 100644 --- a/time/bits/types/struct_timespec.h +++ b/time/bits/types/struct_timespec.h @@ -18,14 +18,14 @@ struct timespec #if __WORDSIZE == 64 \ || (defined __SYSCALL_WORDSIZE && __SYSCALL_WORDSIZE == 64) \ || (__TIMESIZE == 32 && !defined __USE_TIME_BITS64) - __syscall_slong_t tv_nsec; /* Nanoseconds. */ + __snseconds_t tv_nsec; /* Nanoseconds. */ #else # if __BYTE_ORDER == __BIG_ENDIAN - int: 32; /* Padding. */ - long int tv_nsec; /* Nanoseconds. */ + int: 32; /* Padding. */ + __snseconds_t tv_nsec; /* Nanoseconds. */ # else - long int tv_nsec; /* Nanoseconds. */ - int: 32; /* Padding. */ + __snseconds_t tv_nsec; /* Nanoseconds. */ + int: 32; /* Padding. */ # endif #endif };