Update.
authorUlrich Drepper <drepper@redhat.com>
Sat, 29 Jun 2002 23:35:02 +0000 (23:35 +0000)
committerUlrich Drepper <drepper@redhat.com>
Sat, 29 Jun 2002 23:35:02 +0000 (23:35 +0000)
2002-06-18  Amos Waterland  <apw@us.ibm.com>

* sysdeps/pthread/aio_cancel.c (aio_cancel): Add check for invalid
file descriptor.
* sysdeps/pthread/aio_fsync.c (aio_fsync): Add check for invalid fd;
add check for fd not open for writing.

* sysdeps/pthread/aio_suspend.c (aio_suspend): Add check for
completed element(s) and do not suspend thread if so.  Patch
heavily modified by drepper.

* rt/tst-aio7.c: New file.  Regression test for problems which the
above three changes fix.
* rt/Makefile (tests): Add tst-aio7.

* rt/tst-aio6.c: Fix comment.

ChangeLog
rt/Makefile
rt/tst-aio6.c
rt/tst-aio7.c [new file with mode: 0644]
sysdeps/pthread/aio_cancel.c
sysdeps/pthread/aio_fsync.c
sysdeps/pthread/aio_suspend.c

index 98b2695..9c6f148 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,22 @@
+2002-06-18  Amos Waterland  <apw@us.ibm.com>
+
+       * sysdeps/pthread/aio_cancel.c (aio_cancel): Add check for invalid
+       file descriptor.
+       * sysdeps/pthread/aio_fsync.c (aio_fsync): Add check for invalid fd;
+       add check for fd not open for writing.
+
+       * sysdeps/pthread/aio_suspend.c (aio_suspend): Add check for
+       completed element(s) and do not suspend thread if so.  Patch
+       heavily modified by drepper.
+
+       * rt/tst-aio7.c: New file.  Regression test for problems which the
+       above three changes fix.
+       * rt/Makefile (tests): Add tst-aio7.
+
 2002-06-29  Ulrich Drepper  <drepper@redhat.com>
 
+       * rt/tst-aio6.c: Fix comment.
+
        * catgets/gencat.c (read_input_file): Handle more than one slash
        at end of line correctly [PR libc/3926].
        Based on a patch by Steven Kim <steven.kim@peregrine.com>.
index 47ce1fb..5ac4dd4 100644 (file)
@@ -1,4 +1,4 @@
-# Copyright (C) 1997,98,99,2000,01 Free Software Foundation, Inc.
+# Copyright (C) 1997-2001, 2002 Free Software Foundation, Inc.
 # This file is part of the GNU C Library.
 
 # The GNU C Library is free software; you can redistribute it and/or
@@ -39,7 +39,8 @@ librt-routines = $(aio-routines) \
                 $(shm-routines)
 
 tests := tst-shm tst-clock tst-timer \
-        tst-aio tst-aio64 tst-aio2 tst-aio3 tst-aio4 tst-aio5 tst-aio6
+        tst-aio tst-aio64 tst-aio2 tst-aio3 tst-aio4 tst-aio5 tst-aio6 \
+        tst-aio7
 
 extra-libs := librt
 extra-libs-others := $(extra-libs)
index 58695f5..b2da942 100644 (file)
@@ -1,5 +1,5 @@
 /* Test for timeout handling.
-   Copyright (C) 2000 Free Software Foundation, Inc.
+   Copyright (C) 2000, 2002 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -67,7 +67,7 @@ do_test (void)
   /* Get the current time.  */
   gettimeofday (&before, NULL);
 
-  /* Wait for input which is unsuccess and therefore the function will
+  /* Wait for input which is unsuccessful and therefore the function will
      time out.  */
   timeout.tv_sec = 3;
   timeout.tv_nsec = 0;
