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.

diff --git a/daemon/daemon.h b/daemon/daemon.h
index 19dd69c..ebbeaa2 100644 (file)
--- 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 0002d80..ee1c400 100644 (file)
--- 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 bb0e483..8088606 100644 (file)
--- 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 65c6667..3c20d6f 100644 (file)
--- 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;
   }

diff --git a/regressions/Makefile.am b/regressions/Makefile.am
index 2710e82..e8e43cf 100644 (file)
--- a/regressions/Makefile.am
+++ b/regressions/Makefile.am
@@ -27,6 +27,7 @@ TESTS = \
        rhbz503169c10.sh \
        rhbz503169c13.sh \
        rhbz557655.sh \
+       rhbz580246.sh \
        test-cancellation-download-librarycancels.sh \
        test-cancellation-upload-daemoncancels.sh \
        test-find0.sh \

diff --git a/regressions/rhbz580246.sh b/regressions/rhbz580246.sh
new file mode 100755 (executable)
index 0000000..707231e
--- /dev/null
+++ b/regressions/rhbz580246.sh
@@ -0,0 +1,47 @@
+#!/bin/bash -
+# libguestfs
+# Copyright (C) 2010 Red Hat Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
+
+# Test tar_in call when we upload something which is larger than
+# available space.
+# https://bugzilla.redhat.com/show_bug.cgi?id=580246
+
+set -e
+export LANG=C
+
+rm -f test.img test.tar
+
+dd if=/dev/zero of=test.img bs=1M count=2
+tar cf test.tar test.img
+
+output=$(
+../fish/guestfish 2>&1 <<'EOF'
+add test.img
+run
+mkfs ext2 /dev/sda
+mount /dev/sda /
+-tar-in test.tar /
+EOF
+)
+
+rm -f test.img test.tar
+
+# Check for error message in the output.
+if [[ ! $output =~ libguestfs:.error:.tar_in ]]; then
+    echo "Missing error message from tar-in (expecting an error message)"
+    exit 1
+fi