qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v2 1/1] Coverity: Fix failure path for qemu_accept in migration
@ 2014-03-19 13:34 Dr. David Alan Gilbert (git)
  2014-03-19 16:01 ` Markus Armbruster
  0 siblings, 1 reply; 2+ messages in thread
From: Dr. David Alan Gilbert (git) @ 2014-03-19 13:34 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, pbonzini, quintela

From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>

Coverity defects 1005733 & 1005734 complain about passing a negative
value to closesocket in the error paths on incoming migration.

Stash the error value and print it in the message (previously we gave
no indication of the reason for the failure)

Use error_report

Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration-tcp.c  | 17 +++++++++++------
 migration-unix.c | 17 +++++++++++------
 2 files changed, 22 insertions(+), 12 deletions(-)

V2
  Use c < 0 everywhere
  Use error_report instead of fprintf(stdrr,
  Use strerror

diff --git a/migration-tcp.c b/migration-tcp.c
index 782572d..57ea9d2 100644
--- a/migration-tcp.c
+++ b/migration-tcp.c
@@ -13,7 +13,10 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
+#include <string.h>
+
 #include "qemu-common.h"
+#include "qemu/error-report.h"
 #include "qemu/sockets.h"
 #include "migration/migration.h"
 #include "migration/qemu-file.h"
@@ -56,24 +59,26 @@ static void tcp_accept_incoming_migration(void *opaque)
     socklen_t addrlen = sizeof(addr);
     int s = (intptr_t)opaque;
     QEMUFile *f;
-    int c;
+    int c, err;
 
     do {
         c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
-    } while (c == -1 && socket_error() == EINTR);
+        err = socket_error();
+    } while (c < 0 && err == EINTR);
     qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
     closesocket(s);
 
     DPRINTF("accepted migration\n");
 
-    if (c == -1) {
-        fprintf(stderr, "could not accept migration connection\n");
-        goto out;
+    if (c < 0) {
+        error_report("could not accept migration connection (%s)",
+                strerror(err));
+        return;
     }
 
     f = qemu_fopen_socket(c, "rb");
     if (f == NULL) {
-        fprintf(stderr, "could not qemu_fopen socket\n");
+        error_report("could not qemu_fopen socket");
         goto out;
     }
 
diff --git a/migration-unix.c b/migration-unix.c
index 651fc5b..05e2acd 100644
--- a/migration-unix.c
+++ b/migration-unix.c
@@ -13,7 +13,10 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
+#include <string.h>
+
 #include "qemu-common.h"
+#include "qemu/error-report.h"
 #include "qemu/sockets.h"
 #include "qemu/main-loop.h"
 #include "migration/migration.h"
@@ -56,24 +59,26 @@ static void unix_accept_incoming_migration(void *opaque)
     socklen_t addrlen = sizeof(addr);
     int s = (intptr_t)opaque;
     QEMUFile *f;
-    int c;
+    int c, err;
 
     do {
         c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
-    } while (c == -1 && errno == EINTR);
+        err = errno;
+    } while (c < 0 && err == EINTR);
     qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
     close(s);
 
     DPRINTF("accepted migration\n");
 
-    if (c == -1) {
-        fprintf(stderr, "could not accept migration connection\n");
-        goto out;
+    if (c < 0) {
+        error_report("could not accept migration connection (%s)",
+                strerror(err));
+        return;
     }
 
     f = qemu_fopen_socket(c, "rb");
     if (f == NULL) {
-        fprintf(stderr, "could not qemu_fopen socket\n");
+        error_report("could not qemu_fopen socket");
         goto out;
     }
 
-- 
1.8.5.3

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [Qemu-devel] [PATCH v2 1/1] Coverity: Fix failure path for qemu_accept in migration
  2014-03-19 13:34 [Qemu-devel] [PATCH v2 1/1] Coverity: Fix failure path for qemu_accept in migration Dr. David Alan Gilbert (git)
@ 2014-03-19 16:01 ` Markus Armbruster
  0 siblings, 0 replies; 2+ messages in thread
From: Markus Armbruster @ 2014-03-19 16:01 UTC (permalink / raw)
  To: Dr. David Alan Gilbert (git)
  Cc: peter.maydell, quintela, qemu-devel, pbonzini

