[RFC] [PATCH] Make lio_listio set errno to EIO on requests with invalid aio_lio_opcode

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|

[RFC] [PATCH] Make lio_listio set errno to EIO on requests with invalid aio_lio_opcode

Suzuki Poulose
Hi,

According to the POSIX standards, only supported opcodes with
lio_listio() are LIO_READ, LIO_WRITE and LIO_NOP. Currently lio_listio
doesn't check for non-supported/invalid opcodes.

The following patch adds the check and sets the errno to EIO.

Please review.

Thanks.

Suzuki K P
Linux Technology Centre,
IBM Software Labs.



        * sysdeps/pthread/lio_listio.c : Set errno to EIO on non-supported
aio_lio_opcodes in lio_listio().


--- libc/sysdeps/pthread/lio_listio.c 2006-01-06 09:40:03.000000000 +0530
+++ libc-mod/sysdeps/pthread/lio_listio.c 2006-02-10 14:32:38.000000000
+0530
@@ -82,10 +82,18 @@ lio_listio_internal (int mode, struct ai
        {
  if (NO_INDIVIDUAL_EVENT_P (mode))
   list[cnt]->aio_sigevent.sigev_notify = SIGEV_NONE;
-
- requests[cnt] = __aio_enqueue_request ((aiocb_union *) list[cnt],
-       (list[cnt]->aio_lio_opcode
-        | LIO_OPCODE_BASE));
+
+        if (list[cnt]->aio_lio_opcode != LIO_READ
+            && list[cnt]->aio_lio_opcode != LIO_WRITE)
+          {
+            /* Only LIO_{READ,WRITE,NOP} are supported. */
+            requests[cnt] = NULL;
+            __set_errno (EIO);
+          }
+        else
+          requests[cnt] = __aio_enqueue_request ((aiocb_union *) list[cnt],
+                                                 (list[cnt]->aio_lio_opcode
+                                                  | LIO_OPCODE_BASE));

  if (requests[cnt] != NULL)
   /* Successfully enqueued.  */
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] [PATCH] Make lio_listio set errno to EIO on requests with invalid aio_lio_opcode

Roland McGrath
There is no obligation to check for this kind of error, given the standard.
At any rate, the EINVAL will indeed be diagnosed when the request is
examined in the service thread, as I read the code.  If you think this is
not happening, please supply a test program to demonstrate the problem.


Thanks,
Roland
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] [PATCH] Make lio_listio set errno to EIO on requests with invalid aio_lio_opcode

Suzuki Poulose
Roland McGrath wrote:
> There is no obligation to check for this kind of error, given the standard.
According to the standards, lio_listio should return EIO with invalid
opcodes.

> At any rate, the EINVAL will indeed be diagnosed when the request is
> examined in the service thread, as I read the code.

Yes, the service thread does check it and sets EINVAL for the iocb, not
for lio_listio(). But, the lio_listio() would succeed, without returning
an error. It should have returned EIO, if it has some requests with
invalid opcode ( or opcodes other than LIO_{READ,WRITE,NOP}.

> If you think this is
> not happening, please supply a test program to demonstrate the problem.
>
>
> Thanks,
> Roland
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] [PATCH] Make lio_listio set errno to EIO on requests with invalid aio_lio_opcode

Jakub Jelinek
On Tue, Feb 21, 2006 at 12:54:09PM +0530, Suzuki wrote:
> Roland McGrath wrote:
> >There is no obligation to check for this kind of error, given the standard.
> According to the standards, lio_listio should return EIO with invalid
> opcodes.

"According to the standards" is vague, you need to cite precisely what you base
your assertion on.
I don't see such requirement in
http://www.opengroup.org/onlinepubs/009695399/functions/lio_listio.html

        Jakub
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] [PATCH] Make lio_listio set errno to EIO on requests with invalid aio_lio_opcode

Suzuki Poulose
Jakub Jelinek wrote:

> On Tue, Feb 21, 2006 at 12:54:09PM +0530, Suzuki wrote:
>
>>Roland McGrath wrote:
>>
>>>There is no obligation to check for this kind of error, given the standard.
>>
>>According to the standards, lio_listio should return EIO with invalid
>>opcodes.
>
>
> "According to the standards" is vague, you need to cite precisely what you base
> your assertion on.
> I don't see such requirement in
> http://www.opengroup.org/onlinepubs/009695399/functions/lio_listio.html