diff --git a/rt/tst-aio7.c b/rt/tst-aio7.c
new file mode 100644 (file)
index 0000000..1b3bdea
--- /dev/null
@@ -0,0 +1,175 @@
+/* Test for AIO POSIX compliance.
+   Copyright (C) 2001 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, write to the Free
+   Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
+   02111-1307 USA.  */
+
+#include <aio.h>
+#include <error.h>
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+
+
+/* We might wait for 3 seconds, so increase timeout to 10 seconds.  */
+#define TIMEOUT 10
+
+
+#define TEST_FUNCTION do_test ()
+static int
+do_test (void)
+{
+  int result = 0;
+
+  /* Test for aio_cancel() detecting invalid file descriptor.  */
+  {
+    struct aiocb cb;
+    int fd = -1;
+
+    cb.aio_fildes = fd;
+    cb.aio_offset = 0;
+    cb.aio_buf = NULL;
+    cb.aio_nbytes = 0;
+    cb.aio_reqprio = 0;
+    cb.aio_sigevent.sigev_notify = SIGEV_NONE;
+
+    errno = 0;
+
+    /* Case one: invalid fds that match.  */
+    if (aio_cancel (fd, &cb) != -1 || errno != EBADF)
+      {
+       puts ("aio_cancel( -1, {-1..} ) did not return -1 or errno != EBADF");
+       ++result;
+      }
+
+    cb.aio_fildes = -2;
+    errno = 0;
+
+    /* Case two: invalid fds that do not match; just print warning.  */
+    if (aio_cancel (fd, &cb) != -1 || errno != EBADF)
+      puts ("aio_cancel( -1, {-2..} ) did not return -1 or errno != EBADF");
+  }
+
+  /* Test for aio_fsync() detecting bad fd, and fd not open for writing.  */
+  {
+    struct aiocb cb;
+    int fd = -1;
+
+    cb.aio_fildes = fd;
+    cb.aio_offset = 0;
+    cb.aio_buf = NULL;
+    cb.aio_nbytes = 0;
+    cb.aio_reqprio = 0;
+    cb.aio_sigevent.sigev_notify = SIGEV_NONE;
+
+    errno = 0;
+
+    /* Case one: invalid fd.  */
+    if (aio_fsync (O_SYNC, &cb) != -1 || errno != EBADF)
+      {
+       puts ("aio_fsync( op, {-1..} ) did not return -1 or errno != EBADF");
+       ++result;
+      }
+
+    if ((fd = open ("/dev/null", O_RDONLY)) < 0)
+      error (1, errno, "opening /dev/null");
+
+    cb.aio_fildes = fd;
+    errno = 0;
+
+    /* Case two: valid fd but open for read only.  */
+    if (aio_fsync (O_SYNC, &cb) != -1 || errno != EBADF)
+      {
+       puts ("aio_fsync( op, {RO..} ) did not return -1 or errno != EBADF");
+       ++result;
+      }
+
+    close (fd);
+  }
+
+  /* Test for aio_suspend() suspending even if completed elements in list.  */
+  {
+    const int BYTES = 8, ELEMS = 2;
+    int i, r, fd;
+    char buff[BYTES];
+    char name[] = "/tmp/aio7.XXXXXX";
+    struct timespec timeout;
+    struct aiocb cb0, cb1;
+    struct aiocb *list[ELEMS];
+
+    fd = mkstemp (name);
+    if (fd < 0)
+      error (1, errno, "creating temp file");
+
+    if (unlink (name))
+      error (1, errno, "unlinking temp file");
+
+    if (write (fd, "01234567", BYTES) != BYTES)
+      error (1, errno, "writing to temp file");
+
+    cb0.aio_fildes = fd;
+    cb0.aio_offset = 0;
+    cb0.aio_buf = buff;
+    cb0.aio_nbytes = BYTES;
+    cb0.aio_reqprio = 0;
+    cb0.aio_sigevent.sigev_notify = SIGEV_NONE;
+
+    r = aio_read (&cb0);
+    if (r != 0)
+      error (1, errno, "reading from file");
+
+    while (aio_error (&(cb0)) == EINPROGRESS)
+      usleep (10);
+
+    for (i = 0; i < BYTES; i++)
+      printf ("%c ", buff[i]);
+    printf ("\n");
+
+    /* At this point, the first read is completed, so start another one on
+     * stdin, which will not complete unless the user inputs something.
+     */
+    cb1.aio_fildes = 0;
+    cb1.aio_offset = 0;
+    cb1.aio_buf = buff;
+    cb1.aio_nbytes = BYTES;
+    cb1.aio_reqprio = 0;
+    cb1.aio_sigevent.sigev_notify = SIGEV_NONE;
+
+    r = aio_read (&cb1);
+    if (r != 0)
+      error (1, errno, "reading from file");
+
+    /* Now call aio_suspend() with the two reads.  It should return
+     * immediately according to the POSIX spec.
+     */
+    list[0] = &cb0;
+    list[1] = &cb1;
+    timeout.tv_sec = 3;
+    timeout.tv_nsec = 0;
+    r = aio_suspend ((const struct aiocb * const *) list, ELEMS, &timeout);
+
+    if (r == -1 && errno == EAGAIN)
+      {
+       puts ("aio_suspend([done,blocked],2,3) suspended thread");
+       ++result;
+      }
+  }
+
+  return result;
+}
+
+#include "../test-skeleton.c"
index c2fe6da..d62586b 100644 (file)
@@ -1,5 +1,5 @@
 /* Cancel requests associated with given file descriptor.
-   Copyright (C) 1997, 1998, 2000 Free Software Foundation, Inc.
+   Copyright (C) 1997, 1998, 2000, 2002 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@cygnus.com>, 1997.
 
@@ -43,6 +43,13 @@ aio_cancel (fildes, aiocbp)
   struct requestlist *req = NULL;
   int result = AIO_ALLDONE;
 
+  /* If fildes is invalid, error. */
+  if (fcntl (fildes, F_GETFL) < 0)
+    {
+      __set_errno (EBADF);
+      return -1;
+    }
+
   /* Request the mutex.  */
   pthread_mutex_lock (&__aio_requests_mutex);
 
