Libguestfs - Jul 2014 - [PATCH 1/3] ext2: create a struct for the OCaml 't' type

Use an helper struct for holding the ext2_filsys variable, so that can
be used to add more data.
---
 src/ext2fs-c.c | 77 +++++++++++++++++++++++++++++++---------------------------
 1 file changed, 41 insertions(+), 36 deletions(-)

diff --git a/src/ext2fs-c.c b/src/ext2fs-c.c
index 70755c9..8eab24c 100644
--- a/src/ext2fs-c.c
+++ b/src/ext2fs-c.c
@@ -52,6 +52,11 @@
 /* fts.h in glibc is broken, forcing us to use the GNUlib alternative. */
 #include "fts_.h"
 
+struct ext2_data
+{
+  ext2_filsys fs;
+};
+
 static void initialize (void) __attribute__((constructor));
 
 static void
@@ -78,18 +83,18 @@ ext2_handle_closed (void)
   caml_failwith ("ext2fs: function called on a closed handle");
 }
 
-#define Ext2fs_val(v) (*((ext2_filsys *)Data_custom_val(v)))
+#define Ext2fs_val(v) (*((struct ext2_data *)Data_custom_val(v)))
 
 static void
 ext2_finalize (value fsv)
 {
-  ext2_filsys fs = Ext2fs_val (fsv);
+  struct ext2_data data = Ext2fs_val (fsv);
 
-  if (fs) {
+  if (data.fs) {
 #ifdef HAVE_EXT2FS_CLOSE2
-    ext2fs_close2 (fs, EXT2_FLAG_FLUSH_NO_SYNC);
+    ext2fs_close2 (data.fs, EXT2_FLAG_FLUSH_NO_SYNC);
 #else
-    ext2fs_close (fs);
+    ext2fs_close (data.fs);
 #endif
   }
 }
@@ -104,14 +109,14 @@ static struct custom_operations ext2_custom_operations = {
 };
 
 static value
-Val_ext2fs (ext2_filsys fs)
+Val_ext2fs (struct ext2_data *data)
 {
   CAMLparam0 ();
   CAMLlocal1 (fsv);
 
   fsv = caml_alloc_custom (&ext2_custom_operations,
-                           sizeof (ext2_filsys), 0, 1);
-  Ext2fs_val (fsv) = fs;
+                           sizeof (struct ext2_data), 0, 1);
+  Ext2fs_val (fsv) = *data;
   CAMLreturn (fsv);
 }
 
@@ -122,18 +127,18 @@ supermin_ext2fs_open (value filev)
   CAMLlocal1 (fsv);
   int fs_flags = EXT2_FLAG_RW;
   errcode_t err;
-  ext2_filsys fs;
+  struct ext2_data data;
 
 #ifdef EXT2_FLAG_64BITS
   fs_flags |= EXT2_FLAG_64BITS;
 #endif
 
   err = ext2fs_open (String_val (filev), fs_flags, 0, 0,
-                     unix_io_manager, &fs);
+                     unix_io_manager, &data.fs);
   if (err != 0)
     ext2_error_to_exception ("ext2fs_open", err, String_val (filev));
 
-  fsv = Val_ext2fs (fs);
+  fsv = Val_ext2fs (&data);
   CAMLreturn (fsv);
 }
 
@@ -145,7 +150,7 @@ supermin_ext2fs_close (value fsv)
   ext2_finalize (fsv);
 
   /* So we don't double-free in the finalizer. */
-  Ext2fs_val (fsv) = NULL;
+  Ext2fs_val (fsv).fs = NULL;
 
   CAMLreturn (Val_unit);
 }
