Libguestfs - Mar 2016 - [PATCH 0/2] rename icat API as download_inode

"icat" name comes from the employed command line tool which might be
replaced later on with a different implementation.

The command name is a bit confusing because it's similar to "cat"
but act as "donwload". Therefore I am renaming it with a more clear
name.

At the same time I cleaned up a bit the code and improved it's readability
and code comments.

This patch is ready for review.
Code available at:

https://github.com/noxdafox/libguestfs/tree/download_inode

Matteo Cafasso (2):
  rename icat API to download_inode
  updated download_inode tests

 daemon/sleuthkit.c               | 79 ++++++++++++++++++++++------------------
 generator/actions.ml             |  6 ++-
 tests/tsk/Makefile.am            |  2 +-
 tests/tsk/test-download-inode.sh | 53 +++++++++++++++++++++++++++
 tests/tsk/test-icat.sh           | 53 ---------------------------
 5 files changed, 101 insertions(+), 92 deletions(-)
 create mode 100755 tests/tsk/test-download-inode.sh
 delete mode 100755 tests/tsk/test-icat.sh

--
2.8.0.rc3

Signed-off-by: Matteo Cafasso <noxdafox@gmail.com>
---
 daemon/sleuthkit.c   | 79 ++++++++++++++++++++++++++++------------------------
 generator/actions.ml |  6 ++--
 2 files changed, 47 insertions(+), 38 deletions(-)

diff --git a/daemon/sleuthkit.c b/daemon/sleuthkit.c
index 0fe1250..536febb 100644
--- a/daemon/sleuthkit.c
+++ b/daemon/sleuthkit.c
@@ -29,55 +29,55 @@
 #include "actions.h"
 #include "optgroups.h"

-static int file_out (const char *cmd);
+int optgroup_sleuthkit_available(void);
+static int send_command_output(const char *cmd);

-GUESTFSD_EXT_CMD(str_sleuthkit_probe, icat);
+GUESTFSD_EXT_CMD(str_icat, icat);