index 17f4351..4c90d69 100644 (file)
@@ -1,5 +1,5 @@
 /* Synchronize I/O in given file descriptor.
-   Copyright (C) 1997, 1999 Free Software Foundation, Inc.
+   Copyright (C) 1997, 1999, 2002 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@cygnus.com>, 1997.
 
 int
 aio_fsync (int op, struct aiocb *aiocbp)
 {
-  if (op != O_DSYNC && op != O_SYNC)
+  int flags;
+
+  if (op != O_DSYNC && __builtin_expect (op != O_SYNC, 0))
     {
       __set_errno (EINVAL);
       return -1;
     }
 
+  flags = fcntl (aiocbp->aio_fildes, F_GETFL);
+  if (__builtin_expect (flags == -1, 0)
+      || __builtin_expect ((flags & (O_RDWR | O_WRONLY)) == 0, 0))
+    {
+      __set_errno (EBADF);
+      return -1;
+    }
+
   return (__aio_enqueue_request ((aiocb_union *) aiocbp,
                                 op == O_SYNC ? LIO_SYNC : LIO_DSYNC) == NULL
          ? -1 : 0);
index f9c5709..8def01c 100644 (file)
@@ -1,5 +1,5 @@
 /* Suspend until termination of a requests.
-   Copyright (C) 1997, 1998, 1999, 2000 Free Software Foundation, Inc.
+   Copyright (C) 1997, 1998, 1999, 2000, 2002 Free Software Foundation, Inc.
    This file is part of the GNU C Library.
    Contributed by Ulrich Drepper <drepper@cygnus.com>, 1997.
 
@@ -29,7 +29,9 @@
 /* And undo the hack.  */
 #undef aio_suspend64
 
+#include <assert.h>
 #include <errno.h>
+#include <stdbool.h>
 #include <stdlib.h>
 #include <sys/time.h>
 
@@ -48,7 +50,7 @@ aio_suspend (list, nent, timeout)
   int cnt;
   int result = 0;
   int dummy;
-  int none = 1;
+  bool any = false;
 
   /* Request the mutex.  */
   pthread_mutex_lock (&__aio_requests_mutex);
@@ -56,24 +58,34 @@ aio_suspend (list, nent, timeout)
   /* There is not yet a finished request.  Signal the request that
      we are working for it.  */
   for (cnt = 0; cnt < nent; ++cnt)
