Message ID | 20191018145720.11706-1-lukma@denx.de |
---|---|
State | New |
Headers | show |
Series | [1/2] y2038: Helper macro to convert struct __timespec64 to struct timespec | expand |
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
diff --git a/include/time.h b/include/time.h index d93b16a781..c2b6c9b842 100644 --- a/include/time.h +++ b/include/time.h @@ -236,5 +236,28 @@ valid_timespec64_to_timeval (const struct __timespec64 ts64) return tv; } + +/* Check if a value lies with the valid nanoseconds range. */ +#define IS_VALID_NANOSECONDS(ns) ((ns) >= 0 && (ns) <= 999999999) + +/* Check and convert a struct __timespec64 into a struct timespec. + This macro checks if the valid number of nanoseconds has been provided + by ts64 and if not the errno is set to EINVAL and -1 is returned. + Moreover, the number of seconds is check as well, if it is in the time_t + range. If not the errno is set to EOVERFLOW and -1 is returned. */ +#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); }) + #endif #endif