Message ID | 20200203183153.11635-3-alistair.francis@wdc.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 24511 invoked by alias); 3 Feb 2020 18:38:44 -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 24314 invoked by uid 89); 3 Feb 2020 18:38:43 -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: esa3.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=1580755122; x=1612291122; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=kP4bNIW271AJtaXfqnO4HEqg4KTH7NFIKmsYZdN6iYM=; b=hQn3IJkZr/xM09v+zHm8LJT4+86wYgOyaO/rvin9jUg4fsUwSFXid5Ga /+fr3auzzn6x4J2CEUeV6p9iuBdaXZ3v0v15JJYTwEjHhpikcE0668v4d P8VU16S/4eA+dyKbzDCweIZjPzysaDLdl3FlSVE8Hy6FdBbz67+CqYLAo wbcGFZPqeOPbQGp9m9r/+epKdu8InzPRp3PbxAZVH8CtdEYI5nQ46pg/M pVE8Ef0FUzTUXru8X3zv0epsVMlv/Mz3ar1TCnq5w09mxS+2c1i/ywT91 sHp6Fjjko1bNwJ/Og1nFRyGOE2ULMXLEm5Kf2CBQC3IEpUKvPH5ZnN7Cq Q==; IronPort-SDR: 6XonrMSs/1WCGBv8ZyR20+QmI3eZKMnhRrG/4DgRqsuD6+nRxcknPxNUGTBkn8mnN5SgvSdzhm NqrLiGeAYDC4c/liAX4Sga87i5DOO3AF+D49jyHpgOLqchg8RZn4UlpGYMFZFNjqBoyZtbUSmY ZkdzbPtv0XY7ulDnMK4uY9HJc/p4gfGGKDMNXaA5koXsOJyQJMy2oIVG9JRmjEH+k6Wh3L0BX7 oVGj1fbkOsmVyqBzfOnAOoE9ZF/2tyRyh719PgOwDsmdGDig+GodaBA6rvuY8Ky8a3lVqNTkb/ L+s= IronPort-SDR: 1e/lJFf8NdAzjxgM6oiTiLXdngdAccSX0gwX9W1sSrqQv+hYAmbZ5cgtdRegXal5gfxCB9CCsI Luj+dBCJe9OVq0uChtnC60EJ+mIjwN0Bqbwr8rUyxPHUbUh9CY0ewQuChWm5geNnMD4Cw5NWdm bYxca23w2B1l4qiQJ328Obqf88DfGnW5MUBjVcQgOuDGHxfN7uwVt6MS79N/33plBIVFM1USiY ofdJ85rz5YupcRQVO8Vp5qBcg0nqiJWfQBr2APy5GQd8j2a0Izh6Xup5o3AeyFVwknpqxn4RXc AAM3yg+4FmBN4TuOezFfXFh7 IronPort-SDR: QKZZ7ML5Eyajjz/LES4PfU4ffS2IJAjZMu9BB6yxYTQzK8tWpNj1OLo3SQYHU7y5vrnjc0ek7q 8h+4jzQqsJHswnaXKeOl4XYCpR0UbB9Oi9h52qoJpKScvYtSIB51AZbpexePgpLTbCiIr9HKT7 Tdt7j4ecYRysYpqwXWNy6aRUxip5QaeJ+nVPOs4nYOBZATjMPDfWAugLFLbrLuqwtxqRpcD3CN VAsvOuvP6u45jnWHMy13LjBqUzfe6zRuGUWUTrvEGVlxEax0+GvX/7J1KCy9xBcV0j0KHem1cH gXc= WDCIronportException: Internal From: Alistair Francis <alistair.francis@wdc.com> To: libc-alpha@sourceware.org Cc: alistair23@gmail.com, lukma@denx.de, Alistair Francis <alistair.francis@wdc.com> Subject: [PATCH 2/6] time: Add a timeval with a long tv_sec and tv_usec Date: Mon, 3 Feb 2020 10:31:49 -0800 Message-Id: <20200203183153.11635-3-alistair.francis@wdc.com> In-Reply-To: <20200203183153.11635-1-alistair.francis@wdc.com> References: <20200203183153.11635-1-alistair.francis@wdc.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit |
Commit Message
Alistair Francis
Feb. 3, 2020, 6:31 p.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 | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+)
Comments
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 struct to be used internally. ^^^^^^^^^^ - I think that this shall be updated. > --- > include/time.h | 43 +++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 43 insertions(+) > > diff --git a/include/time.h b/include/time.h > index d425c69ede..c2c05bb671 100644 > --- a/include/time.h > +++ b/include/time.h > @@ -388,6 +388,49 @@ timespec64_to_timeval64 (const struct > __timespec64 ts64) return tv64; > } > > +/* A version of 'struct timeval' with `long` time_t > + and suseconds_t. */ > +struct __timeval32 > +{ > + long tv_sec; /* Seconds. */ As __timeval32 will be used in e.g __setitimer64 (which in turn will be aliased to setitimer on archs with __WORDSIZE==64 && __TIMESIZE==64) long seems to be the best option as it will be 64 bits for those machines. For ones with __WORDSIZE==32 it will be 32 bits instead. Am I correct? > + long tv_usec; /* Microseconds. */ > +}; > + > +/* Conversion functions for converting to/from __timeval32 > +. If the seconds field of a __timeval32 would > + overflow, they write { INT32_MAX, 999999 } to the output. */ > +static inline struct __timeval64 > +valid_timeval32_to_timeval64 (const struct __timeval32 tv) > +{ > + return (struct __timeval64) { tv.tv_sec, tv.tv_usec }; > +} > + > +static inline struct __timeval32 > +valid_timeval64_to_timeval32 (const struct __timeval64 tv64) > +{ > + if (__glibc_unlikely (tv64.tv_sec > (time_t) 2147483647)) > + return (struct __timeval32) { 2147483647, 999999}; What is the purpose of this code? The valid_* prefix shall indicate that the timeval64 will fit the timeval32 and there is no need for any explicit check. I would expect usage of this function as presented here: https://patchwork.ozlabs.org/patch/1230884/ if (! in_time_t_range (tv64.tv_sec)) { __set_errno (EOVERFLOW); return -1; } if (tv) *tv = valid_timeval64_to_timeval (tv64); > + return (struct __timeval32) { tv64.tv_sec, tv64.tv_usec }; > +} > + > +static inline struct timeval > +valid_timeval32_to_timeval (const struct __timeval32 tv) > +{ > + return (struct timeval) { tv.tv_sec, tv.tv_usec }; > +} > + > +static inline struct timespec > +valid_timeval32_to_timespec (const struct __timeval32 tv) > +{ > + return (struct timespec) { tv.tv_sec, tv.tv_usec * 1000 }; > +} > + > +static inline struct __timeval32 > +valid_timespec_to_timeval32 (const struct timespec ts) > +{ > + return (struct __timeval32) { (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 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, Feb 4, 2020 at 6:24 AM 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 struct to be used internally. > ^^^^^^^^^^ - I think that this shall be > updated. I'm not clear what you mean here, what should be changed? > > > --- > > include/time.h | 43 +++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 43 insertions(+) > > > > diff --git a/include/time.h b/include/time.h > > index d425c69ede..c2c05bb671 100644 > > --- a/include/time.h > > +++ b/include/time.h > > @@ -388,6 +388,49 @@ timespec64_to_timeval64 (const struct > > __timespec64 ts64) return tv64; > > } > > > > +/* A version of 'struct timeval' with `long` time_t > > + and suseconds_t. */ > > +struct __timeval32 > > +{ > > + long tv_sec; /* Seconds. */ > > As __timeval32 will be used in e.g __setitimer64 (which in turn will be > aliased to setitimer on archs with __WORDSIZE==64 && __TIMESIZE==64) > long seems to be the best option as it will be 64 bits for those > machines. Yep, that's the plan :) > > For ones with __WORDSIZE==32 it will be 32 bits instead. Am I correct? Yes. > > > + long tv_usec; /* Microseconds. */ > > +}; > > + > > +/* Conversion functions for converting to/from __timeval32 > > +. If the seconds field of a __timeval32 would > > + overflow, they write { INT32_MAX, 999999 } to the output. */ > > +static inline struct __timeval64 > > +valid_timeval32_to_timeval64 (const struct __timeval32 tv) > > +{ > > + return (struct __timeval64) { tv.tv_sec, tv.tv_usec }; > > +} > > + > > +static inline struct __timeval32 > > +valid_timeval64_to_timeval32 (const struct __timeval64 tv64) > > +{ > > + if (__glibc_unlikely (tv64.tv_sec > (time_t) 2147483647)) > > + return (struct __timeval32) { 2147483647, 999999}; > > What is the purpose of this code? > > The valid_* prefix shall indicate that the timeval64 will fit the > timeval32 and there is no need for any explicit check. Ah ok. I'll remove the check from here. > > I would expect usage of this function as presented here: > https://patchwork.ozlabs.org/patch/1230884/ > > if (! in_time_t_range (tv64.tv_sec)) > { > __set_errno (EOVERFLOW); > return -1; > } > > if (tv) > *tv = valid_timeval64_to_timeval (tv64); and I have added the check to here. Alistair > > > > + return (struct __timeval32) { tv64.tv_sec, tv64.tv_usec }; > > +} > > + > > +static inline struct timeval > > +valid_timeval32_to_timeval (const struct __timeval32 tv) > > +{ > > + return (struct timeval) { tv.tv_sec, tv.tv_usec }; > > +} > > + > > +static inline struct timespec > > +valid_timeval32_to_timespec (const struct __timeval32 tv) > > +{ > > + return (struct timespec) { tv.tv_sec, tv.tv_usec * 1000 }; > > +} > > + > > +static inline struct __timeval32 > > +valid_timespec_to_timeval32 (const struct timespec ts) > > +{ > > + return (struct __timeval32) { (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 > > > > > 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 Tue, Feb 4, 2020 at 6:24 AM 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 struct to be used > > > internally. > > ^^^^^^^^^^ - I think that this shall be > > updated. > > I'm not clear what you mean here, what should be changed? timeval_long -> timeval long ? > > > > > > --- > > > include/time.h | 43 +++++++++++++++++++++++++++++++++++++++++++ > > > 1 file changed, 43 insertions(+) > > > > > > diff --git a/include/time.h b/include/time.h > > > index d425c69ede..c2c05bb671 100644 > > > --- a/include/time.h > > > +++ b/include/time.h > > > @@ -388,6 +388,49 @@ timespec64_to_timeval64 (const struct > > > __timespec64 ts64) return tv64; > > > } > > > > > > +/* A version of 'struct timeval' with `long` time_t > > > + and suseconds_t. */ > > > +struct __timeval32 > > > +{ > > > + long tv_sec; /* Seconds. */ > > > > As __timeval32 will be used in e.g __setitimer64 (which in turn > > will be aliased to setitimer on archs with __WORDSIZE==64 && > > __TIMESIZE==64) long seems to be the best option as it will be 64 > > bits for those machines. > > Yep, that's the plan :) > > > > > For ones with __WORDSIZE==32 it will be 32 bits instead. Am I > > correct? > > Yes. > > > > > > + long tv_usec; /* Microseconds. */ > > > +}; > > > + > > > +/* Conversion functions for converting to/from __timeval32 > > > +. If the seconds field of a __timeval32 would > > > + overflow, they write { INT32_MAX, 999999 } to the output. */ > > > +static inline struct __timeval64 > > > +valid_timeval32_to_timeval64 (const struct __timeval32 tv) > > > +{ > > > + return (struct __timeval64) { tv.tv_sec, tv.tv_usec }; > > > +} > > > + > > > +static inline struct __timeval32 > > > +valid_timeval64_to_timeval32 (const struct __timeval64 tv64) > > > +{ > > > + if (__glibc_unlikely (tv64.tv_sec > (time_t) 2147483647)) > > > + return (struct __timeval32) { 2147483647, 999999}; > > > > What is the purpose of this code? > > > > The valid_* prefix shall indicate that the timeval64 will fit the > > timeval32 and there is no need for any explicit check. > > Ah ok. I'll remove the check from here. > > > > > I would expect usage of this function as presented here: > > https://patchwork.ozlabs.org/patch/1230884/ > > > > if (! in_time_t_range (tv64.tv_sec)) > > { > > __set_errno (EOVERFLOW); > > return -1; > > } > > > > if (tv) > > *tv = valid_timeval64_to_timeval (tv64); > > and I have added the check to here. Thanks :-) > > Alistair > > > > > > > > + return (struct __timeval32) { tv64.tv_sec, tv64.tv_usec }; > > > +} > > > + > > > +static inline struct timeval > > > +valid_timeval32_to_timeval (const struct __timeval32 tv) > > > +{ > > > + return (struct timeval) { tv.tv_sec, tv.tv_usec }; > > > +} > > > + > > > +static inline struct timespec > > > +valid_timeval32_to_timespec (const struct __timeval32 tv) > > > +{ > > > + return (struct timespec) { tv.tv_sec, tv.tv_usec * 1000 }; > > > +} > > > + > > > +static inline struct __timeval32 > > > +valid_timespec_to_timeval32 (const struct timespec ts) > > > +{ > > > + return (struct __timeval32) { (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 > > > > > > > > > > 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
On Thu, Feb 6, 2020 at 1:56 AM Lukasz Majewski <lukma@denx.de> wrote: > > Hi Alistair, > > > On Tue, Feb 4, 2020 at 6:24 AM 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 struct to be used > > > > internally. > > > ^^^^^^^^^^ - I think that this shall be > > > updated. > > > > I'm not clear what you mean here, what should be changed? > > timeval_long -> timeval long ? Ah! You are right. It should be __timeval32 Alistair > > > > > > > > > > --- > > > > include/time.h | 43 +++++++++++++++++++++++++++++++++++++++++++ > > > > 1 file changed, 43 insertions(+) > > > > > > > > diff --git a/include/time.h b/include/time.h > > > > index d425c69ede..c2c05bb671 100644 > > > > --- a/include/time.h > > > > +++ b/include/time.h > > > > @@ -388,6 +388,49 @@ timespec64_to_timeval64 (const struct > > > > __timespec64 ts64) return tv64; > > > > } > > > > > > > > +/* A version of 'struct timeval' with `long` time_t > > > > + and suseconds_t. */ > > > > +struct __timeval32 > > > > +{ > > > > + long tv_sec; /* Seconds. */ > > > > > > As __timeval32 will be used in e.g __setitimer64 (which in turn > > > will be aliased to setitimer on archs with __WORDSIZE==64 && > > > __TIMESIZE==64) long seems to be the best option as it will be 64 > > > bits for those machines. > > > > Yep, that's the plan :) > > > > > > > > For ones with __WORDSIZE==32 it will be 32 bits instead. Am I > > > correct? > > > > Yes. > > > > > > > > > + long tv_usec; /* Microseconds. */ > > > > +}; > > > > + > > > > +/* Conversion functions for converting to/from __timeval32 > > > > +. If the seconds field of a __timeval32 would > > > > + overflow, they write { INT32_MAX, 999999 } to the output. */ > > > > +static inline struct __timeval64 > > > > +valid_timeval32_to_timeval64 (const struct __timeval32 tv) > > > > +{ > > > > + return (struct __timeval64) { tv.tv_sec, tv.tv_usec }; > > > > +} > > > > + > > > > +static inline struct __timeval32 > > > > +valid_timeval64_to_timeval32 (const struct __timeval64 tv64) > > > > +{ > > > > + if (__glibc_unlikely (tv64.tv_sec > (time_t) 2147483647)) > > > > + return (struct __timeval32) { 2147483647, 999999}; > > > > > > What is the purpose of this code? > > > > > > The valid_* prefix shall indicate that the timeval64 will fit the > > > timeval32 and there is no need for any explicit check. > > > > Ah ok. I'll remove the check from here. > > > > > > > > I would expect usage of this function as presented here: > > > https://patchwork.ozlabs.org/patch/1230884/ > > > > > > if (! in_time_t_range (tv64.tv_sec)) > > > { > > > __set_errno (EOVERFLOW); > > > return -1; > > > } > > > > > > if (tv) > > > *tv = valid_timeval64_to_timeval (tv64); > > > > and I have added the check to here. > > Thanks :-) > > > > > Alistair > > > > > > > > > > > > + return (struct __timeval32) { tv64.tv_sec, tv64.tv_usec }; > > > > +} > > > > + > > > > +static inline struct timeval > > > > +valid_timeval32_to_timeval (const struct __timeval32 tv) > > > > +{ > > > > + return (struct timeval) { tv.tv_sec, tv.tv_usec }; > > > > +} > > > > + > > > > +static inline struct timespec > > > > +valid_timeval32_to_timespec (const struct __timeval32 tv) > > > > +{ > > > > + return (struct timespec) { tv.tv_sec, tv.tv_usec * 1000 }; > > > > +} > > > > + > > > > +static inline struct __timeval32 > > > > +valid_timespec_to_timeval32 (const struct timespec ts) > > > > +{ > > > > + return (struct __timeval32) { (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 > > > > > > > > > > > > > > > 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 d425c69ede..c2c05bb671 100644 --- a/include/time.h +++ b/include/time.h @@ -388,6 +388,49 @@ timespec64_to_timeval64 (const struct __timespec64 ts64) return tv64; } +/* A version of 'struct timeval' with `long` time_t + and suseconds_t. */ +struct __timeval32 +{ + long tv_sec; /* Seconds. */ + long tv_usec; /* Microseconds. */ +}; + +/* Conversion functions for converting to/from __timeval32 +. If the seconds field of a __timeval32 would + overflow, they write { INT32_MAX, 999999 } to the output. */ +static inline struct __timeval64 +valid_timeval32_to_timeval64 (const struct __timeval32 tv) +{ + return (struct __timeval64) { tv.tv_sec, tv.tv_usec }; +} + +static inline struct __timeval32 +valid_timeval64_to_timeval32 (const struct __timeval64 tv64) +{ + if (__glibc_unlikely (tv64.tv_sec > (time_t) 2147483647)) + return (struct __timeval32) { 2147483647, 999999}; + return (struct __timeval32) { tv64.tv_sec, tv64.tv_usec }; +} + +static inline struct timeval +valid_timeval32_to_timeval (const struct __timeval32 tv) +{ + return (struct timeval) { tv.tv_sec, tv.tv_usec }; +} + +static inline struct timespec +valid_timeval32_to_timespec (const struct __timeval32 tv) +{ + return (struct timespec) { tv.tv_sec, tv.tv_usec * 1000 }; +} + +static inline struct __timeval32 +valid_timespec_to_timeval32 (const struct timespec ts) +{ + return (struct __timeval32) { (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