Re: gettimeofday() for Win32

[Paul Eggert]

Thanks for bird-dogging this.  On further review I have some comments
and suggestions.

The gettimeofday replacement should use the standard POSIX signature,
which uses 'restrict'.  See
<http://www.opengroup.org/onlinepubs/009695399/functions/gettimeofday.html>.

> +#ifdef _WIN32
> +#include <sys/timeb.h>
> +#endif

This should be '#if HAVE_SYS_TIMEB_H', and gettimeofday.m4 should
check for sys/timeb.h (in the PREREQ area).

> +#ifndef HAVE_GETTIMEOFDAY
> +int
> +gettimeofday (struct timeval *tp, void *tzp)
> +{
> +#ifdef _WIN32

Similarly, this should be '#if HAVE__FTIME', and gettimeofday.m4
should check for _ftime.

> +  struct _timeb timebuf;
> +
> +  _ftime (&timebuf);
> +  tp->tv_sec = timebuf.time;
> +  tp->tv_usec = (long)(timebuf.millitm) * (1000000/1000);

Just write this:

  tp->tv_usec = timebuf.millitm * 1000;

GNU code assumes that int is at least 32 bits, so this is safe.

> +#else
> +  tp->tv_sec = time (NULL);
> +  tp->tv_usec = 0;
> +#endif
> +
> +  return 0;
> +}
> +#endif /* HAVE_GETTIMEOFDAY */


This should check for the failure of time.  E.g.:

  time_t t = time (NULL);
  if (t == (time_t) -1)
    return -1;


> +#ifdef GETTIMEOFDAY_USE_REPLACEMENTS

There are two replacements for gettimeofday, and this is confusing.
There should be just one.  It can do all the things necessary for all
the portability problems with gettimeofday: the Windows port, the
copying of struct tm, the fixing of the signature for older hosts
lacking 'restrict', and whatever else comes up.

> +#ifndef HAVE_GETTIMEOFDAY
> +int gettimeofday (struct timeval *tp, void *tzp);
> +#endif

Some older Unix systems define gettimeofday with 1 arg in their
standard headers.  So it'd be better to do this:

#ifndef HAVE_GETTIMEOFDAY_POSIX_SIGNATURE
# undef gettimeofday
# define gettimeofday rpl_gettimeofday
int gettimeofday (struct timeval *restrict, void *restrict);
#endif

where HAVE_GETTIMEOFDAY_POSIX_SIGNATURE checks that we have a gettimeofday that
seems to conform to posix.  We can then remove the rpl_gettimeofday
from config.h; we don't want it in config.h because we don't want the
gettimeofday in sys/time.h to be rpl_'ed.

> +AC_DEFUN([gl_FUNC_GETTIMEOFDAY],
> +[
> +  AC_LIBSOURCES([gettimeofday.c, gettimeofday.h])
> +  gl_PREREQ_GETTIMEOFDAY
> +  AC_REPLACE_FUNCS(gettimeofday)
> +  if test $ac_cv_func_gettimeofday = yes; then
> +    AC_FUNC_GETTIMEOFDAY_CLOBBER
> +  fi
> +])

This doesn't seem right to me.  Generally, gl_PREREQ_GETTIMEOFDAY is
invoked only if we know we need to compile gettimeofday.c.  But on
most hosts we don't need to compile it.  Better would be
something like this (this is only a sketch):

  AC_LIBSOURCES([gettimeofday.c, gettimeofday.h])
  AC_REQUIRE([gl_C_RESTRICT])
  AC_CACHE_CHECK([for gettimeofday whose signature conforms to POSIX],
    [ac_cv_func_gettimeofday_posix_signature = no
     AC_LINK_IFELSE(
       [AC_LANG_PROGRAM(
          [[#include <sys/time.h>
            time_t a;
            suseconds_t b;
            struct timeval c;
          ]],
          [[
            int x = gettimeofday (&c, 0);
            int (*f) (struct timeval *restrict, void *restrict) = gettimeofday;
            return !(x | c.tv_sec | tv_usec);
          ]])],
       [ac_cv_func_gettimeofday_posix_signature = yes])])
  if test $ac_cv_func_gettimeofday_posix_signature = yes; then
    AC_DEFINE([HAVE_GETTIMEOFDAY_POSIX_SIGNATURE], 1,
      [Define if gettimeofday's signature conforms to POSIX.])
    AC_FUNC_GETTIMEOFDAY_CLOBBER
  fi
    if test $ac_cv_func_gettimeofday_posix_signature != yes; then
    gl_PREREQ_GETTIMEOFDAY
    AC_LIBOBJ([gettimeofday])
  fi

where this code is removed from AC_FUNC_GETTIMEOFDAY_CLOBBER:

  if test $jm_cv_func_gettimeofday_clobber = yes; then
    gl_GETTIMEOFDAY_REPLACE_LOCALTIME

    AC_DEFINE(gettimeofday, rpl_gettimeofday,
      [Define to rpl_gettimeofday if the replacement function should be used.])
  fi

>  AC_DEFUN([gl_GETTIMEOFDAY_REPLACE_LOCALTIME], [
> +  AC_DEFINE(GETTIMEOFDAY_USE_REPLACEMENTS, 1,
> +    [Define if some time related replacement functions should be used.])

I'd rename this macro from GETTIMEOFDAY_USE_REPLACEMENTS to
GETTIMEOFDAY_CLOBBERS_LOCALTIME or something like that, since that's
what it really means.