Re: Bug in nanosleep() implementation for Unix platforms lacking same

[Pádraig Brady]

On 18/09/15 04:27, Daniel Richard G. wrote:
> Hello list,
> 
> Lately I am testing and enhancing Gnulib on a relatively exotic
> POSIX platform.
> 
> This platform lacks nanosleep(), and so uses the implementation
> starting at lib/nanosleep.c:227 (Git master). Investigating a failure
> in test-nanosleep, I found that the following assertion...
> 
>     ASSERT (nanosleep (&ts, &ts) == -1);
> 
> ...tripped because the function returned 0, despite having been
> interrupted by a SIGALRM. This appeared to occur not because of a
> platform issue, but due to the implementation itself. I was able to
> reproduce this failure on Linux after forcing Gnulib to use that same
> implementation.
> 
> In nanosleep.c, the sighandler() function is never called (is the
> program supposed to receive SIGCONT when some other signal interrupts
> select()?),

I think the SIGCONT handling is to handle reception of
explicit SIGSTOP and SIGCONT

> and even if it did, the nanosleep() implementation would
> then return 1 instead of the correct value of -1.

Yes that looks incorrect.
Perhaps something like this suffices:

diff --git a/lib/nanosleep.c b/lib/nanosleep.c
index 3060db0..1364f7a 100644
--- a/lib/nanosleep.c
+++ b/lib/nanosleep.c
@@ -202,7 +202,7 @@ sighandler (int sig)

 /* Suspend execution for at least *TS_DELAY seconds.  */

-static void
+static int
 my_usleep (const struct timespec *ts_delay)
 {
   struct timeval tv_delay;
@@ -218,7 +218,7 @@ my_usleep (const struct timespec *ts_delay)
           tv_delay.tv_usec = 0;
         }
     }
-  select (0, NULL, NULL, NULL, &tv_delay);
+  return select (0, NULL, NULL, NULL, &tv_delay);
 }

 /* Suspend execution for at least *REQUESTED_DELAY seconds.  The
@@ -256,19 +256,21 @@ nanosleep (const struct timespec *requested_delay,

   suspended = 0;

-  my_usleep (requested_delay);
-
-  if (suspended)
+  if (my_usleep (requested_delay) == -1)
     {
-      /* Calculate time remaining.  */
-      /* FIXME: the code in sleep doesn't use this, so there's no
-         rush to implement it.  */
+      if (suspended)
+        {
+          /* Calculate time remaining.  */
+          /* FIXME: the code in sleep doesn't use this, so there's no
+             rush to implement it.  */

-      errno = EINTR;
+          errno = EINTR;
+        }
+      return -1;
     }

   /* FIXME: Restore sig handler?  */

-  return suspended;
+  return 0;
 }
 #endif