Message ID | 85291ec316dcd3f3a3155488a8c290298650dc77.1578824547.git.alistair.francis@wdc.com |
---|---|
State | Committed |
Headers |
Received: (qmail 9022 invoked by alias); 12 Jan 2020 10:40:08 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <libc-alpha.sourceware.org> List-Unsubscribe: <mailto:libc-alpha-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:libc-alpha-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/libc-alpha/> List-Post: <mailto:libc-alpha@sourceware.org> List-Help: <mailto:libc-alpha-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 8960 invoked by uid 89); 12 Jan 2020 10:40:07 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-26.6 required=5.0 tests=AWL, BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3 autolearn=ham version=3.3.1 spammy=HContent-Transfer-Encoding:8bit X-HELO: esa6.hgst.iphmx.com DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=wdc.com; i=@wdc.com; q=dns/txt; s=dkim.wdc.com; t=1578825606; x=1610361606; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=x3wP5EGEY1w4imPJ4IMeNIBOhQLIXjRSay3JDuNPISk=; b=d8wB084accEHNUJ92ZNofSWYNtuUBNpb9M81uSkQ3mVx+wvVPnnjEi5Y +e5aelqKO3osW18UoJgWgMtaVJzXVj6qgL2a4gvTLbZ2Nt2kfalhSoMNz +QjtWqVfueXKQ4mo/v0Jq0AQD/cOzJnE/NoEVBYA4A2mBpo5qY369J8w3 ma74gaXmI1z3bbN3Ygpm3INhissDBwGlNqLN0sak15emFSNNQAxVM4q31 l8igohulyko4c2sXQ2UhrB5qBXwzgKzrZgycLzedXetfkKWAbBMZUXXLo cK+amG/AXdO8wUG8uAXQPk7qG0tIFg/PldtRy5YY9tK/HEHdxNssStKJx Q==; IronPort-SDR: EVSv4oM6xi8fbwCx36Mx9leuSkga/l72g8HKX9aGL6aH4YKTX5n8+TY76JOC249mNmTwz/dMb3 /xaGXmaD+O/fWpCmAN2uJcTce1bgcqng57qUVs9bH8NJjEjual7zaRqPC7zFLRxkBYDlkAQc9I Z20TzU921WbBm8gJvCUAwATC9Gu0m8cOhc+GFDCUm/a9u6A+qIhI5QWBSqQobM9oNHstAkcYoB 2p2UaJzAerOc8ZQescLuhr+rU1CdfRDdiKv0HuIMBM7m/RzxBP/1k6J5WExAcb43diAC8q5ozw E80= IronPort-SDR: JyROPqBuRibCdxjN2UDMx7PtIlxYdWfGyr/HFMLgWcCtYz6Q6tBX7KdrI4HswFU+TGiOgjiWMp c0bkHhTXMgAxMn6RP47vnB9kd6qkTLgcAQLdY8sX11HZn6MTRQCG1tbB/uBbCXIJxfZHWYP/Ix o9FlI2PLwno+bUXajTFjAqiepOt5c79UEjKF1pvY9pOWLxt5raveveFbF2hSHH+9ngquWtbLO3 oALKvGH5ozOYLRHAjVLsh9+lzKEU6Nq5v1S/WBpQVDyDtzerRYCW3B+E/GZIP7xIavaFYXZJiC +zlpmrpqulA5jP54Wq2WzqWr IronPort-SDR: //gRvITJop81LuLAshsyUjW6n1qumSuB1buEb6j8fpl9a8XIpj9rVg/oFQGRtTF0GCgKXkRFOI LsFKVuDPznLV+XU1s8U5CocW9XPTglnDaYrUZ1VivMYmovwA9hjPl/WvQIyK1lgw+kifVwpK1r dJBwT0gWmaelmRGWBeiKK7J702/tdH6azuNrgx6N5B0NoIT/oqvV8O1X4yOK6oH1x2gvsjUN/y eAygsiTLEPZSx1d82AvfzVVps5EjWldt6pYgMRjZhIsJiTgfxM122v/wXN8RAFIYyGy4Gmx9l9 qIg= WDCIronportException: Internal From: Alistair Francis <alistair.francis@wdc.com> To: libc-alpha@sourceware.org Cc: arnd@arndb.de, adhemerval.zanella@linaro.org, fweimer@redhat.com, joseph@codesourcery.com, palmerdabbelt@google.com, macro@wdc.com, zongbox@gmail.com, alistair.francis@wdc.com, alistair23@gmail.com Subject: [RFC v6 03/23] time: Add a timeval with a long tv_sec and tv_usec Date: Sun, 12 Jan 2020 02:33:38 -0800 Message-Id: <85291ec316dcd3f3a3155488a8c290298650dc77.1578824547.git.alistair.francis@wdc.com> In-Reply-To: <cover.1578824547.git.alistair.francis@wdc.com> References: <cover.1578824547.git.alistair.francis@wdc.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit |
Commit Message
Alistair Francis
Jan. 12, 2020, 10:33 a.m. UTC
On y2038 safe 32-bit systems the Linux kernel expects itimerval to use a 32-bit time_t, even though the other time_t's are 64-bit. To address this let's add a timeval_long struct to be used internally. --- include/time.h | 29 +++++++++++++++++++++++++++++ time/bits/types/struct_timeval.h | 8 ++++++++ 2 files changed, 37 insertions(+)
Comments
On Sun, 12 Jan 2020, Alistair Francis wrote: > On y2038 safe 32-bit systems the Linux kernel expects itimerval to > use a 32-bit time_t, even though the other time_t's are 64-bit. To > address this let's add a timeval_long struct to be used internally. > --- > include/time.h | 29 +++++++++++++++++++++++++++++ > time/bits/types/struct_timeval.h | 8 ++++++++ bits/types/ headers are *installed* headers that should only define the type in question. If there is some good reason to need __timeval_long in *installed* headers (i.e. a *public* interface using this type) it should go in bits/types/struct___timeval_long.h, otherwise it should go in a non-installed header.
On Mon, Jan 13, 2020 at 12:48 AM Joseph Myers <joseph@codesourcery.com> wrote: > > On Sun, 12 Jan 2020, Alistair Francis wrote: > > > On y2038 safe 32-bit systems the Linux kernel expects itimerval to > > use a 32-bit time_t, even though the other time_t's are 64-bit. To > > address this let's add a timeval_long struct to be used internally. > > --- > > include/time.h | 29 +++++++++++++++++++++++++++++ > > time/bits/types/struct_timeval.h | 8 ++++++++ > > bits/types/ headers are *installed* headers that should only define the > type in question. If there is some good reason to need __timeval_long in > *installed* headers (i.e. a *public* interface using this type) it should > go in bits/types/struct___timeval_long.h, otherwise it should go in a > non-installed header. Ok, I don't think this should be installed. The only possible reason installing this would be if someone was manually calling the syscalls and wanted to use this type to do that. Do you have a recommendation of a non installed header where it should go? Alistair > > -- > Joseph S. Myers > joseph@codesourcery.com
Hi Alistair, > On y2038 safe 32-bit systems the Linux kernel expects itimerval to > use a 32-bit time_t, even though the other time_t's are 64-bit. To > address this let's add a timeval_long ^^^^^^^^^^^^^ - I'm not so seasoned glibc developer, but I think that the _long suffix is a bit misleading. Maybe it would be more readable to name it as struct __timeval32 ? In that way one can see from the outset that we operate on 32 bit values. Community feedback is welcome :-) > struct to be used internally. > --- > include/time.h | 29 +++++++++++++++++++++++++++++ > time/bits/types/struct_timeval.h | 8 ++++++++ > 2 files changed, 37 insertions(+) > > diff --git a/include/time.h b/include/time.h > index e5e8246eac..201342d1ca 100644 > --- a/include/time.h > +++ b/include/time.h > @@ -310,6 +310,35 @@ valid_timespec64_to_timeval (const struct > __timespec64 ts64) return tv; > } > > +/* Conversion functions for converting to/from __timeval_long > +. If the seconds field of a __timeval_long would > + overflow, they write { INT32_MAX, 999999 } to the output. */ > +static inline struct timeval > +valid_timeval_long_to_timeval64 (const struct __timeval_long tv) > +{ > + return (struct timeval) { tv.tv_sec, tv.tv_usec }; > +} > + > +static inline struct __timeval_long > +valid_timeval64_to_timeval_long (const struct timeval tv64) > +{ > + if (__glibc_unlikely (tv64.tv_sec > (time_t) 2147483647)) > + return (struct __timeval_long) { 2147483647, 999999}; > + return (struct __timeval_long) { tv64.tv_sec, tv64.tv_usec }; > +} > + > +static inline struct timespec > +valid_timeval_long_to_timespec (const struct __timeval_long tv) > +{ > + return (struct timespec) { tv.tv_sec, tv.tv_usec * 1000 }; > +} > + > +static inline struct __timeval_long > +valid_timespec_to_timeval_long (const struct timespec ts) > +{ > + return (struct __timeval_long) { (time_t) ts.tv_sec, ts.tv_nsec / > 1000 }; +} > + > /* Check if a value is in the valid nanoseconds range. Return true if > it is, false otherwise. */ > static inline bool > diff --git a/time/bits/types/struct_timeval.h > b/time/bits/types/struct_timeval.h index 70394ce886..73697689cc 100644 > --- a/time/bits/types/struct_timeval.h > +++ b/time/bits/types/struct_timeval.h > @@ -10,4 +10,12 @@ struct timeval > __time_t tv_sec; /* Seconds. */ > __suseconds_t tv_usec; /* Microseconds. */ > }; > + > +/* A version of 'struct timeval' with `long` time_t > + and suseconds_t. */ > +struct __timeval_long > +{ > + long tv_sec; /* Seconds. */ > + long tv_usec; /* Microseconds. */ > +}; > #endif Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
Hi Alistair, > On Mon, Jan 13, 2020 at 12:48 AM Joseph Myers > <joseph@codesourcery.com> wrote: > > > > On Sun, 12 Jan 2020, Alistair Francis wrote: > > > > > On y2038 safe 32-bit systems the Linux kernel expects itimerval to > > > use a 32-bit time_t, even though the other time_t's are 64-bit. To > > > address this let's add a timeval_long struct to be used > > > internally. --- > > > include/time.h | 29 > > > +++++++++++++++++++++++++++++ time/bits/types/struct_timeval.h | > > > 8 ++++++++ > > > > bits/types/ headers are *installed* headers that should only define > > the type in question. If there is some good reason to need > > __timeval_long in *installed* headers (i.e. a *public* interface > > using this type) it should go in > > bits/types/struct___timeval_long.h, otherwise it should go in a > > non-installed header. > > Ok, I don't think this should be installed. The only possible reason > installing this would be if someone was manually calling the syscalls > and wanted to use this type to do that. > > Do you have a recommendation of a non installed header where it > should go? I'm not sure if this hint will match your use case, but similar: struct __timespec64 and struct __itimerspec64 are defined in ./include/time.h This is the place for glibc internal definitions (not installed/exported). > > Alistair > > > > > -- > > Joseph S. Myers > > joseph@codesourcery.com Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Tue, Jan 14, 2020 at 8:17 PM Lukasz Majewski <lukma@denx.de> wrote: > > Hi Alistair, > > > On y2038 safe 32-bit systems the Linux kernel expects itimerval to > > use a 32-bit time_t, even though the other time_t's are 64-bit. To > > address this let's add a timeval_long > ^^^^^^^^^^^^^ - I'm not so seasoned glibc > developer, but I think that the _long suffix > is a bit misleading. > > Maybe it would be more readable to name it as struct > __timeval32 ? In that way one can see from the outset that we > operate on 32 bit values. It isn't explicitly 32-bit, it's just always a long which is why I went with long in the name instead. I'm happy to change the name, it doesn't really matter too much as it's only user internally. I'll wait to see what others think before changing it. Alistair > > Community feedback is welcome :-) > > > struct to be used internally. > > --- > > include/time.h | 29 +++++++++++++++++++++++++++++ > > time/bits/types/struct_timeval.h | 8 ++++++++ > > 2 files changed, 37 insertions(+) > > > > diff --git a/include/time.h b/include/time.h > > index e5e8246eac..201342d1ca 100644 > > --- a/include/time.h > > +++ b/include/time.h > > @@ -310,6 +310,35 @@ valid_timespec64_to_timeval (const struct > > __timespec64 ts64) return tv; > > } > > > > +/* Conversion functions for converting to/from __timeval_long > > +. If the seconds field of a __timeval_long would > > + overflow, they write { INT32_MAX, 999999 } to the output. */ > > +static inline struct timeval > > +valid_timeval_long_to_timeval64 (const struct __timeval_long tv) > > +{ > > + return (struct timeval) { tv.tv_sec, tv.tv_usec }; > > +} > > + > > +static inline struct __timeval_long > > +valid_timeval64_to_timeval_long (const struct timeval tv64) > > +{ > > + if (__glibc_unlikely (tv64.tv_sec > (time_t) 2147483647)) > > + return (struct __timeval_long) { 2147483647, 999999}; > > + return (struct __timeval_long) { tv64.tv_sec, tv64.tv_usec }; > > +} > > + > > +static inline struct timespec > > +valid_timeval_long_to_timespec (const struct __timeval_long tv) > > +{ > > + return (struct timespec) { tv.tv_sec, tv.tv_usec * 1000 }; > > +} > > + > > +static inline struct __timeval_long > > +valid_timespec_to_timeval_long (const struct timespec ts) > > +{ > > + return (struct __timeval_long) { (time_t) ts.tv_sec, ts.tv_nsec / > > 1000 }; +} > > + > > /* Check if a value is in the valid nanoseconds range. Return true if > > it is, false otherwise. */ > > static inline bool > > diff --git a/time/bits/types/struct_timeval.h > > b/time/bits/types/struct_timeval.h index 70394ce886..73697689cc 100644 > > --- a/time/bits/types/struct_timeval.h > > +++ b/time/bits/types/struct_timeval.h > > @@ -10,4 +10,12 @@ struct timeval > > __time_t tv_sec; /* Seconds. */ > > __suseconds_t tv_usec; /* Microseconds. */ > > }; > > + > > +/* A version of 'struct timeval' with `long` time_t > > + and suseconds_t. */ > > +struct __timeval_long > > +{ > > + long tv_sec; /* Seconds. */ > > + long tv_usec; /* Microseconds. */ > > +}; > > #endif > > Best regards, > > Lukasz Majewski > > -- > > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
On Wed, Jan 15, 2020 at 6:15 AM Alistair Francis <alistair23@gmail.com> wrote: > > On Tue, Jan 14, 2020 at 8:17 PM Lukasz Majewski <lukma@denx.de> wrote: > > > > Hi Alistair, > > > > > On y2038 safe 32-bit systems the Linux kernel expects itimerval to > > > use a 32-bit time_t, even though the other time_t's are 64-bit. To > > > address this let's add a timeval_long > > ^^^^^^^^^^^^^ - I'm not so seasoned glibc > > developer, but I think that the _long suffix > > is a bit misleading. > > > > Maybe it would be more readable to name it as struct > > __timeval32 ? In that way one can see from the outset that we > > operate on 32 bit values. > > It isn't explicitly 32-bit, it's just always a long which is why I > went with long in the name instead. > > I'm happy to change the name, it doesn't really matter too much as > it's only user internally. > > I'll wait to see what others think before changing it. I think you can look at it either way: the Your timeval_long works fine on both 32-bit and 64-bit architectures, but you really only need it on 32-bit ones because on 64-bit architectures (except sparc64, which has 32-bit suseconds_t !) this is the same as the default timeval anyway. In the kernel we actually ended up having both: there is a "struct __kernel_old_timeval" that is part of the uapi headers for defining structures based on the old type with no replacement such as rusage and itimerval, and internally there is a "struct old_timeval32" with two "int32_t" members that is used for providing compatibility handlers for old interfaces that have a 64-bit replacement, so the same compatibility handler can be used on native 32-bit kernels and on 64-bit kernels running 32-bit user space. Arnd
Hi Alistair, > On Tue, Jan 14, 2020 at 8:17 PM Lukasz Majewski <lukma@denx.de> wrote: > > > > Hi Alistair, > > > > > On y2038 safe 32-bit systems the Linux kernel expects itimerval to > > > use a 32-bit time_t, even though the other time_t's are 64-bit. To > > > address this let's add a timeval_long > > ^^^^^^^^^^^^^ - I'm not so seasoned glibc > > developer, but I think that the _long > > suffix is a bit misleading. > > > > Maybe it would be more readable to name it as struct > > __timeval32 ? In that way one can see from the outset that > > we operate on 32 bit values. > > It isn't explicitly 32-bit, it's just always a long which is why I > went with long in the name instead. > > I'm happy to change the name, it doesn't really matter too much as > it's only user internally. > > I'll wait to see what others think before changing it. Yes, I also think that it would be best to wait for more input from the community. > > > Alistair > > > > > Community feedback is welcome :-) > > > > > struct to be used internally. > > > --- > > > include/time.h | 29 > > > +++++++++++++++++++++++++++++ time/bits/types/struct_timeval.h | > > > 8 ++++++++ 2 files changed, 37 insertions(+) > > > > > > diff --git a/include/time.h b/include/time.h > > > index e5e8246eac..201342d1ca 100644 > > > --- a/include/time.h > > > +++ b/include/time.h > > > @@ -310,6 +310,35 @@ valid_timespec64_to_timeval (const struct > > > __timespec64 ts64) return tv; > > > } > > > > > > +/* Conversion functions for converting to/from __timeval_long > > > +. If the seconds field of a __timeval_long would > > > + overflow, they write { INT32_MAX, 999999 } to the output. */ > > > +static inline struct timeval > > > +valid_timeval_long_to_timeval64 (const struct __timeval_long tv) > > > +{ > > > + return (struct timeval) { tv.tv_sec, tv.tv_usec }; > > > +} > > > + > > > +static inline struct __timeval_long > > > +valid_timeval64_to_timeval_long (const struct timeval tv64) > > > +{ > > > + if (__glibc_unlikely (tv64.tv_sec > (time_t) 2147483647)) > > > + return (struct __timeval_long) { 2147483647, 999999}; > > > + return (struct __timeval_long) { tv64.tv_sec, tv64.tv_usec }; > > > +} > > > + > > > +static inline struct timespec > > > +valid_timeval_long_to_timespec (const struct __timeval_long tv) > > > +{ > > > + return (struct timespec) { tv.tv_sec, tv.tv_usec * 1000 }; > > > +} > > > + > > > +static inline struct __timeval_long > > > +valid_timespec_to_timeval_long (const struct timespec ts) > > > +{ > > > + return (struct __timeval_long) { (time_t) ts.tv_sec, > > > ts.tv_nsec / 1000 }; +} > > > + > > > /* Check if a value is in the valid nanoseconds range. Return > > > true if it is, false otherwise. */ > > > static inline bool > > > diff --git a/time/bits/types/struct_timeval.h > > > b/time/bits/types/struct_timeval.h index 70394ce886..73697689cc > > > 100644 --- a/time/bits/types/struct_timeval.h > > > +++ b/time/bits/types/struct_timeval.h > > > @@ -10,4 +10,12 @@ struct timeval > > > __time_t tv_sec; /* Seconds. */ > > > __suseconds_t tv_usec; /* Microseconds. */ > > > }; > > > + > > > +/* A version of 'struct timeval' with `long` time_t > > > + and suseconds_t. */ > > > +struct __timeval_long > > > +{ > > > + long tv_sec; /* Seconds. */ > > > + long tv_usec; /* Microseconds. */ > > > +}; > > > #endif > > > > Best regards, > > > > Lukasz Majewski > > > > -- > > > > DENX Software Engineering GmbH, Managing Director: Wolfgang > > Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, > > Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: > > lukma@denx.de Best regards, Lukasz Majewski -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de
diff --git a/include/time.h b/include/time.h index e5e8246eac..201342d1ca 100644 --- a/include/time.h +++ b/include/time.h @@ -310,6 +310,35 @@ valid_timespec64_to_timeval (const struct __timespec64 ts64) return tv; } +/* Conversion functions for converting to/from __timeval_long +. If the seconds field of a __timeval_long would + overflow, they write { INT32_MAX, 999999 } to the output. */ +static inline struct timeval +valid_timeval_long_to_timeval64 (const struct __timeval_long tv) +{ + return (struct timeval) { tv.tv_sec, tv.tv_usec }; +} + +static inline struct __timeval_long +valid_timeval64_to_timeval_long (const struct timeval tv64) +{ + if (__glibc_unlikely (tv64.tv_sec > (time_t) 2147483647)) + return (struct __timeval_long) { 2147483647, 999999}; + return (struct __timeval_long) { tv64.tv_sec, tv64.tv_usec }; +} + +static inline struct timespec +valid_timeval_long_to_timespec (const struct __timeval_long tv) +{ + return (struct timespec) { tv.tv_sec, tv.tv_usec * 1000 }; +} + +static inline struct __timeval_long +valid_timespec_to_timeval_long (const struct timespec ts) +{ + return (struct __timeval_long) { (time_t) ts.tv_sec, ts.tv_nsec / 1000 }; +} + /* Check if a value is in the valid nanoseconds range. Return true if it is, false otherwise. */ static inline bool diff --git a/time/bits/types/struct_timeval.h b/time/bits/types/struct_timeval.h index 70394ce886..73697689cc 100644 --- a/time/bits/types/struct_timeval.h +++ b/time/bits/types/struct_timeval.h @@ -10,4 +10,12 @@ struct timeval __time_t tv_sec; /* Seconds. */ __suseconds_t tv_usec; /* Microseconds. */ }; + +/* A version of 'struct timeval' with `long` time_t + and suseconds_t. */ +struct __timeval_long +{ + long tv_sec; /* Seconds. */ + long tv_usec; /* Microseconds. */ +}; #endif