Patchwork [34/41] savevm: Only qemu_fflush() can generate errors

login
register
mail settings
Submitter Juan Quintela
Date Sept. 21, 2012, 8:47 a.m.
Message ID <1348217255-22441-35-git-send-email-quintela@redhat.com>
Download mbox | patch
Permalink /patch/185668/
State New
Headers show

Comments

Juan Quintela - Sept. 21, 2012, 8:47 a.m.
Move the error check to the beggining of the callers.  Once this is fixed
qemu_file_set_if_error() is not used anymore, so remove it.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 savevm.c | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)
Paolo Bonzini - Sept. 21, 2012, 12:49 p.m.
Il 21/09/2012 10:47, Juan Quintela ha scritto:
> Move the error check to the beggining of the callers.  Once this is fixed
> qemu_file_set_if_error() is not used anymore, so remove it.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  savevm.c | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
> 
> diff --git a/savevm.c b/savevm.c
> index 4e4aa3c..59ec8bf 100644
> --- a/savevm.c
> +++ b/savevm.c
> @@ -445,18 +445,6 @@ void qemu_file_set_error(QEMUFile *f, int ret)
>      f->last_error = ret;
>  }
> 
> -/** Sets last_error conditionally
> - *
> - * Sets last_error only if ret is negative _and_ no error
> - * was set before.
> - */
> -static void qemu_file_set_if_error(QEMUFile *f, int ret)
> -{
> -    if (ret < 0 && !f->last_error) {
> -        qemu_file_set_error(f, ret);
> -    }
> -}
> -
>  /** Flushes QEMUFile buffer
>   *
>   */
> @@ -544,13 +532,17 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
>  {
>      int l;
> 
> -    if (!f->last_error && f->is_write == 0 && f->buf_index > 0) {
> +    if (f->last_error) {
> +        return;
> +    }
> +
> +    if (f->is_write == 0 && f->buf_index > 0) {
>          fprintf(stderr,
>                  "Attempted to write to buffer while read buffer is not empty\n");
>          abort();
>      }
> 
> -    while (!f->last_error && size > 0) {
> +    while (size > 0) {
>          l = IO_BUF_SIZE - f->buf_index;
>          if (l > size)
>              l = size;
> @@ -561,14 +553,21 @@ void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
>          size -= l;
>          if (f->buf_index >= IO_BUF_SIZE) {
>              int ret = qemu_fflush(f);
> -            qemu_file_set_if_error(f, ret);
> +            if (ret < 0) {
> +                qemu_file_set_error(f, ret);
> +                break;
> +            }
>          }
>      }
>  }
> 
>  void qemu_put_byte(QEMUFile *f, int v)
>  {
> -    if (!f->last_error && f->is_write == 0 && f->buf_index > 0) {
> +    if (f->last_error) {
> +        return;
> +    }
> +
> +    if (f->is_write == 0 && f->buf_index > 0) {
>          fprintf(stderr,
>                  "Attempted to write to buffer while read buffer is not empty\n");
>          abort();
> @@ -578,7 +577,9 @@ void qemu_put_byte(QEMUFile *f, int v)
>      f->is_write = 1;
>      if (f->buf_index >= IO_BUF_SIZE) {
>          int ret = qemu_fflush(f);
> -        qemu_file_set_if_error(f, ret);
> +        if (ret < 0) {
> +            qemu_file_set_error(f, ret);
> +        }
>      }
>  }
> 

Just a matter of taste, but I happen to agree.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Patch

diff --git a/savevm.c b/savevm.c
index 4e4aa3c..59ec8bf 100644
--- a/savevm.c
+++ b/savevm.c
@@ -445,18 +445,6 @@  void qemu_file_set_error(QEMUFile *f, int ret)
     f->last_error = ret;
 }

-/** Sets last_error conditionally
- *
- * Sets last_error only if ret is negative _and_ no error
- * was set before.
- */
-static void qemu_file_set_if_error(QEMUFile *f, int ret)
-{
-    if (ret < 0 && !f->last_error) {
-        qemu_file_set_error(f, ret);
-    }
-}
-
 /** Flushes QEMUFile buffer
  *
  */
@@ -544,13 +532,17 @@  void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
 {
     int l;

-    if (!f->last_error && f->is_write == 0 && f->buf_index > 0) {
+    if (f->last_error) {
+        return;
+    }
+
+    if (f->is_write == 0 && f->buf_index > 0) {
         fprintf(stderr,
                 "Attempted to write to buffer while read buffer is not empty\n");
         abort();
     }

-    while (!f->last_error && size > 0) {
+    while (size > 0) {
         l = IO_BUF_SIZE - f->buf_index;
         if (l > size)
             l = size;
@@ -561,14 +553,21 @@  void qemu_put_buffer(QEMUFile *f, const uint8_t *buf, int size)
         size -= l;
         if (f->buf_index >= IO_BUF_SIZE) {
             int ret = qemu_fflush(f);
-            qemu_file_set_if_error(f, ret);
+            if (ret < 0) {
+                qemu_file_set_error(f, ret);
+                break;
+            }
         }
     }
 }

 void qemu_put_byte(QEMUFile *f, int v)
 {
-    if (!f->last_error && f->is_write == 0 && f->buf_index > 0) {
+    if (f->last_error) {
+        return;
+    }
+
+    if (f->is_write == 0 && f->buf_index > 0) {
         fprintf(stderr,
                 "Attempted to write to buffer while read buffer is not empty\n");
         abort();
@@ -578,7 +577,9 @@  void qemu_put_byte(QEMUFile *f, int v)
     f->is_write = 1;
     if (f->buf_index >= IO_BUF_SIZE) {
         int ret = qemu_fflush(f);
-        qemu_file_set_if_error(f, ret);
+        if (ret < 0) {
+            qemu_file_set_error(f, ret);
+        }
     }
 }