Re: [RFC PATCH 51/52] Y2038: add RPC functions

[Paul Eggert <eggert at cs dot ucla dot edu>]

On 09/08/2017 10:49 AM, Albert ARIBAUD (3ADEV) wrote:
> +CLIENT *
> +__clntudp_create64 (struct sockaddr_in *raddr, u_long program, u_long version,
> +		    struct __timeval64 wait, int *sockp)
> +{
> +  struct timeval wait32;
> +  if (wait.tv_sec > INT_MAX)
> +  {
> +    return NULL;
> +  }
> +  return clntudp_create (raddr, program, version, wait32, sockp);
> +}

I'm not seeing how this could work. The code does not copy the value of 
'wait' to 'wait32'. And the code doesn't have a proper check for fitting 
in 'int', e.g., it will do the wrong thing for INT_MIN - 1L. And there's 
no error status set when the time is out of range.

I haven't reviewed the patches carefully, just caught this in a spot 
check. Please look systematically for similar errors.

While you're doing that systematic review, I suggest putting something 
like the following code into a suitable private include file, and using 
it to tell whether a __time64_t value is in time_t range. This will 
generate zero instructions when time_t is 64-bit, so generic callers can 
use this function without needing any ifdefs and without losing any 
performance on 64-bit time_t platforms. You should write similar static 
functions for checking whether struct __timeval64 is in struct timeval 
range, and similarly for struct __timespec64. These can check for the 
subseconds parts being in range, as needed (watch for x32 here). The 
idea is to be systematic about this stuff and to do it in one place, to 
avoid ticky-tack range bugs such as are in the above-quoted code.

  /* time_t is always 'long int' in the GNU C Library.  */
  #define TIME_T_MIN LONG_MIN
  #define TIME_T_MAX LONG_MAX

  static inline bool
  fits_in_time_t (__time64_t t)
  {
  #if 7 <= __GNUC__
    return !__builtin_add_overflow_p (t, 0, TIME_T_MAX);
  #endif
    return TIME_T_MIN <= t && t <= TIME_T_MAX;
  }