-    if (list[cnt] != NULL && list[cnt]->__error_code == EINPROGRESS)
+    if (list[cnt] != NULL)
       {
-       requestlist[cnt] = __aio_find_req ((aiocb_union *) list[cnt]);
-
-       if (requestlist[cnt] != NULL)
+       if (list[cnt]->__error_code == EINPROGRESS)
          {
-           waitlist[cnt].cond = &cond;
-           waitlist[cnt].next = requestlist[cnt]->waiting;
-           waitlist[cnt].counterp = &dummy;
-           waitlist[cnt].sigevp = NULL;
-           waitlist[cnt].caller_pid = 0;       /* Not needed.  */
-           requestlist[cnt]->waiting = &waitlist[cnt];
-           none = 0;
+           requestlist[cnt] = __aio_find_req ((aiocb_union *) list[cnt]);
+
+           if (requestlist[cnt] != NULL)
+             {
+               waitlist[cnt].cond = &cond;
+               waitlist[cnt].next = requestlist[cnt]->waiting;
+               waitlist[cnt].counterp = &dummy;
+               waitlist[cnt].sigevp = NULL;
+               waitlist[cnt].caller_pid = 0;   /* Not needed.  */
+               requestlist[cnt]->waiting = &waitlist[cnt];
+               any = true;
+             }
+           else
+             /* We will never suspend.  */
+             break;
          }
+       else
+         /* We will never suspend.  */
+         break;
       }
 
-  /* If there is a not finished request wait for it.  */
-  if (!none)
+  /* If there is no finished request wait for it.  In any case we have
+     to dequeue the requests if we enqueued them.  */
+  if (any)
     {
       int oldstate;
 
@@ -82,39 +94,45 @@ aio_suspend (list, nent, timeout)
         which we must remove.  So defer cancelation for now.  */
       pthread_setcancelstate (PTHREAD_CANCEL_DISABLE, &oldstate);
 
-      if (timeout == NULL)
-       result = pthread_cond_wait (&cond, &__aio_requests_mutex);
-      else
+      /* Only if none of the entries is NULL or finished to be wait.  */
+      if (cnt == nent)
        {
-         /* We have to convert the relative timeout value into an
-            absolute time value with pthread_cond_timedwait expects.  */
-         struct timeval now;
-         struct timespec abstime;
-
-         __gettimeofday (&now, NULL);
-         abstime.tv_nsec = timeout->tv_nsec + now.tv_usec * 1000;
-         abstime.tv_sec = timeout->tv_sec + now.tv_sec;
-         if (abstime.tv_nsec >= 1000000000)
+         if (timeout == NULL)
+           result = pthread_cond_wait (&cond, &__aio_requests_mutex);
+         else
            {
-             abstime.tv_nsec -= 1000000000;
-             abstime.tv_sec += 1;
+             /* We have to convert the relative timeout value into an
+                absolute time value with pthread_cond_timedwait expects.  */
+             struct timeval now;
+             struct timespec abstime;
+
+             __gettimeofday (&now, NULL);
+             abstime.tv_nsec = timeout->tv_nsec + now.tv_usec * 1000;
+             abstime.tv_sec = timeout->tv_sec + now.tv_sec;
+             if (abstime.tv_nsec >= 1000000000)
+               {
+                 abstime.tv_nsec -= 1000000000;
+                 abstime.tv_sec += 1;
+               }
+
+             result = pthread_cond_timedwait (&cond, &__aio_requests_mutex,
+                                              &abstime);
            }
-
-         result = pthread_cond_timedwait (&cond, &__aio_requests_mutex,
-                                          &abstime);
        }
 
       /* Now remove the entry in the waiting list for all requests
         which didn't terminate.  */
-      for (cnt = 0; cnt < nent; ++cnt)
-       if (list[cnt] != NULL && list[cnt]->__error_code == EINPROGRESS
-           && requestlist[cnt] != NULL)
+      while (cnt-- > 0)
+       if (list[cnt] != NULL && list[cnt]->__error_code == EINPROGRESS)
          {
-           struct waitlist **listp = &requestlist[cnt]->waiting;
+           struct waitlist **listp;
+
+           assert (requestlist[cnt] != NULL);
 
            /* There is the chance that we cannot find our entry anymore.
               This could happen if the request terminated and restarted
               again.  */
+           listp = &requestlist[cnt]->waiting;
            while (*listp != NULL && *listp != &waitlist[cnt])
              listp = &(*listp)->next;
 
@@ -126,7 +144,7 @@ aio_suspend (list, nent, timeout)
       pthread_setcancelstate (oldstate, NULL);
 
       /* Release the conditional variable.  */
-      if (pthread_cond_destroy (&cond) != 0)
+      if (__builtin_expect (pthread_cond_destroy (&cond) != 0, 0))
        /* This must never happen.  */
        abort ();