-int
-optgroup_sleuthkit_available (void)
-{
-  return prog_exists (str_sleuthkit_probe);
-}
-
-int
-do_icat (const mountable_t *mountable, int64_t inode)
+/* Has one FileOut parameter. */
+int do_download_inode(const mountable_t *mountable, int64_t inode)
 {
   CLEANUP_FREE char *cmd = NULL;

   /* Inode must be greater than 0 */
   if (inode < 0) {
-    reply_with_error ("inode must be >= 0");
+    reply_with_error("inode must be >= 0");
+
     return -1;
   }

   /* Construct the command. */
-  if (asprintf (&cmd, "icat -r %s %" PRIi64,
mountable->device, inode) == -1) {
-    reply_with_perror ("asprintf");
+  if (asprintf(&cmd, "%s -r %s %" PRIi64,
+               str_icat, mountable->device, inode) == -1) {
+    reply_with_perror("asprintf");
+
     return -1;
   }

-  return file_out (cmd);
+  return send_command_output(cmd);
 }

-static int
-file_out (const char *cmd)
+/* Runs the given command, collects the output and sends it to the appliance.
+ * Return 0 on success, -1 on error.
+ */
+static int send_command_output(const char *cmd)
 {
   int r;
   FILE *fp;
   CLEANUP_FREE char *buffer = NULL;

   if (verbose)
-    fprintf (stderr, "%s\n", cmd);
+    fprintf(stderr, "%s\n", cmd);
+
+  if (!(buffer = malloc(GUESTFS_MAX_CHUNK_SIZE))) {
+    reply_with_perror("malloc");

-  buffer = malloc (GUESTFS_MAX_CHUNK_SIZE);
-  if (buffer == NULL) {
-    reply_with_perror ("malloc");
     return -1;
   }

-  fp = popen (cmd, "r");
-  if (fp == NULL) {
-    reply_with_perror ("%s", cmd);
+  if (!(fp = popen(cmd, "r"))) {
+    reply_with_perror("%s", cmd);
+
     return -1;
   }

@@ -85,30 +85,37 @@ file_out (const char *cmd)
    * this there is no opportunity in the protocol to send any error
    * message back.  Instead we can only cancel the transfer.
    */
-  reply (NULL, NULL);
+  reply(NULL, NULL);
+
+  while ((r = fread(buffer, 1, sizeof buffer, fp)) > 0)
+    if (send_file_write(buffer, r) < 0) {
+      pclose(fp);

-  while ((r = fread (buffer, 1, sizeof buffer, fp)) > 0) {
-    if (send_file_write (buffer, r) < 0) {
-      pclose (fp);
       return -1;
     }
-  }

-  if (ferror (fp)) {
-    fprintf (stderr, "fread: %m");
-    send_file_end (1);		/* Cancel. */
-    pclose (fp);
+  if (ferror(fp)) {
+    fprintf(stderr, "fread: %m");
+    send_file_end(1);		/* Cancel. */
+    pclose(fp);
+
     return -1;
   }

-  if (pclose (fp) != 0) {
-    fprintf (stderr, "pclose: %m");
-    send_file_end (1);		/* Cancel. */
+  if (pclose(fp) != 0) {
+    fprintf(stderr, "pclose: %m");
+    send_file_end(1);		/* Cancel. */
+
     return -1;
   }

-  if (send_file_end (0))	/* Normal end of file. */
+  if (send_file_end(0))	        /* Normal end of file. */
     return -1;

   return 0;
 }
+
+int optgroup_sleuthkit_available(void)
+{
+  return prog_exists(str_icat);
+}
diff --git a/generator/actions.ml b/generator/actions.ml
index ff72cfe..f52db58 100644
--- a/generator/actions.ml
+++ b/generator/actions.ml
@@ -12946,7 +12946,7 @@ The filesystem from which to extract the file must be
unmounted,
 otherwise the call will fail." };

   { defaults with
-    name = "icat"; added = (1, 33, 14);
+    name = "download_inode"; added = (1, 33, 14);
     style = RErr, [Mountable "device"; Int64 "inode";
FileOut "filename"], [];
     proc_nr = Some 464;
     optional = Some "sleuthkit";
@@ -12956,7 +12956,9 @@ otherwise the call will fail." };
 Download a file given its inode from the disk partition (eg.
F</dev/sda1>)
 and save it as F<filename> on the local machine.

-This allows to download deleted or inaccessible files." };
+It is not required to mount the disk to run this command.
+
+The command allows is capable of downloading deleted or inaccessible
files." };

 ]

--
2.8.0.rc3

Signed-off-by: Matteo Cafasso <noxdafox@gmail.com>
---
 tests/tsk/Makefile.am            |  2 +-
 tests/tsk/test-download-inode.sh | 53 ++++++++++++++++++++++++++++++++++++++++
 tests/tsk/test-icat.sh           | 53 ----------------------------------------
 3 files changed, 54 insertions(+), 54 deletions(-)
 create mode 100755 tests/tsk/test-download-inode.sh
 delete mode 100755 tests/tsk/test-icat.sh

diff --git a/tests/tsk/Makefile.am b/tests/tsk/Makefile.am
index e060e58..0cd7c03 100644
--- a/tests/tsk/Makefile.am
+++ b/tests/tsk/Makefile.am
@@ -18,7 +18,7 @@
 include $(top_srcdir)/subdir-rules.mk

 TESTS = \
-	test-icat.sh
+	test-download-inode.sh

 TESTS_ENVIRONMENT = $(top_builddir)/run --test

diff --git a/tests/tsk/test-download-inode.sh b/tests/tsk/test-download-inode.sh
new file mode 100755
index 0000000..9c65aa9
--- /dev/null
+++ b/tests/tsk/test-download-inode.sh
@@ -0,0 +1,53 @@
+#!/bin/bash -
+# libguestfs
+# Copyright (C) 2016 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+# Test the download_inode command.
+
+set -e
+
+if [ -n "$SKIP_TEST_DOWNLOAD_INODE_SH" ]; then
+    echo "$0: test skipped because environment variable is set."
+    exit 77
+fi
+
+rm -f test-mft.bin
+
+# Skip if TSK is not supported by the appliance.
+if ! guestfish add /dev/null : run : available "sleuthkit"; then
+    echo "$0: skipped because TSK is not available in the appliance"
+    exit 77
+fi
+
+if [ ! -s ../../test-data/phony-guests/windows.img ]; then
+    echo "$0: skipped because windows.img is zero-sized"
+    exit 77
+fi
+
+# download Master File Table ($MFT).
+guestfish --ro -a ../../test-data/phony-guests/windows.img <<EOF
+run
+download-inode /dev/sda2 0 test-mft.bin
+EOF
+
+# test extracted file is the Master File Table
+if [ `head -c 5 test-mft.bin` != "FILE0" ]; then
+    echo "$0: wrong file extracted."
+    exit 1
+fi
+
+rm -f test-mft.bin
diff --git a/tests/tsk/test-icat.sh b/tests/tsk/test-icat.sh
deleted file mode 100755
index 3b0fca4..0000000
--- a/tests/tsk/test-icat.sh
+++ /dev/null
@@ -1,53 +0,0 @@
-#!/bin/bash -
-# libguestfs
-# Copyright (C) 2016 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., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
-
-# Test the icat command.
-
-set -e
-
-if [ -n "$SKIP_TEST_ICAT_SH" ]; then
-    echo "$0: test skipped because environment variable is set."
-    exit 77
-fi
-
-rm -f test-mft.bin
-
-# Skip if TSK is not supported by the appliance.
-if ! guestfish add /dev/null : run : available "sleuthkit"; then
-    echo "$0: skipped because TSK is not available in the appliance"
-    exit 77
-fi
-
-if [ ! -s ../../test-data/phony-guests/windows.img ]; then
-    echo "$0: skipped because windows.img is zero-sized"
-    exit 77
-fi
-
-# download Master File Table ($MFT).
-guestfish --ro -a ../../test-data/phony-guests/windows.img <<EOF
-run
-icat /dev/sda2 0 test-mft.bin
-EOF
-
-# test extracted file is the Master File Table
-if [ `head -c 5 test-mft.bin` != "FILE0" ]; then
-    echo "$0: wrong file extracted."
-    exit 1
-fi
-
-rm -f test-mft.bin
--
2.8.0.rc3

On Tuesday 29 March 2016 23:48:07 Matteo Cafasso wrote:
> Signed-off-by: Matteo Cafasso <noxdafox@gmail.com>
> ---
See general notes sent as
https://www.redhat.com/archives/libguestfs/2016-March/msg00269.html

This patch mixes two things:
a) rename of API
b) coding style changes (and some of them are wrong, see above link)
c) small documentation changes

Please slit this patch in the above list of changes, so it makes
browsing the git history much easier.

Also, please merge patch #2 of this series together with (a), otherwise
bisecting in git history is broken.

Thanks,
-- 
Pino Toscano