Libguestfs - Sep 2009 - [PATCH] Daemon: fix handling of errors from xread.

This caused me to waste about 3 hours looking for a non-existent "data
corrupter" bug, which turned out to be the result of not checking the
return value of xread ...

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
-------------- next part --------------
>From 52ed541501934eb0464bdb8dff67a99d9ea0aaff Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones at trick.home.annexia.org>
Date: Thu, 17 Sep 2009 15:28:41 +0100
Subject: [PATCH 1/3] Daemon: fix handling of errors from xread.

If xread returns -1, that indicates a read error and we should exit.
Note that xread has already printed the error message.
---
 daemon/proto.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/daemon/proto.c b/daemon/proto.c
index 817a995..037a9e0 100644
--- a/daemon/proto.c
+++ b/daemon/proto.c
@@ -62,7 +62,9 @@ main_loop (int _sock)
 #endif
 
     /* Read the length word. */
-    xread (sock, lenbuf, 4);
+    if (xread (sock, lenbuf, 4) == -1)
+      exit (1);
+
     xdrmem_create (&xdr, lenbuf, 4, XDR_DECODE);
     xdr_uint32_t (&xdr, &len);
     xdr_destroy (&xdr);
@@ -79,7 +81,8 @@ main_loop (int _sock)
       continue;
     }
 
-    xread (sock, buf, len);
+    if (xread (sock, buf, len) == -1)
+      exit (1);
 
 #if 0
     if (verbose) {
@@ -307,7 +310,9 @@ receive_file (receive_cb cb, void *opaque)
 
   for (;;) {
     /* Read the length word. */
-    xread (sock, lenbuf, 4);
+    if (xread (sock, lenbuf, 4) == -1)
+      exit (1);
+
     xdrmem_create (&xdr, lenbuf, 4, XDR_DECODE);
     xdr_uint32_t (&xdr, &len);
     xdr_destroy (&xdr);
@@ -327,7 +332,8 @@ receive_file (receive_cb cb, void *opaque)
       return -1;
     }
 
-    xread (sock, buf, len);
+    if (xread (sock, buf, len) == -1)
+      exit (1);
 
     xdrmem_create (&xdr, buf, len, XDR_DECODE);
     memset (&chunk, 0, sizeof chunk);
@@ -444,10 +450,8 @@ check_for_library_cancellation (void)
 
   /* Read the message from the daemon. */
   r = xread (sock, buf, sizeof buf);
-  if (r == -1) {
-    perror ("read");
+  if (r == -1)
     return 0;
-  }
 
   xdrmem_create (&xdr, buf, sizeof buf, XDR_DECODE);
   xdr_uint32_t (&xdr, &flag);
-- 
1.6.2.5

On Thu, Sep 17, 2009 at 03:33:24PM +0100, Richard W.M. Jones
wrote:
> 
> This caused me to waste about 3 hours looking for a non-existent "data
> corrupter" bug, which turned out to be the result of not checking the
> return value of xread ...
You might want to add 

 __attribute__((__warn_unused_result__))

to the header file decl of xread/xwrite so that the compiler will
warn if any other code areas forget to check the return error code

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|