Code cleanups related to RHBZ#580246.

This includes various code cleanups:

 (a) A regression test for RHBZ#580246.

 (b) Use write instead of fwrite to write out the tar file.  This is
     just because the error handling of write seems to be better
     specified and easier to use.

 (c) Use size_t instead of int for length.

 (d) Clearer debug messages when in verbose mode.

4 files changed, 32 insertions, 24 deletions

diff --git a/daemon/daemon.h b/daemon/daemon.h
index 19dd69cd..ebbeaa2e 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -133,7 +133,7 @@ extern void reply_with_perror_errno (int err, const char *fs, ...)
 /* daemon functions that receive files (FileIn) should call
  * receive_file for each FileIn parameter.
  */
-typedef int (*receive_cb) (void *opaque, const void *buf, int len);
+typedef int (*receive_cb) (void *opaque, const void *buf, size_t len);
 extern int receive_file (receive_cb cb, void *opaque);
 
 /* daemon functions that receive files (FileIn) can call this
diff --git a/daemon/proto.c b/daemon/proto.c
index 0002d80a..ee1c400d 100644
--- a/daemon/proto.c
+++ b/daemon/proto.c
@@ -324,6 +324,9 @@ receive_file (receive_cb cb, void *opaque)
   uint32_t len;
 
   for (;;) {
+    if (verbose)
+      fprintf (stderr, "receive_file: reading length word\n");
+
     /* Read the length word. */
     if (xread (sock, lenbuf, 4) == -1)
       exit (EXIT_FAILURE);
@@ -361,15 +364,18 @@ receive_file (receive_cb cb, void *opaque)
     free (buf);
 
     if (verbose)
-      printf ("receive_file: got chunk: cancel = %d, len = %d, buf = %p\n",
-              chunk.cancel, chunk.data.data_len, chunk.data.data_val);
+      fprintf (stderr, "receive_file: got chunk: cancel = %d, len = %d, buf = %p\n",
+               chunk.cancel, chunk.data.data_len, chunk.data.data_val);
 
     if (chunk.cancel) {
-      fprintf (stderr, "receive_file: received cancellation from library\n");
+      if (verbose)
+        fprintf (stderr, "receive_file: received cancellation from library\n");
       xdr_free ((xdrproc_t) xdr_guestfs_chunk, (char *) &chunk);
       return -2;
     }
     if (chunk.data.data_len == 0) {
+      if (verbose)
+        fprintf (stderr, "receive_file: end of file, leaving function\n");
       xdr_free ((xdrproc_t) xdr_guestfs_chunk, (char *) &chunk);
       return 0;			/* end of file */
     }
@@ -380,8 +386,11 @@ receive_file (receive_cb cb, void *opaque)
       r = 0;
 
     xdr_free ((xdrproc_t) xdr_guestfs_chunk, (char *) &chunk);
-    if (r == -1)		/* write error */
+    if (r == -1) {		/* write error */
+      if (verbose)
+        fprintf (stderr, "receive_file: write error\n");
       return -1;
+    }
   }
 }
 
diff --git a/daemon/tar.c b/daemon/tar.c
index bb0e4830..8088606f 100644
--- a/daemon/tar.c
+++ b/daemon/tar.c
@@ -28,10 +28,10 @@
 #include "actions.h"
 
 static int
-fwrite_cb (void *fp_ptr, const void *buf, int len)
+write_cb (void *fd_ptr, const void *buf, size_t len)
 {
-  FILE *fp = *(FILE **)fp_ptr;
-  return fwrite (buf, len, 1, fp) == 1 ? 0 : -1;
+  int fd = *(int *)fd_ptr;
+  return xwrite (fd, buf, len);
 }
 
 /* Has one FileIn parameter. */
@@ -71,12 +71,15 @@ do_tar_in (const char *dir)
   }
   free (cmd);
 
-  r = receive_file (fwrite_cb, &fp);
+  /* The semantics of fwrite are too undefined, so write to the
+   * file descriptor directly instead.
+   */
+  int fd = fileno (fp);
+
+  r = receive_file (write_cb, &fd);
   if (r == -1) {		/* write error */
-    err = errno;
     cancel_receive ();
-    errno = err;
-    reply_with_perror ("write: %s", dir);
+    reply_with_error ("write error on directory: %s", dir);
     pclose (fp);
     return -1;
   }
@@ -87,11 +90,9 @@ do_tar_in (const char *dir)
   }
 
   if (pclose (fp) != 0) {
-    err = errno;
     if (r == -1)                /* if r == 0, file transfer ended already */
       cancel_receive ();
-    errno = err;
-    reply_with_perror ("pclose: %s", dir);
+    reply_with_error ("tar subcommand failed on directory: %s", dir);
     return -1;
   }
 
@@ -193,12 +194,12 @@ do_tgz_in (const char *dir)
   }
   free (cmd);
 
-  r = receive_file (fwrite_cb, &fp);
+  int fd = fileno (fp);
+
+  r = receive_file (write_cb, &fd);
   if (r == -1) {		/* write error */
-    err = errno;
     cancel_receive ();
-    errno = err;
-    reply_with_perror ("write: %s", dir);
+    reply_with_error ("write error on directory: %s", dir);
     pclose (fp);
     return -1;
   }
@@ -209,11 +210,9 @@ do_tgz_in (const char *dir)
   }
 
   if (pclose (fp) != 0) {
-    err = errno;
     if (r == -1)                /* if r == 0, file transfer ended already */
       cancel_receive ();
-    errno = err;
-    reply_with_perror ("pclose: %s", dir);
+    reply_with_error ("tar subcommand failed on directory: %s", dir);
     return -1;
   }
 
diff --git a/daemon/upload.c b/daemon/upload.c
index 65c66675..3c20d6f0 100644
--- a/daemon/upload.c
+++ b/daemon/upload.c
@@ -28,7 +28,7 @@
 #include "actions.h"
 
 static int
-write_cb (void *fd_ptr, const void *buf, int len)
+write_cb (void *fd_ptr, const void *buf, size_t len)
 {
   int fd = *(int *)fd_ptr;
   return xwrite (fd, buf, len);
@@ -65,7 +65,7 @@ do_upload (const char *filename)
     err = errno;
     cancel_receive ();
     errno = err;
-    reply_with_perror ("write: %s", filename);
+    reply_with_error ("write error: %s", filename);
     close (fd);
     return -1;
   }