[PATCH] xfstests: fix async io error handling in fsx

[Felix Blyakher]

On Mar 30, 2009, at 12:54 PM, Christoph Hellwig wrote:

> On Mon, Mar 30, 2009 at 12:30:50AM -0500, Felix Blyakher wrote:
>> --- a/ltp/fsx.c
>> +++ b/ltp/fsx.c
>> @@ -976,19 +976,41 @@ __aio_rw(int rw, int fd, char *buf, unsigned  
>> len, unsigned offset)
>> 		fprintf(stderr, "errcode=%d\n", ret);
>> 		fprintf(stderr, "aio_rw: io_submit failed: %s\n",
>> 				strerror(ret));
>> -		return(-1);
>> +		errno = -ret;
>> +		return -1;
>
> I think we'd be better off having all these places that set errno in  
> one
> place and use goto labels.  That way you can also add a small comment
> about it.

Makes sense, will do.

>
>
>> +		/*
>> +		 * The b0rked libaio defines event.res as signed.
>> +		 * However the kernel strucuture has it unsigned,
>> +		 * and it's used to pass negated error value.
>> +		 * Till the library is fixed use the temp var.
>> +		 */
>
> This comment seems backwards to the patch description and the actual
> code.

Hmm, not really.
If the libaio library would be correct (do you know whom to talk
to and where to send a patch), the code would look like this:

                 if (event.res >= 0)
                         fprintf(stderr, "bad io length: %lu instead  
of %u\n",
                                         event.res, len);
                 else {
                         fprintf(stderr, "errcode=%d\n", -event.res);
                         fprintf(stderr, "aio_rw: async io failed: %s 
\n",
                                         strerror(-event.res));
                         errno = -event.res;
                         return -1;
                 }


and no need for the temp var of right type, as in a patch now.
The comments just explains the need for that "long res" var.
It does not really describe the need to check the error value,
that's just the part of interface, I assume.
Though, I think, it would be clearer if I state the need to
check the error first, and then note why it could not be
done as above.

I'll repost.

Thanks,
Felix