Message ID | 20200203183153.11635-3-alistair.francis@wdc.com |
---|---|
State | New |
Headers | show |
Series | Always use 32-bit time_t for certain syscalls | expand |
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