"Dr. David Alan Gilbert (git)" <dgilbert@redhat.com> writes:

> From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
>
> Coverity defects 1005733 & 1005734 complain about passing a negative
> value to closesocket in the error paths on incoming migration.
>
> Stash the error value and print it in the message (previously we gave
> no indication of the reason for the failure)
>
> Use error_report
>
> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  migration-tcp.c  | 17 +++++++++++------
>  migration-unix.c | 17 +++++++++++------
>  2 files changed, 22 insertions(+), 12 deletions(-)
>
> V2
>   Use c < 0 everywhere
>   Use error_report instead of fprintf(stdrr,
>   Use strerror
>
> diff --git a/migration-tcp.c b/migration-tcp.c
> index 782572d..57ea9d2 100644
> --- a/migration-tcp.c
> +++ b/migration-tcp.c
> @@ -13,7 +13,10 @@
>   * GNU GPL, version 2 or (at your option) any later version.
>   */
>  
> +#include <string.h>
> +
>  #include "qemu-common.h"
> +#include "qemu/error-report.h"
>  #include "qemu/sockets.h"
>  #include "migration/migration.h"
>  #include "migration/qemu-file.h"
> @@ -56,24 +59,26 @@ static void tcp_accept_incoming_migration(void *opaque)
>      socklen_t addrlen = sizeof(addr);
>      int s = (intptr_t)opaque;
>      QEMUFile *f;
> -    int c;
> +    int c, err;
>  
>      do {
>          c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
> -    } while (c == -1 && socket_error() == EINTR);
> +        err = socket_error();
> +    } while (c < 0 && err == EINTR);
>      qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
>      closesocket(s);
>  
>      DPRINTF("accepted migration\n");
>  
> -    if (c == -1) {
> -        fprintf(stderr, "could not accept migration connection\n");
> -        goto out;
> +    if (c < 0) {
> +        error_report("could not accept migration connection (%s)",
> +                strerror(err));

If you need to respin for some other reason (which I doubt), fix up
indentation here.

> +        return;
>      }
>  
>      f = qemu_fopen_socket(c, "rb");
>      if (f == NULL) {
> -        fprintf(stderr, "could not qemu_fopen socket\n");
> +        error_report("could not qemu_fopen socket");
>          goto out;
>      }
>  
> diff --git a/migration-unix.c b/migration-unix.c
> index 651fc5b..05e2acd 100644
> --- a/migration-unix.c
> +++ b/migration-unix.c
> @@ -13,7 +13,10 @@
>   * GNU GPL, version 2 or (at your option) any later version.
>   */
>  
> +#include <string.h>
> +
>  #include "qemu-common.h"
> +#include "qemu/error-report.h"
>  #include "qemu/sockets.h"
>  #include "qemu/main-loop.h"
>  #include "migration/migration.h"
> @@ -56,24 +59,26 @@ static void unix_accept_incoming_migration(void *opaque)
>      socklen_t addrlen = sizeof(addr);
>      int s = (intptr_t)opaque;
>      QEMUFile *f;
> -    int c;
> +    int c, err;
>  
>      do {
>          c = qemu_accept(s, (struct sockaddr *)&addr, &addrlen);
> -    } while (c == -1 && errno == EINTR);
> +        err = errno;
> +    } while (c < 0 && err == EINTR);
>      qemu_set_fd_handler2(s, NULL, NULL, NULL, NULL);
>      close(s);
>  
>      DPRINTF("accepted migration\n");
>  
> -    if (c == -1) {
> -        fprintf(stderr, "could not accept migration connection\n");
> -        goto out;
> +    if (c < 0) {
> +        error_report("could not accept migration connection (%s)",
> +                strerror(err));

Likewise.

> +        return;
>      }
>  
>      f = qemu_fopen_socket(c, "rb");
>      if (f == NULL) {
> -        fprintf(stderr, "could not qemu_fopen socket\n");
> +        error_report("could not qemu_fopen socket");
>          goto out;
>      }

Reviewed-by: Markus Armbruster <armbru@redhat.com>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2014-03-19 16:01 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-19 13:34 [Qemu-devel] [PATCH v2 1/1] Coverity: Fix failure path for qemu_accept in migration Dr. David Alan Gilbert (git)
2014-03-19 16:01 ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).