Sorry for the vague description. I was referring to the same document.
It says :

"The aio_lio_opcode field of each aiocb structure specifies the
operation to be performed. The supported operations are LIO_READ,
LIO_WRITE, and LIO_NOP; these symbols are defined in <aio.h>. The
LIO_NOP operation causes the list entry to be ignored. "
....

"[EIO]
     One or more of the individual I/O operations failed. The
application may check the error status for each aiocb structure to
determine the individual request(s) that failed."

If the opcode is not supported then that I/O operation should fail. For
those requests, since we know that I/O operation has failed, even before
enqueuing a request, shouldn't lio_listio() return EIO ?


Thanks,

Suzuki
>
> Jakub
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] [PATCH] Make lio_listio set errno to EIO on requests with invalid aio_lio_opcode

Roland McGrath
> If the opcode is not supported then that I/O operation should fail. For
> those requests, since we know that I/O operation has failed, even before
> enqueuing a request, shouldn't lio_listio() return EIO ?

There is no obligation for lio_listio "know" by the time that it returns
that an operation has failed, unless you use LIO_WAIT.  
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] [PATCH] Make lio_listio set errno to EIO on requests with invalid aio_lio_opcode

Ulrich Drepper
In reply to this post by Suzuki Poulose
Suzuki wrote:
> If the opcode is not supported then that I/O operation should fail. For
> those requests, since we know that I/O operation has failed, even before
> enqueuing a request, shouldn't lio_listio() return EIO ?

No, there is no such requirement.  The current code is correct and if a
program has problems it is that program's fault.

--
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖


signature.asc (259 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] [PATCH] Make lio_listio set errno to EIO on requests with invalid aio_lio_opcode

Mark Brown-8

Ulrich wrote:
> Suzuki wrote:
> > If the opcode is not supported then that I/O operation should fail. For
> > those requests, since we know that I/O operation has failed, even before
> > enqueuing a request, shouldn't lio_listio() return EIO ?
>
> No, there is no such requirement.  The current code is correct and if a
> program has problems it is that program's fault.

So the following doesn't apply? I know it's rationale, but it directs as to
the proper use of EIO....

----- quote from SUSv3 (TC2), System Interfaces, P697 lines 23139-23147 ----
Although it may appear that there are inconsistencies in the specified
circumstances for error codes, the [EIO] error condition applies when any
circumstance relating to an individual operation makes that operation fail.
This might be due to a badly formulated request (for example, the
aio_lio_opcode field is invalid, and aio_error ( ) returns [EINVAL]) or
might arise from application behavior (for example, the file descriptor is
closed before the operation is initiated, and aio_error ( ) returns [EBADF]).

--
Mark Brown/Austin
[hidden email]
Reply | Threaded
Open this post in threaded view
|

Re: [RFC] [PATCH] Make lio_listio set errno to EIO on requests with invalid aio_lio_opcode

Roland McGrath
> So the following doesn't apply? I know it's rationale, but it directs as to
> the proper use of EIO....

I read that rationale as explaining why lio_listio itself does not return
EINVAL or EBADF in such a case, as one might naively expect.  It is an
unusual part of the interface that EIO means the call filled in a buffer
with results for you to look at.  The normative text clearly explains the
EIO behavior to diagnose errors detected at the time of the lio_listio
call, without giving any requirement that any errors are necessarily
diagnosed then.  I don't think the example in the rationale is intended to
suggest that these particular errors are necessarily diagnosed at the time
of the call.

> ----- quote from SUSv3 (TC2), System Interfaces, P697 lines 23139-23147 ----
> Although it may appear that there are inconsistencies in the specified
> circumstances for error codes, the [EIO] error condition applies when any
> circumstance relating to an individual operation makes that operation fail.
> This might be due to a badly formulated request (for example, the
> aio_lio_opcode field is invalid, and aio_error ( ) returns [EINVAL]) or
> might arise from application behavior (for example, the file descriptor is
> closed before the operation is initiated, and aio_error ( ) returns [EBADF]).


Thanks,
Roland