[1/2] y2038: Helper macro to convert struct __timespec64 to struct timespec

  This macro allows conversion between Y2038 safe struct __timespec64 and
struct timespec.
The struct __timespec64's tv_nsec field is checked if it is in the correct
range.
Moreover, the tv_sec is asserted if it fits into the time_t range.
When error is detected the errno is set accordingly and the function, which
uses this macro returns -1.

Tested with scripts/build-many-glibcs.py script.
---
 include/time.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
  

On 10/18/19 7:57 AM, Lukasz Majewski wrote:

> +/* Check if a value lies with the valid nanoseconds range.  */
> +#define IS_VALID_NANOSECONDS(ns) ((ns) >= 0 && (ns) <= 999999999)

This should be a static or inline function; there's no need for the 
excessive power of a macro here.

> +#define timespec64_to_timespec(ts64)                                           \
> +  ({                                                                           \
> +    if (! IS_VALID_NANOSECONDS (ts64.tv_nsec))                                 \
> +      {                                                                        \
> +        __set_errno (EINVAL);                                                  \
> +        return -1;                                                             \
> +      }                                                                        \
> +    if (! in_time_t_range (ts64.tv_sec))                                       \
> +      {                                                                        \
> +        __set_errno (EOVERFLOW);                                               \
> +        return -1;                                                             \
> +      }                                                                        \
> +    valid_timespec64_to_timespec (ts64); })

This macro is too confusing. Instead, if there's a need for this sort of 
thing, I suggest a static or inline function that returns true or false 
(setting errno); the caller can decide what to do if it returns false.
  

Hi Paul,

> On 10/18/19 7:57 AM, Lukasz Majewski wrote:
> 
> > +/* Check if a value lies with the valid nanoseconds range.  */
> > +#define IS_VALID_NANOSECONDS(ns) ((ns) >= 0 && (ns) <= 999999999)  
> 
> This should be a static or inline function; there's no need for the 
> excessive power of a macro here.

Ok. I can add this as an inline static function.

> 
> > +#define timespec64_to_timespec(ts64)
> >             \
> > +  ({
> >             \
> > +    if (! IS_VALID_NANOSECONDS (ts64.tv_nsec))
> >             \
> > +      {
> >             \
> > +        __set_errno (EINVAL);
> >             \
> > +        return -1;
> >             \
> > +      }
> >             \
> > +    if (! in_time_t_range (ts64.tv_sec))
> >             \
> > +      {
> >             \
> > +        __set_errno (EOVERFLOW);
> >             \
> > +        return -1;
> >             \
> > +      }
> >             \
> > +    valid_timespec64_to_timespec (ts64); })  
> 
> This macro is too confusing. Instead, if there's a need for this sort
> of thing, I suggest a static or inline function that returns true or
> false (setting errno);

My first attempt on this conversion helper was based on static inline
function. Unfortunately, such approach has the issue with __set_errno(),
which is not accessible in include/time.h (as it is defined in
include/errno.h).

Moreover, in the glibc the pattern with defining such macros is widely
used - in e.g. math/math-narrow.h or in various sysdep.h headers.

Last but not least - as Joseph pointed out in the other mail - maybe it
would be just enough in this particular case to drop the
IS_VALID_NANOSECONDS() check as this shall be done in the kernel (and
if an error is detected the syscall would fail).
The in_time_t_range() check for clock_getres is also problematic - as
it would be required to have a _really_ bad clock with tv_sec to be
overflowed.

To sum up - for the clock_getres() conversion - I do opt for using
valid_timespec64_to_timespec() (as it is already available in
include/time.h) and drop this patch (but keeping the
IS_VALID_NANOSECONDS() check in the form of static inline).

> the caller can decide what to do if it returns
> false.




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