This is the mail archive of the libc-alpha@sources.redhat.com mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

PATCH: aio_read() POSIX compliance


This is a small patch to fix some POSIX compliance issues in aio_read().

The IEEE Std 1003.1-2001
(http://www.opengroup.org/onlinepubs/007904975/toc.htm) says
(http://www.opengroup.org/onlinepubs/007904975/functions/aio_read.html)
with respect to aio_read():

    int aio_read(struct aiocb *aiocbp);

    If any of the conditions below are detected synchronously, the
    aio_read() function shall return -1 and set errno to the corresponding
    value.

    The aio_read() function shall fail if:

    [EBADF] The aiocbp->aio_fildes argument is not a valid file descriptor
    open for reading.

1. Right now, glibc segfaults if passed a null pointer for aiocbp (see below
for code listing for test0013.c):

    % ./test0013
    Segmentation fault

I suggest that when aiocbp is NULL, aio_read() return -1 and set errno to
EBADF.  Note that fopen(3) and related functions perform a similar check.

2. Right now, aio_read() sometimes returns with errno set to EINTR (#defined
as 4), even if it succeeded (see below for code listing for test0014.c):

    % ./test0014
    aio_read() ret: 0       errno: 4
    n a m e s e r v
    aio_return() ret: 8     BYTES: 8        errno: 0

The reason for this behavior is that the TEMP_FAILURE_RETRY macro does not
reset errno from EINVAL to 0 if pread() succeeds after being interrupted.  I
suggest that when aio_read() succeeds, it return with errno set to zero.
The patch chooses to fix the problem in aio_read() rather than to change the
macro with possibly far-reaching subtle ramifications.

3. Right now, aio_read() does not synchronously detect an invalid file
descriptor.  For example, if it is passed a file descriptor that is open for
write only, aio_read() returns 0 (see below for code listing of test0015.c):

    %  ./test0015
    aio_read() ret: 0       errno: 0
    aio_return() ret: -1

I suggest that when the passed file descriptor is invalid, aio_read() return
-1 and set errno to EBADF.

4. Right now, aio_read() does not synchronously detect an invalid value for
aiocbp->aio_nbytes.  For example, if it is passed a negative value, aio_read()
returns 0 (see below for code listing for test0017.c).

    % ./test0017
    ./test0017: nbytes: -1; errno: 4 != EINVAL or ret: 0 != -1

I suggest that when aiocbp->aio_nbytes is invalid, aio_read() return -1 and
set errno to EINVAL.

5. Right now, aio_read() does not synchronously detect an invalid value for
aiocpb->aio_offset.  For example, if it is passed a negative value,
aio_read() returns 0 (see below for code listing for test0018.c).

    % ./test0018
    ./test0018: offset: -1; errno: 4 != EINVAL or ret: 0 != -1

I suggest that when aiocbp->aio_offset is invalid, aio_read() return -1 and
set errno to EINVAL.

Here is the patch (authored by Tom Gall and Amos Waterland of IBM LTC).

---- Begin patch ----

Index: sysdeps/pthread/aio_read.c
===================================================================
RCS file: /cvs/glibc/libc/sysdeps/pthread/aio_read.c,v
retrieving revision 1.2
diff -u -r1.2 aio_read.c
--- sysdeps/pthread/aio_read.c	6 Jul 2001 04:56:02 -0000	1.2
+++ sysdeps/pthread/aio_read.c	12 Jun 2002 20:03:35 -0000
@@ -19,14 +19,43 @@
    02111-1307 USA.  */
 
 #include <aio.h>
+#include <errno.h>
 
 #include "aio_misc.h"
 
-
 int
 aio_read (aiocbp)
      struct aiocb *aiocbp;
 {
-  return (__aio_enqueue_request ((aiocb_union *) aiocbp, LIO_READ) == NULL
-	  ? -1 : 0);
+  int flags;
+
+  if (aiocbp == NULL ||
+      (flags = fcntl (aiocbp->aio_fildes, F_GETFL)) < 0 || (flags & O_WRONLY))
+    {
+      __set_errno(EBADF);
+      return -1;
+    }
+
+  /* Typecast the values to get signed comparison. */
+  if ((long int)aiocbp->aio_offset < 0 || (long int)aiocbp->aio_nbytes < 0)
+    {
+      __set_errno(EINVAL);
+      return -1;
+    }
+
+  if (__aio_enqueue_request ((aiocb_union *) aiocbp, LIO_READ) == NULL)
+    {
+      return -1;
+    }
+
+  /* The TEMP_FAILURE_RETRY macro does not reset errno from EINVAL to 0 if
+   * pread() was interrupted.  Fix this here rather than change the macro
+   * with possibly far-reaching subtle ramifications.
+   */
+  if (errno == EINTR)
+    {
+      __set_errno(0);
+    }
+
+  return 0;
 }

---- End patch ----

---- Begin test0013.c ----

/* Show that aio_read() segfaults if passed a null pointer.
 * Amos Waterland <apw@us.ibm.com>
 * 10 June 2002
 */

#include <aio.h>

int main( int argc, char *argv[] )
{
    aio_read( NULL );

    return 0;
}

---- End test0013.c ----

---- Begin test0014.c ----

/* Show that aio_read() leaves errno equal to EINTR even if it succeeds.
 * Amos Waterland <apw@us.ibm.com>
 * 5 June 2002
 */

#include <aio.h>
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>

#define BYTES 8

int main( int argc, char *argv[] )
{
    int i, r;
    int fildes;
    struct aiocb cb;
    char buff[BYTES];
    
    if ((fildes = open( "/etc/resolv.conf", O_RDONLY )) < 0) {
        perror( "opening file" ); return 1;
    }

    cb.aio_fildes = fildes;
    cb.aio_offset = 0;
    cb.aio_buf = buff;
    cb.aio_nbytes = BYTES;
    cb.aio_reqprio = 0;
    cb.aio_sigevent.sigev_notify = SIGEV_NONE;

    errno = 0;
    r = aio_read( &cb );
    printf( "aio_read() ret: %i\terrno: %i\n", r, errno );

    while (aio_error( &cb ) == EINPROGRESS) { usleep( 10 ); }

    for (i = 0; i < BYTES; i++) { printf( "%c ", buff[i] ); } printf( "\n" );

    errno = 0;
    r = aio_return( &cb );
    printf( "aio_return() ret: %i\tBYTES: %i\terrno: %i\n", r, BYTES, errno );

    return 0;
}

---- End test0014.c ----

---- Begin test0015.c ----

/* Show that aio_read() does not detect an invalid file descriptor.
 * Amos Waterland <apw@us.ibm.com>
 * 5 June 2002
 */

#include <aio.h>
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>

#define BYTES 8

int main( int argc, char *argv[] )
{
    int r;
    int fildes;
    struct aiocb cb;
    char out[BYTES] = "0123456";
    char in[BYTES];
    
    if ((fildes = open( "/tmp/test0015.dat", O_WRONLY|O_CREAT, 0600 )) < 0) {
        perror( "opening file" ); return 1;
    }

    cb.aio_fildes = fildes;
    cb.aio_offset = 0;
    cb.aio_buf = out;
    cb.aio_nbytes = BYTES;
    cb.aio_reqprio = 0;
    cb.aio_sigevent.sigev_notify = SIGEV_NONE;

    /* first write some bytes to the O_WRONLY file descriptor */
    r = aio_write( &cb );
    while (aio_error( &cb ) == EINPROGRESS) { usleep( 10 ); }
    if (aio_return( &cb ) != BYTES) { perror( "write" ); return 1; }

    /* now try reading from the same file descriptor */
    cb.aio_buf = in;
    errno = 0;
    r = aio_read( &cb );
    printf( "aio_read() ret: %i\terrno: %i\n", r, errno );
    while (aio_error( &cb ) == EINPROGRESS) { usleep( 10 ); }
    if ((r = aio_return( &cb )) != BYTES) { 
        printf( "aio_return() ret: %i\n", r );
    }

    return 0;
}

---- End test0015.c ----

---- Begin test0017.c ----

/* Show that aio_read() does not detect an invalid value for nbytes.
 * Amos Waterland <apw@us.ibm.com>
 * 12 June 2002
 */

#include <aio.h>
#include <error.h>
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>

int main( int argc, char *argv[] )
{
    const int BYTES = 8;
    int r, fildes;
    struct aiocb cb;
    char buff[BYTES];
    
    if ((fildes = open( "/etc/resolv.conf", O_RDONLY )) < 0) {
        perror( "error opening file" ); return 1;
    }

    cb.aio_fildes = fildes;
    cb.aio_offset = 0;
    cb.aio_buf = buff;
    cb.aio_nbytes = -1;
    cb.aio_reqprio = 0;
    cb.aio_sigevent.sigev_notify = SIGEV_NONE;

    errno = 0;
    r = aio_read( &cb );

    if (r != -1 || errno != EINVAL) {
        error(0,0,"nbytes: -1; errno: %i != EINVAL or ret: %i != -1",errno,r);
    }

    return 0;
}

---- End test0017.c ----

---- Begin test0018.c ----

/* Show that aio_read() does not detect an invalid value for offset.
 * Amos Waterland <apw@us.ibm.com>
 * 12 June 2002
 */

#include <aio.h>
#include <error.h>
#include <errno.h>
#include <stdio.h>
#include <string.h>
#include <unistd.h>

int main( int argc, char *argv[] )
{
    const int BYTES = 8;
    int r, fildes;
    struct aiocb cb;
    char buff[BYTES];
    
    if ((fildes = open( "/etc/resolv.conf", O_RDONLY )) < 0) {
        perror( "error opening file" ); return 1;
    }

    cb.aio_fildes = fildes;
    cb.aio_offset = -1;
    cb.aio_buf = buff;
    cb.aio_nbytes = 0;
    cb.aio_reqprio = 0;
    cb.aio_sigevent.sigev_notify = SIGEV_NONE;

    errno = 0;
    r = aio_read( &cb );

    if (r != -1 || errno != EINVAL) {
        error(0,0,"offset: -1; errno: %i != EINVAL or ret: %i != -1",errno,r);
    }

    return 0;
}

---- End test0018.c ----


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]