Re: [Qemu-devel] [PATCH 1/6] file-posix: Fix EINTR handling

[Stefan Hajnoczi]

On Fri, Jun 08, 2018 at 02:04:12PM +0800, Fam Zheng wrote:
> EINTR should be checked against errno, not ret. While fixing the bug,
> collecting the branches with a switch block.
> 
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  block/file-posix.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 513d371bb1..c6dae38f94 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -1473,20 +1473,20 @@ static ssize_t 
> handle_aiocb_copy_range(RawPosixAIOData *aiocb)
>          ssize_t ret = copy_file_range(aiocb->aio_fildes, &in_off,
>                                        aiocb->aio_fd2, &out_off,
>                                        bytes, 0);
> -        if (ret == -EINTR) {
> -            continue;
> -        }
> -        if (ret < 0) {
> -            if (errno == ENOSYS) {
> +        if (ret <= 0) {
> +            switch (errno) {
> +            case 0:
> +                /* No progress (e.g. when beyond EOF), let the caller fall 
> back
> +                 * to buffer I/O. */
> +                /* fall through */

The C11 standard says:

  The value of errno in the initial thread is zero at program startup
  (the initial value of errno in other threads is an indeterminate
  value), but is never set to zero by any library function.

So this code is buggy because it assumes copy_file_range(2) sets errno
to 0 on success.  errno could actually contain a stale value from a
previous function).

signature.asc
Description: PGP signature