bug#9101: timeout should use setitimer if available

[Paul Eggert]

On 07/19/11 04:00, Pádraig Brady wrote:

> +  if (timer_create (CLOCK_REALTIME, NULL, &timerid) == -1)
> +    error (EXIT_FAILURE, errno, _("error in timer_create"));
> +  if (timer_settime (timerid, 0, &its, NULL) == -1)
> +    error (EXIT_FAILURE, errno, _("error in timer_settime"));

A minor point: the usual (more-conservative, and often-faster) style
in coreutils is to write "foo (...) != 0" rather than "foo (...) ==
-1" for system calls that return 0 or -1.

> We could remove the setitimer stuff altogether and
> just support 1 second resolution on darwin et. al.
> That's by far the most common use case anyway.

That sounds good to me.

Another thought, to make the code more bulletproof, is to fall back on
alarm if timer_create and timer_settime fail due to ENOTSUP or EAGAIN
or whatever.

Something like this (untested) code:

/* Start the timeout after which we'll receive a SIGALRM.
   Round DURATION up to the next representable value.
   Treat out-of-range values as if they were maximal,
   as that's more useful in practice than reporting an error.
   '0' means don't timeout.  */
static void
settimeout (double duration)
{
#if HAVE_TIMER_SETTIME
  /* Prefer timer_settime if it works, as it's higher-resolution.  */
  struct timespec ts = dtotimespec (duration);
  struct itimerspec its = { {0, 0}, ts };
  timer_t timerid;
  if (timer_create (CLOCK_REALTIME, NULL, &timerid) == 0)
    {
      if (timer_settime (timerid, 0, &its, NULL) == 0)
        return;
      timer_delete (timerid);
    }
#endif

  if (UINT_MAX <= duration)
    alarm (UINT_MAX);
  else
    {
      unsigned int duration_floor = duration;
      alarm (duration_floor + (duration_floor < duration));
    }
}



> The gnulib check could be lumped into clock_time, like:

Yes, that would work, but the clock-time module probably should stay
decoupled from timer_settime.  How about the following (untested)
patch instead?  The idea is to append "timeout_LDADD +=
$(LIB_TIMER_SETTIME)" to src/Makefile.am.

diff --git a/m4/jm-macros.m4 b/m4/jm-macros.m4
index 9bb6fa4..c32cbd3 100644
--- a/m4/jm-macros.m4
+++ b/m4/jm-macros.m4
@@ -43,6 +43,11 @@ AC_DEFUN([coreutils_MACROS],
 
   # used by ls
   AC_REQUIRE([gl_CLOCK_TIME])
+  coreutils_lib_pattern='-l\(.*\)'
+  coreutils_lib_rt=`
+    expr "X$ac_cv_search_clock_gettime" : "X$coreutils_lib_pattern"
+  `
+
   # used by shred
   AC_CHECK_FUNCS_ONCE([directio])
 
@@ -97,13 +102,24 @@ AC_DEFUN([coreutils_MACROS],
   # for dd.c and shred.c
   coreutils_saved_libs=$LIBS
     LIB_FDATASYNC=
-    AC_SEARCH_LIBS([fdatasync], [rt posix4],
+    AC_SEARCH_LIBS([fdatasync], [$coreutils_lib_rt],
                    [test "$ac_cv_search_fdatasync" = "none required" ||
                     LIB_FDATASYNC=$ac_cv_search_fdatasync])
     AC_SUBST([LIB_FDATASYNC])
     AC_CHECK_FUNCS([fdatasync])
   LIBS=$coreutils_saved_libs
 
+  # used by timeout.c
+  AC_SUBST([LIB_TIMER_SETTIME])
+  coreutils_saved_libs=$LIBS
+    LIB_TIMER_SETTIME=
+    AC_SEARCH_LIBS([timer_settime], [$coreutils_lib_rt],
+                   [test "$ac_cv_search_timer_settime" = "none required" ||
+                    LIB_TIMER_SETTIME=$ac_cv_search_timer_settime])
+    AC_SUBST([LIB_TIMER_SETTIME])
+    AC_CHECK_FUNCS([timer_settim])
+  LIBS=$coreutils_saved_libs
+
   # Check whether libcap is usable -- for ls --color support
   LIB_CAP=
   AC_ARG_ENABLE([libcap],