[1/6] file-posix: Fix EINTR handling

Commit Message

Fam Zheng June 8, 2018, 6:04 a.m. UTC

EINTR should be checked against errno, not ret. While fixing the bug,
collecting the branches with a switch block.

Signed-off-by: Fam Zheng <[e-mail]>
---
 block/file-posix.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

Stefan Hajnoczi June 15, 2018, 2:38 p.m. UTC | #1

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 <[e-mail]>
> ---
>  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).

Patch

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 */
+            case ENOSYS:
                 return -ENOTSUP;
-            } else {
+            case EINTR:
+                continue;
+            default:
                 return -errno;
             }
         }
-        if (!ret) {
-            /* No progress (e.g. when beyond EOF), fall back to buffer I/O. */
-            return -ENOTSUP;
-        }
         bytes -= ret;
     }
     return 0;