@@ -154,14 +159,14 @@ value
 supermin_ext2fs_read_bitmaps (value fsv)
 {
   CAMLparam1 (fsv);
-  ext2_filsys fs;
+  struct ext2_data data;
   errcode_t err;
 
-  fs = Ext2fs_val (fsv);
-  if (fs == NULL)
+  data = Ext2fs_val (fsv);
+  if (data.fs == NULL)
     ext2_handle_closed ();
 
-  err = ext2fs_read_bitmaps (fs);
+  err = ext2fs_read_bitmaps (data.fs);
   if (err != 0)
     ext2_error_to_exception ("ext2fs_read_bitmaps", err, NULL);
 
@@ -173,7 +178,7 @@ static void ext2_empty_inode (ext2_filsys fs, ext2_ino_t
dir_ino, const char *di
 static void ext2_write_file (ext2_filsys fs, ext2_ino_t ino, const char *buf,
size_t size, const char *filename);
 static void ext2_link (ext2_filsys fs, ext2_ino_t dir_ino, const char
*basename, ext2_ino_t ino, int dir_ft);
 static void ext2_clean_path (ext2_filsys fs, ext2_ino_t dir_ino, const char
*dirname, const char *basename, int isdir);
-static void ext2_copy_file (ext2_filsys fs, const char *src, const char *dest);
+static void ext2_copy_file (struct ext2_data *data, const char *src, const char
*dest);
 
 /* Copy the host filesystem file/directory 'src' to the destination
  * 'dest'.  Directories are NOT copied recursively - the directory is
@@ -185,13 +190,13 @@ supermin_ext2fs_copy_file_from_host (value fsv, value
srcv, value destv)
   CAMLparam3 (fsv, srcv, destv);
   const char *src = String_val (srcv);
   const char *dest = String_val (destv);
-  ext2_filsys fs;
+  struct ext2_data data;
 
-  fs = Ext2fs_val (fsv);
-  if (fs == NULL)
+  data = Ext2fs_val (fsv);
+  if (data.fs == NULL)
     ext2_handle_closed ();
 
-  ext2_copy_file (fs, src, dest);
+  ext2_copy_file (&data, src, dest);
 
   CAMLreturn (Val_unit);
 }
@@ -207,7 +212,7 @@ supermin_ext2fs_copy_dir_recursively_from_host (value fsv,
   const char *srcdir = String_val (srcdirv);
   const char *destdir = String_val (destdirv);
   size_t srclen = strlen (srcdir);
-  ext2_filsys fs;
+  struct ext2_data data;
   char *paths[2];
   FTS *fts;
   FTSENT *entry;
@@ -216,8 +221,8 @@ supermin_ext2fs_copy_dir_recursively_from_host (value fsv,
   size_t i, n;
   int r;
 
-  fs = Ext2fs_val (fsv);
-  if (fs == NULL)
+  data = Ext2fs_val (fsv);
+  if (data.fs == NULL)
     ext2_handle_closed ();
 
   paths[0] = (char *) srcdir;
@@ -269,7 +274,7 @@ supermin_ext2fs_copy_dir_recursively_from_host (value fsv,
       }
     }
 
-    ext2_copy_file (fs, entry->fts_path, destpath);
+    ext2_copy_file (&data, entry->fts_path, destpath);
     free (destpath);
   }
 
@@ -538,7 +543,7 @@ read_whole_file (const char *filename, size_t size)
 
 /* Copy a file (or directory etc) from the host. */
 static void
-ext2_copy_file (ext2_filsys fs, const char *src, const char *dest)
+ext2_copy_file (struct ext2_data *data, const char *src, const char *dest)
 {
   errcode_t err;
   struct stat statbuf;
@@ -551,13 +556,13 @@ ext2_copy_file (ext2_filsys fs, const char *src, const
char *dest)
    * Note we cheat by looking at fs->device_name (which is the output
    * file).  We could store this filename separately.
    */
-  if (fs->device_name && statvfs (fs->device_name,
&statvfsbuf) == 0) {
+  if (data->fs->device_name && statvfs
(data->fs->device_name, &statvfsbuf) == 0) {
     uint64_t space = statvfsbuf.f_bavail * statvfsbuf.f_bsize;
     uint64_t estimate = 128*1024 + 2 * statbuf.st_size;
 
     if (space < estimate)
       unix_error (ENOSPC, (char *) "statvfs",
-                  caml_copy_string (fs->device_name));
+                  caml_copy_string (data->fs->device_name));
   }
 
 #if 0
@@ -638,7 +643,7 @@ ext2_copy_file (ext2_filsys fs, const char *src, const char
*dest)
   cont:
 
     /* Look up the parent directory. */
-    err = ext2fs_namei (fs, EXT2_ROOT_INO, EXT2_ROOT_INO, dirname,
&dir_ino);
+    err = ext2fs_namei (data->fs, EXT2_ROOT_INO, EXT2_ROOT_INO, dirname,
&dir_ino);
     if (err != 0) {
       /* This is the most popular supermin "WTF" error, so make
        * sure we capture as much information as possible.
@@ -654,7 +659,7 @@ ext2_copy_file (ext2_filsys fs, const char *src, const char
*dest)
     }
   }
 
-  ext2_clean_path (fs, dir_ino, dirname, basename, S_ISDIR (statbuf.st_mode));
+  ext2_clean_path (data->fs, dir_ino, dirname, basename, S_ISDIR
(statbuf.st_mode));
 
   int dir_ft;
 
@@ -670,13 +675,13 @@ ext2_copy_file (ext2_filsys fs, const char *src, const
char *dest)
         goto skip_unreadable_file;
     }
 
-    ext2_empty_inode (fs, dir_ino, dirname, basename,
+    ext2_empty_inode (data->fs, dir_ino, dirname, basename,
                       statbuf.st_mode, statbuf.st_uid, statbuf.st_gid,
                       statbuf.st_ctime, statbuf.st_atime, statbuf.st_mtime,
                       0, 0, EXT2_FT_REG_FILE, &ino);
 
     if (statbuf.st_size > 0) {
-      ext2_write_file (fs, ino, buf, statbuf.st_size, dest);
+      ext2_write_file (data->fs, ino, buf, statbuf.st_size, dest);
       free (buf);
     }
   skip_unreadable_file: ;
@@ -684,7 +689,7 @@ ext2_copy_file (ext2_filsys fs, const char *src, const char
*dest)
   /* Create a symlink. */
   else if (S_ISLNK (statbuf.st_mode)) {
     ext2_ino_t ino;
-    ext2_empty_inode (fs, dir_ino, dirname, basename,
+    ext2_empty_inode (data->fs, dir_ino, dirname, basename,
                       statbuf.st_mode, statbuf.st_uid, statbuf.st_gid,
                       statbuf.st_ctime, statbuf.st_atime, statbuf.st_mtime,
                       0, 0, EXT2_FT_SYMLINK, &ino);
@@ -693,11 +698,11 @@ ext2_copy_file (ext2_filsys fs, const char *src, const
char *dest)
     ssize_t r = readlink (src, buf, sizeof buf);
     if (r == -1)
       unix_error (errno, (char *) "readlink", caml_copy_string
(src));
-    ext2_write_file (fs, ino, buf, r, dest);
+    ext2_write_file (data->fs, ino, buf, r, dest);
   }
   /* Create directory. */
   else if (S_ISDIR (statbuf.st_mode))
-    ext2_mkdir (fs, dir_ino, dirname, basename,
+    ext2_mkdir (data->fs, dir_ino, dirname, basename,
                 statbuf.st_mode, statbuf.st_uid, statbuf.st_gid,
                 statbuf.st_ctime, statbuf.st_atime, statbuf.st_mtime);
   /* Create a special file. */
@@ -714,7 +719,7 @@ ext2_copy_file (ext2_filsys fs, const char *src, const char
*dest)
   } else if (S_ISSOCK (statbuf.st_mode)) {
     dir_ft = EXT2_FT_SOCK;
   make_special:
-    ext2_empty_inode (fs, dir_ino, dirname, basename,
+    ext2_empty_inode (data->fs, dir_ino, dirname, basename,
                       statbuf.st_mode, statbuf.st_uid, statbuf.st_gid,
                       statbuf.st_ctime, statbuf.st_atime, statbuf.st_mtime,
                       major (statbuf.st_rdev), minor (statbuf.st_rdev),
-- 
1.9.3

On Wed, Jul 30, 2014 at 05:36:26PM +0200, Pino Toscano
wrote:
> Use an helper struct for holding the ext2_filsys variable, so that can
> be used to add more data.
ACK.

I have pushed all three patches in this series, thanks.

I noticed that we don't enable any gcc compiler warnings -- oops!
I'm going to turn those on and see what bad stuff happens.

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 Wed, Jul 30, 2014 at 06:06:37PM +0100, Richard W.M. Jones
wrote:
> On Wed, Jul 30, 2014 at 05:36:26PM +0200, Pino Toscano wrote:
> > Use an helper struct for holding the ext2_filsys variable, so that can
> > be used to add more data.
> 
> ACK.
> 
> I have pushed all three patches in this series, thanks.
> 
> I noticed that we don't enable any gcc compiler warnings -- oops!
> I'm going to turn those on and see what bad stuff happens.
-Wall shows nothing as it turns out, although -Wextra does show some
possible problems.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html