diff mbox

[COMMITTED] Clarify comments in Linux times() implementation.

Message ID 0290c1d5-bbdd-4aca-8750-9399f559d156@redhat.com
State New
Headers show

Commit Message

Carlos O'Donell June 19, 2016, 7:45 p.m. UTC
The comments in __times talk about -1, but the reality of the
situation is that we only care about EFAULT for the faulting
case and (clock_t) -1 when we talk about POSIX conformance and
error returns. I have expanded the comments to cover both cases
clearly.

Checked in.

Comments

Andreas Schwab June 20, 2016, 7:32 a.m. UTC | #1
Carlos O'Donell <carlos@redhat.com> writes:

> +  /* On Linux this function never fails except with EFAULT.
> +     POSIX says that returning a value (clock_t) -1 indicates an error,
> +     but on Linux this is simply one of the valid clock values after
> +     clock_t wraps.  Therefore when we would return (clock_t) -1, we
> +     instead return (clock_t) 0, and loose a tick of accuracy (having

s/loose/lose/

Andreas.
diff mbox

Patch

diff --git a/sysdeps/unix/sysv/linux/times.c b/sysdeps/unix/sysv/linux/times.c
index 8f3033b..f96b654 100644
--- a/sysdeps/unix/sysv/linux/times.c
+++ b/sysdeps/unix/sysv/linux/times.c
@@ -29,11 +29,13 @@  __times (struct tms *buf)
       && __builtin_expect (INTERNAL_SYSCALL_ERRNO (ret, err) == EFAULT, 0)
       && buf)
     {
-      /* This might be an error or not.  For architectures which have
-	 no separate return value and error indicators we cannot
-	 distinguish a return value of -1 from an error.  Do it the
-	 hard way.  We crash applications which pass in an invalid
-	 non-NULL BUF pointer.  Linux allows BUF to be NULL. */
+      /* This might be an error or not.  For architectures which have no
+	 separate return value and error indicators we cannot
+	 distinguish a return value of e.g. (clock_t) -14 from -EFAULT.
+	 Therefore the only course of action is to dereference the user
+	 -supplied structure on a return of (clock_t) -14.  This will crash
+	 applications which pass in an invalid non-NULL BUF pointer.
+	 Note that Linux allows BUF to be NULL in which case we skip this.  */
 #define touch(v) \
       do {								      \
 	clock_t temp = v;						      \
@@ -45,13 +47,18 @@  __times (struct tms *buf)
       touch (buf->tms_cutime);
       touch (buf->tms_cstime);
 
-      /* If we come here the memory is valid (or BUF is NULL, which is
-         a valid condition for the kernel syscall) and the kernel did not
-	 return an EFAULT error.  Return the value given by the kernel.  */
+      /* If we come here the memory is valid and the kernel did not
+	 return an EFAULT error, but rather e.g. (clock_t) -14.
+	 Return the value given by the kernel.  */
     }
 
-  /* Return value (clock_t) -1 signals an error, but if there wasn't any,
-     return the following value.  */
+  /* On Linux this function never fails except with EFAULT.
+     POSIX says that returning a value (clock_t) -1 indicates an error,
+     but on Linux this is simply one of the valid clock values after
+     clock_t wraps.  Therefore when we would return (clock_t) -1, we
+     instead return (clock_t) 0, and loose a tick of accuracy (having
+     returned 0 for two consecutive calls even though the clock
+     advanced).  */
   if (ret == (clock_t) -1)
     return (clock_t) 0;
---