xfs
[Top] [All Lists]

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

To: Felix Blyakher <felixb@xxxxxxx>
Subject: Re: [PATCH] xfstests: fix async io error handling in fsx
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Wed, 1 Apr 2009 03:21:17 -0400
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1238442101-24311-1-git-send-email-felixb@xxxxxxx>
References: <1238442101-24311-1-git-send-email-felixb@xxxxxxx>
User-agent: Mutt/1.5.18 (2008-05-17)
On Mon, Mar 30, 2009 at 02:41:41PM -0500, Felix Blyakher wrote:
> The result of async io returned in the event.res in addition
> to the number of bytes read/written provides negated error
> number. The broken libaio defines event.res as unsigned
> while the same structure in the kernel defines it as signed.
> The kernel indeed treat it as signed, and returns the
> negated error number. Till libaio is fixed we provide
> the signed long temp var.
> Also set errno for each error condition in aio_rw, as the
> caller is not aio aware and expects ret(-1)+errno by the
> traditional libc convention.
> 
> Signed-off-by: Felix Blyakher <felixb@xxxxxxx>
> ---
>  ltp/fsx.c |   42 +++++++++++++++++++++++++++++++++++-------
>  1 files changed, 35 insertions(+), 7 deletions(-)

>       if (len != event.res) {
> -             fprintf(stderr, "bad read length: %lu instead of %u\n",
> -                             event.res, len);
> +             /*
> +              * The b0rked libaio defines event.res as unsigned.
> +              * However the kernel strucuture has it signed,
> +              * and it's used to pass negated error value.
> +              * Till the library is fixed use the temp var.
> +              */
> +             res = (long)event.res;
> +             if (res >= 0)
> +                     fprintf(stderr, "bad io length: %lu instead of %u\n",
> +                                     res, len);
> +             else {
> +                     fprintf(stderr, "errcode=%d\n", -res);
> +                     fprintf(stderr, "aio_rw: async io failed: %s\n",
> +                                     strerror(-res));
> +                     goto out_error;
> +             }
> +
>       }
>       return event.res;
> +
> +out_error:
> +     /*
> +      * The caller expects error return in traditional libc
> +      * convention, i.e. -1 and the errno set to error.
> +      */
> +     errno = ret <= 0 ? -ret : -res;
> +     return -1;

I wonder why this doesn't give a compiler warning.  res is only
initialized in that last branch above.  Wouldn't it be better to set
ret to res inside that branch and only use ret down here?

<Prev in Thread] Current Thread [Next in Thread>