Libguestfs - Mar 2017 - [PATCH 2/2] daemon: fix memory leak in xfs implementation of vfs_minimum_size

This way it is possible to cleanup properly structs in the daemon, when
using them within other daemon functions.
---
 .gitignore           |   2 +
 daemon/Makefile.am   |   4 ++
 daemon/daemon.h      |   1 +
 generator/daemon.ml  | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++
 generator/daemon.mli |   2 +
 generator/main.ml    |   4 ++
 6 files changed, 114 insertions(+)

diff --git a/.gitignore b/.gitignore
index 7dd49e2..c82745e 100644
--- a/.gitignore
+++ b/.gitignore
@@ -163,6 +163,8 @@ Makefile.in
 /daemon/optgroups.h
 /daemon/lvm-tokenization.c
 /daemon/stamp-guestfsd.pod
+/daemon/structs-cleanups.c
+/daemon/structs-cleanups.h
 /daemon/stubs-?.c
 /daemon/stubs.h
 /depcomp
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index e3ad053..8632c37 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -22,6 +22,8 @@ generator_built = \
 	dispatch.c \
 	names.c \
 	lvm-tokenization.c \
+	structs-cleanups.c \
+	structs-cleanups.h \
 	stubs-0.c \
 	stubs-1.c \
 	stubs-2.c \
@@ -142,6 +144,8 @@ guestfsd_SOURCES = \
 	stat.c \
 	statvfs.c \
 	strings.c \
+	structs-cleanups.c \
+	structs-cleanups.h \
 	stubs-0.c \
 	stubs-1.c \
 	stubs-2.c \
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 793074d..abec087 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -33,6 +33,7 @@
 #include "guestfs-internal-all.h"
 
 #include "cleanups.h"
+#include "structs-cleanups.h"
 #include "command.h"
 
 /* Mountables */
diff --git a/generator/daemon.ml b/generator/daemon.ml
index 3941d97..9453d12 100644
--- a/generator/daemon.ml
+++ b/generator/daemon.ml
@@ -840,3 +840,104 @@ let generate_daemon_optgroups_h ()    ) optgroups;
 
   pr "#endif /* GUESTFSD_OPTGROUPS_H */\n"
+
+(* Generate structs-cleanups.c file. *)
+and generate_daemon_structs_cleanups_c () +  generate_header CStyle GPLv2plus;
+
+  pr "\
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include \"daemon.h\"
+#include \"guestfs_protocol.h\"
+
+";
+
+  pr "/* Cleanup functions used by CLEANUP_* macros.  Do not call\n";
+  pr " * these functions directly.\n";
+  pr " */\n";
+  pr "\n";
+
+  List.iter (
+    fun { s_name = typ; s_cols = cols } ->
+      pr "void\n";
+      pr "cleanup_free_int_%s (void *ptr)\n" typ;
+      pr "{\n";
+      pr "  struct guestfs_int_%s *x = (* (struct guestfs_int_%s **)
ptr);\n" typ typ;
+      pr "\n";
+      pr "  if (x) {\n";
+      pr "    xdr_free ((xdrproc_t) xdr_guestfs_int_%s, (char *)
x);\n" typ;
+      pr "    free (x);\n";
+      pr "  }\n";
+      pr "}\n";
+      pr "\n";
+
+      pr "void\n";
+      pr "cleanup_free_int_%s_list (void *ptr)\n" typ;
+      pr "{\n";
+      pr "  struct guestfs_int_%s_list *x = (* (struct guestfs_int_%s_list
**) ptr);\n"
+        typ typ;
+      pr "\n";
+      pr "  if (x) {\n";
+      pr "    xdr_free ((xdrproc_t) xdr_guestfs_int_%s_list, (char *)
x);\n"
+        typ;
+      pr "    free (x);\n";
+      pr "  }\n";
+      pr "}\n";
+      pr "\n";
+
+  ) structs
+
+(* Generate structs-cleanups.h file. *)
+and generate_daemon_structs_cleanups_h () +  generate_header CStyle GPLv2plus;
+
+  pr "\
+/* These CLEANUP_* macros automatically free the struct or struct list
+ * pointed to by the local variable at the end of the current scope.
+ */
+
+#ifndef GUESTFS_DAEMON_STRUCTS_CLEANUPS_H_
+#define GUESTFS_DAEMON_STRUCTS_CLEANUPS_H_
+
+#ifdef HAVE_ATTRIBUTE_CLEANUP
+";
+
+  List.iter (
+    fun { s_name = name } ->
+      pr "#define CLEANUP_FREE_%s \\\n" (String.uppercase_ascii
name);
+      pr "  __attribute__((cleanup(cleanup_free_int_%s)))\n" name;
+      pr "#define CLEANUP_FREE_%s_LIST \\\n" (String.uppercase_ascii
name);
+      pr "  __attribute__((cleanup(cleanup_free_int_%s_list)))\n"
name
+  ) structs;
+
+  pr "#else /* !HAVE_ATTRIBUTE_CLEANUP */\n";
+
+  List.iter (
+    fun { s_name = name } ->
+      pr "#define CLEANUP_FREE_%s\n" (String.uppercase_ascii name);
+      pr "#define CLEANUP_FREE_%s_LIST\n" (String.uppercase_ascii
name)
+  ) structs;
+
+  pr "\
+#endif /* !HAVE_ATTRIBUTE_CLEANUP */
+
+/* These functions are used internally by the CLEANUP_* macros.
+ * Don't call them directly.
+ */
+
+";
+
+  List.iter (
+    fun { s_name = name } ->
+      pr "extern void cleanup_free_int_%s (void *ptr);\n"
+        name;
+      pr "extern void cleanup_free_int_%s_list (void *ptr);\n"
+        name
+  ) structs;
+
+  pr "\n";
+  pr "#endif /* GUESTFS_INTERNAL_FRONTEND_CLEANUPS_H_ */\n"
diff --git a/generator/daemon.mli b/generator/daemon.mli
index b7966c9..ff008bf 100644
--- a/generator/daemon.mli
+++ b/generator/daemon.mli
@@ -24,3 +24,5 @@ val generate_daemon_lvm_tokenization : unit -> unit
 val generate_daemon_names : unit -> unit
 val generate_daemon_optgroups_c : unit -> unit
 val generate_daemon_optgroups_h : unit -> unit
+val generate_daemon_structs_cleanups_c : unit -> unit
+val generate_daemon_structs_cleanups_h : unit -> unit
diff --git a/generator/main.ml b/generator/main.ml
index febede5..fe5da5a 100644
--- a/generator/main.ml
+++ b/generator/main.ml
@@ -137,6 +137,10 @@ Run it from the top source directory using the command
             Daemon.generate_daemon_optgroups_h;
   output_to "daemon/lvm-tokenization.c"
             Daemon.generate_daemon_lvm_tokenization;
+  output_to "daemon/structs-cleanups.c"
+            Daemon.generate_daemon_structs_cleanups_c;
+  output_to "daemon/structs-cleanups.h"
+            Daemon.generate_daemon_structs_cleanups_h;
 
   output_to "fish/cmds-gperf.gperf"
             Fish.generate_fish_cmds_gperf;
-- 
2.9.3

Use the proper cleanup handler for deleting the guestfs_int_xfsinfo
struct (so the string fields are deleted too).

Fixes commit f5caa421d1bb826559ec7a6d98c1a6b6b1f0a629.
---
 daemon/xfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/daemon/xfs.c b/daemon/xfs.c
index 7f72e6a..a0d08b2 100644
--- a/daemon/xfs.c
+++ b/daemon/xfs.c
@@ -664,7 +664,7 @@ do_xfs_repair (const char *device,
 int64_t
 xfs_minimum_size (const char *path)
 {
-  CLEANUP_FREE guestfs_int_xfsinfo *info = do_xfs_info (path);
+  CLEANUP_FREE_XFSINFO struct guestfs_int_xfsinfo *info = do_xfs_info (path);
 
   if (info == NULL)
     return -1;
-- 
2.9.3

On Fri, Mar 10, 2017 at 02:01:17PM +0100, Pino Toscano
wrote:
> Use the proper cleanup handler for deleting the guestfs_int_xfsinfo
> struct (so the string fields are deleted too).
> 
> Fixes commit f5caa421d1bb826559ec7a6d98c1a6b6b1f0a629.
> ---
>  daemon/xfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/daemon/xfs.c b/daemon/xfs.c
> index 7f72e6a..a0d08b2 100644
> --- a/daemon/xfs.c
> +++ b/daemon/xfs.c
> @@ -664,7 +664,7 @@ do_xfs_repair (const char *device,
>  int64_t
>  xfs_minimum_size (const char *path)
>  {
> -  CLEANUP_FREE guestfs_int_xfsinfo *info = do_xfs_info (path);
> +  CLEANUP_FREE_XFSINFO struct guestfs_int_xfsinfo *info = do_xfs_info
(path);
>  
>    if (info == NULL)
>      return -1;
> -- 
> 2.9.3
ACK series.

Thanks,

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW