Libguestfs - Jun 2015 - Re: [PATCH 1/3] inspection: Add func for merging fs inspections

Add a new guestfs_int_merge_fs_inspections() function that merges the OS
inspection information of two inspect_fs instances into one. This
function is useful if the inspection information for an OS are gathered
by inspecting multiple filesystems.

Signed-off-by: Nikos Skalkotos <skalkoto@grnet.gr>
---
 src/guestfs-internal.h |   1 +
 src/inspect-fs.c       | 115 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 116 insertions(+)

diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 4f06c37..01cbca7 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -788,6 +788,7 @@ extern char *guestfs_int_first_line_of_file (guestfs_h *g,
const char *filename)
 extern int guestfs_int_first_egrep_of_file (guestfs_h *g, const char *filename,
const char *eregex, int iflag, char **ret);
 extern void guestfs_int_check_package_format (guestfs_h *g, struct inspect_fs
*fs);
 extern void guestfs_int_check_package_management (guestfs_h *g, struct
inspect_fs *fs);
+extern int guestfs_int_merge_fs_inspections (guestfs_h *g, struct inspect_fs
*dst, struct inspect_fs *src);
 
 /* inspect-fs-unix.c */
 extern int guestfs_int_check_linux_root (guestfs_h *g, struct inspect_fs *fs);
diff --git a/src/inspect-fs.c b/src/inspect-fs.c
index 932e5e7..5f55f1d 100644
--- a/src/inspect-fs.c
+++ b/src/inspect-fs.c
@@ -655,3 +655,118 @@ guestfs_int_first_egrep_of_file (guestfs_h *g, const char
*filename,
 
   return 1;
 }
+
+/* Merge the missing OS inspection information found on the src inspect_fs into
+ * the ones of the dst inspect_fs. This function is useful if the inspection
+ * information for an OS are gathered by inspecting multiple file systems.
+ *
+ * Returns: 0 = success
+ *          -1 = error
+ */
+int
+guestfs_int_merge_fs_inspections (guestfs_h *g, struct inspect_fs *dst, struct
inspect_fs *src)
+{
+  size_t n, i, old;
+  struct inspect_fstab_entry *fstab = NULL;
+  char ** mappings = NULL;
+
+  if (dst->type == 0)
+    dst->type = src->type;
+
+  if (dst->distro == 0)
+    dst->distro = src->distro;
+
+  if (dst->package_format == 0)
+    dst->package_format = src->package_format;
+
+  if (dst->package_management == 0)
+    dst->package_management = src->package_management;
+
+  if (dst->product_name == NULL) {
+    dst->product_name = src->product_name;
+    src->product_name = NULL;
+  }
+
+  if (dst->product_variant == NULL) {
+    dst->product_variant= src->product_variant;
+    src->product_variant = NULL;
+  }
+
+  if (dst->major_version == 0 && dst->minor_version == 0) {
+    dst->major_version = src->major_version;
+    dst->minor_version = src->minor_version;
+  }
+
+  if (dst->arch == NULL) {
+    dst->arch = src->arch;
+    src->arch = NULL;
+  }
+
+  if (dst->hostname == NULL) {
+    dst->hostname = src->hostname;
+    src->hostname = NULL;
+  }
+
+  if (dst->windows_systemroot == NULL) {
+    dst->windows_systemroot = src->windows_systemroot;
+    src->windows_systemroot = NULL;
+  }
+
+  if (dst->windows_current_control_set == NULL) {
+    dst->windows_current_control_set = src->windows_current_control_set;
+    src->windows_current_control_set = NULL;
+  }
+
+  if (src->drive_mappings != NULL) {
+    if (dst->drive_mappings == NULL) {
+      /* Adopt the drive mappings of src */
+      dst->drive_mappings = src->drive_mappings;
+      src->drive_mappings = NULL;
+    } else {
+      n = 0;
+      for (; dst->drive_mappings[n] != NULL; n++)
+        ;
+      old = n;
+      for (; src->drive_mappings[n] != NULL; n++)
+        ;
+
+      /* Merge the src mappings to dst */
+      mappings = realloc (dst->drive_mappings, (n + 1) * sizeof (char *));
+      if (mappings == NULL) {
+        perrorf (g, "realloc");
+        return -1;
+      }
+
+      for (i = old; i < n; i++)
+        mappings[i] = src->drive_mappings[i - old];
+
+      mappings[n] = NULL;
+      dst->drive_mappings = mappings;
+
+      free(src->drive_mappings);
+      src->drive_mappings = NULL;
+    }
+  }
+
+  if (src->nr_fstab > 0) {
+    n = dst->nr_fstab + src->nr_fstab;
+    fstab = realloc (dst->fstab, n * sizeof (struct inspect_fstab_entry));
+    if (fstab == NULL) {
+      perrorf (g, "realloc");
+      return -1;
+    }
+
+    for (i = 0; i < src->nr_fstab; i++) {
+      fstab[dst->nr_fstab + i].mountable = src->fstab[i].mountable;
+      fstab[dst->nr_fstab + i].mountpoint = src->fstab[i].mountpoint;
+    }
+    free(src->fstab);
+    src->fstab = NULL;
+    src->nr_fstab = 0;
+
+    dst->fstab = fstab;
+    dst->nr_fstab = n;
+  }
+
+  return 0;
+}
-- 
2.1.0

On Fri, May 29, 2015 at 12:24:38PM +0300, Nikos Skalkotos
wrote:
> +      /* Merge the src mappings to dst */
> +      mappings = realloc (dst->drive_mappings, (n + 1) * sizeof (char
*));
> +      if (mappings == NULL) {
> +        perrorf (g, "realloc");
> +        return -1;
> +      }
[...]
> +    n = dst->nr_fstab + src->nr_fstab;
> +    fstab = realloc (dst->fstab, n * sizeof (struct
inspect_fstab_entry));
> +    if (fstab == NULL) {
> +      perrorf (g, "realloc");
> +      return -1;
> +    }
These both leak the original pointers on failure, and also leave the
dst / src structures in a half-merged state.

Since allocation failures are unlikely to be recoverable, just call
`safe_realloc (g, ...)' instead.  It calls the per-handle out of
memory handler (guestfs_set_out_of_memory_handler) on failure.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

On Tue, Jun 02, 2015 at 03:08:44PM +0100, Richard W.M. Jones
wrote:
> On Fri, May 29, 2015 at 12:24:38PM +0300, Nikos Skalkotos wrote:
> > +      /* Merge the src mappings to dst */
> > +      mappings = realloc (dst->drive_mappings, (n + 1) * sizeof
(char *));
> > +      if (mappings == NULL) {
> > +        perrorf (g, "realloc");
> > +        return -1;
> > +      }
> [...]
> > +    n = dst->nr_fstab + src->nr_fstab;
> > +    fstab = realloc (dst->fstab, n * sizeof (struct
inspect_fstab_entry));
> > +    if (fstab == NULL) {
> > +      perrorf (g, "realloc");
> > +      return -1;
> > +    }
> 
> These both leak the original pointers on failure, and also leave the
> dst / src structures in a half-merged state.
> 
> Since allocation failures are unlikely to be recoverable, just call
> `safe_realloc (g, ...)' instead.  It calls the per-handle out of
> memory handler (guestfs_set_out_of_memory_handler) on failure.
And in case that's not clear, you can just ignore the error case when
using safe_realloc.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v