Hi,

some of the comments for patch #3 apply also for this one, namely:
- wrapping of commit message
- indentation of forward declarations
- usage of XDR deserialization from guestfs_protocol.h
-

On Sunday 03 April 2016 16:30:49 Matteo Cafasso wrote:
> The filesystem_walk command is the appliance's
> counterpart of the daemon's
> internal_filesystem_walk command.
>
> It writes the daemon's command output
> on a temporary file and parses it, deserialising
> the XDR formatted tsk_dirent structs.
>
> It returns to the caller the list
> of tsk_dirent structs generated by the
> internal_filesystem_walk command.
>
> Signed-off-by: Matteo Cafasso <noxdafox@gmail.com>
> ---
>  generator/actions.ml |  69 ++++++++++++++++++++++
>  src/Makefile.am      |   1 +
>  src/tsk.c            | 162 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 232 insertions(+)
>  create mode 100644 src/tsk.c
>
> diff --git a/generator/actions.ml b/generator/actions.ml
> index 449ffa0..9457c3f 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -3546,6 +3546,75 @@ The environment variable C<XDG_RUNTIME_DIR> controls the default
>  value: If C<XDG_RUNTIME_DIR> is set, then that is the default.
>  Else F</tmp> is the default." };
>
> +  { defaults with
> +    name = "filesystem_walk"; added = (1, 33, 17);
> +    style = RStructList ("dirents", "tsk_dirent"), [Mountable "device";], [];
> +    optional = Some "libtsk";
> +    progress = true; cancellable = true;
> +    shortdesc = "walk through the filesystem content";
> +    longdesc = "\
> +Walk through the internal structures of a disk partition
> +(eg. F</dev/sda1>) in order to return a list of all the files
> +and directories stored within.
> +
> +It is not necessary to mount the disk partition to run this command.
> +
> +All entries in the filesystem are returned, excluding C<.> and
> +C<..>. This function can list deleted or unaccessible files.
> +The entries are I<not> sorted.
> +
> +If the entry is not allocated (ex: it has been deleted),
> +its inode, type or size might not be recovered correctly.
> +In such case, the inode and the size will be 0 while the type
> +will be unidentified 'u'.

As said for patch #3, unknown sizes should be -1 and not 0.

> +This call returns as well basic file type information about each
> +file.  The C<tsk_type> field will contain one of the following characters:
> +
> +=over 4
> +
> +=item 'b'
> +
> +Block special
> +
> +=item 'c'
> +
> +Char special
> +
> +=item 'd'
> +
> +Directory
> +
> +=item 'f'
> +
> +FIFO (named pipe)
> +
> +=item 'l'
> +
> +Symbolic link
> +
> +=item 'r'
> +
> +Regular file
> +
> +=item 's'
> +
> +Socket
> +
> +=item 'h'
> +
> +Shadow inode (Solaris)
> +
> +=item 'w'
> +
> +Whiteout inode (BSD)
> +
> +=item 'u'
> +
> +Unknown file type
> +
> +=back" };
> +
>  ]
>
>  (* daemon_functions are any functions which cause some action
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 3b4cd10..9f8af4c 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -130,6 +130,7 @@ libguestfs_la_SOURCES = \
>       structs-copy.c \
>       structs-free.c \
>       tmpdirs.c \
> +     tsk.c \
>       whole-file.c \
>       libguestfs.syms
>
> diff --git a/src/tsk.c b/src/tsk.c
> new file mode 100644
> index 0000000..d77bc0a
> --- /dev/null
> +++ b/src/tsk.c
> @@ -0,0 +1,162 @@
> +/* libguestfs
> + * Copyright (C) 2016 Red Hat Inc.
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +#include <config.h>
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <string.h>
> +#include <rpc/xdr.h>
> +#include <rpc/types.h>
> +
> +#include "full-read.h"
> +
> +#include "guestfs.h"
> +#include "guestfs_protocol.h"
> +#include "guestfs-internal.h"
> +#include "guestfs-internal-all.h"
> +#include "guestfs-internal-actions.h"
> +
> +static struct guestfs_tsk_dirent_list *parse_filesystem_walk
> +(guestfs_h *g, char *buf, size_t bufsize);
> +int deserialise_dirent_list (guestfs_h *g, char *buf, size_t bufsize,
> +                             struct guestfs_tsk_dirent_list **dirents);
> +static int deserialise_dirent (guestfs_h *g, XDR *xdrs,
> +                               struct guestfs_tsk_dirent *dirent);
> +
> +struct guestfs_tsk_dirent_list *
> +guestfs_impl_filesystem_walk (guestfs_h *g, const char *mountable)
> +{
> +  int ret = 0;
> +  size_t size = 0;
> +  CLEANUP_FREE char *buf = NULL;
> +  CLEANUP_UNLINK_FREE char *tmpfile = NULL;
> +
> +  ret = guestfs_int_lazy_make_tmpdir (g);
> +  if (ret < 0)
> +    return NULL;
> +
> +  tmpfile = safe_asprintf (g, "%s/filesystem_walk%d", g->tmpdir, ++g->unique);
> +
> +  ret = guestfs_internal_filesystem_walk (g, mountable, tmpfile);
> +  if (ret < 0)
> +    return NULL;
> +
> +  ret = guestfs_int_read_whole_file (g, tmpfile, &buf, &size);
> +  if (ret < 0)
> +    return NULL;
> +
> +  return parse_filesystem_walk (g, buf, size);  /* caller frees */
> +}
> +
> +/* Parse buf content and return dirents list.
> + * Return a list of tsk_dirent on success, NULL on error.
> + */
> +static struct guestfs_tsk_dirent_list *
> +parse_filesystem_walk (guestfs_h *g, char *buf, size_t bufsize)
> +{
> +  int ret = 0;
> +  struct guestfs_tsk_dirent_list *dirents = NULL;
> +
> +  /* Initialise results array. */
> +  dirents = safe_malloc (g, sizeof (struct guestfs_tsk_dirent_list));
> +  dirents->len = 8;
> +  dirents->val = safe_malloc (g, dirents->len *
> +                              sizeof (struct guestfs_tsk_dirent));

sizeof (*dirents->val) (and below too) can make the code slightly
shorter (just an hint).

> +
> +  /* Deserialise buffer into dirent list. */
> +  ret = deserialise_dirent_list (g, buf, bufsize, &dirents);
> +  if (ret < 0) {
> +    guestfs_free_tsk_dirent_list (dirents);
> +    return NULL;
> +  }
> +
> +  /* Resize the array to correct number of entries. */
> +  dirents->len = ret;
> +  dirents->val = safe_realloc (g, dirents->val,
> +                               dirents->len *
> +                               sizeof (struct guestfs_tsk_dirent));

You don't need to shrink the array with values -- users knows already
they don't have to access it past 'len' values.

> +
> +  return dirents;
> +}
> +
> +/* Deserialise buf content and populate the dirent list.
> + * Return the number of deserialised dirents, -1 on error.
> + */
> +int
> +deserialise_dirent_list (guestfs_h *g, char *buf, size_t bufsize,
> +                         struct guestfs_tsk_dirent_list **dirents)

Since you don't change the location of 'dirents', you don't need to
pass a pointer to it to change its values.

Thanks,
--
Pino Toscano
_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs