[Libguestfs] [PATCH 2/3] lib: Add CLEANUP_* macros which automatically free things when leaving scope.

[Richard W.M. Jones]

From: "Richard W.M. Jones" <rjones at redhat.com>

Use the macro like this to create temporary variables which are
automatically cleaned up when the scope is exited:

  {
    CLEANUP_FREE char *foo = safe_strdup (bar);
    ...
    // no need to call free (foo)!
  }

The following code is also valid.  The initialization of foo as 'NULL'
prevents any chance of free being called on an uninitialized pointer.
It may not be required in all cases.

  {
    CLEANUP_FREE char *foo = NULL;
    ...
    foo = safe_malloc (100);
    ...
    // no need to call free (foo)!
  }

This is also valid:

  {
    CLEANUP_FREE char *foo = ..., *bar = ...;
    ...
    // no need to call free (foo) or free (bar)!
  }

The CLEANUP_FREE_STRING_LIST macro calls guestfs___free_string_list
on its argument.  The argument may be NULL.

The CLEANUP_HASH_FREE macro calls hash_free on its argument.  The
argument may be NULL.

Important implementation note:
------------------------------

On GCC and LLVM, this is implemented using __attribute__((cleanup(...))).

There is no known way to implement this macro on other C compilers, so
this construct will cause a resource leak.

Important note about close/fclose:
----------------------------------

We did NOT implement 'CLEANUP_CLOSE' or 'CLEANUP_FCLOSE' macros.  The
reason is that I am not convinced that these can be used safely.  It
would be OK to use these to collect file handles along failure paths,
but you would still want a regular call to 'close'/'fclose' since you
must test for errors, and so you end up having to do:

  if (close (fd) == -1) {
    // failure case
    // avoid double-close in cleanup handler:
    fd = -1;
    ...
  }
  // avoid double-close in cleanup handler:
  fd = -1;
  ...
---
 configure.ac           | 42 ++++++++++++++++++++++++++++++++++++++++++
 src/alloc.c            | 27 +++++++++++++++++++++++++++
 src/guestfs-internal.h | 19 +++++++++++++++++++
 3 files changed, 88 insertions(+)

diff --git a/configure.ac b/configure.ac
index 39c79cf..576d5e9 100644
--- a/configure.ac
+++ b/configure.ac
@@ -191,6 +191,48 @@ AC_SYS_LARGEFILE
 dnl Check sizeof long.
 AC_CHECK_SIZEOF([long])
 
+dnl Check if __attribute__((cleanup(...))) works.
+dnl XXX It would be nice to use AC_COMPILE_IFELSE here, but gcc just
+dnl emits a warning for attributes that it doesn't understand.
+AC_MSG_CHECKING([if __attribute__((cleanup(...))) works with this compiler])
+AC_RUN_IFELSE([
+AC_LANG_SOURCE([[
+#include <stdio.h>
+#include <stdlib.h>
+
+void
+freep (void *ptr)
+{
+  exit (0);
+}
+
+void
+test (void)
+{
+  __attribute__((cleanup(freep))) char *ptr = malloc (100);
+}
+
+int
+main (int argc, char *argv[])
+{
+  test ();
+  exit (1);
+}
+]])
+    ],[
+    AC_MSG_RESULT([yes])
+    AC_DEFINE([HAVE_ATTRIBUTE_CLEANUP],[1],[Define to 1 if '__attribute__((cleanup(...)))' works with this compiler.])
+    ],[
+    AC_MSG_WARN(
+['__attribute__((cleanup(...)))' does not work.
+
+You may not be using a sufficiently recent version of GCC or CLANG, or
+you may be using a C compiler which does not support this attribute,
+or the configure test may be wrong.
+
+The code will still compile, but is likely to leak memory and other
+resources when it runs.])])
+
 dnl Check if dirent (readdir) supports d_type member.
 AC_STRUCT_DIRENT_D_TYPE
 
diff --git a/src/alloc.c b/src/alloc.c
index 25d7f42..a15d4f7 100644
--- a/src/alloc.c
+++ b/src/alloc.c
@@ -26,6 +26,8 @@
 #include "guestfs.h"
 #include "guestfs-internal.h"
 
+#include "hash.h"
+
 void *
 guestfs___safe_malloc (guestfs_h *g, size_t nbytes)
 {
@@ -122,3 +124,28 @@ guestfs___safe_asprintf (guestfs_h *g, const char *fs, ...)
 
   return msg;
 }
+
+/* These functions are used internally by the CLEANUP_* macros.
+ * Don't call them directly.
+ */
+
+void
+guestfs___cleanup_free (void *ptr)
+{
+  free (* (void **) ptr);
+}
+
+void
+guestfs___cleanup_free_string_list (void *ptr)
+{
+  guestfs___free_string_list (* (char ***) ptr);
+}
+
+void
+guestfs___cleanup_hash_free (void *ptr)
+{
+  Hash_table *h = * (Hash_table **) ptr;
+
+  if (h)
+    hash_free (h);
+}
diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h
index 870207b..3a6f232 100644
--- a/src/guestfs-internal.h
+++ b/src/guestfs-internal.h
@@ -72,6 +72,18 @@
 #define TRACE4(name, arg1, arg2, arg3, arg4)
 #endif
 
+#ifdef HAVE_ATTRIBUTE_CLEANUP
+#define CLEANUP_FREE __attribute__((cleanup(guestfs___cleanup_free)))
+#define CLEANUP_FREE_STRING_LIST                                \
+  __attribute__((cleanup(guestfs___cleanup_free_string_list)))
+#define CLEANUP_HASH_FREE                               \
+  __attribute__((cleanup(guestfs___cleanup_hash_free)))
+#else
+#define CLEANUP_FREE
+#define CLEANUP_FREE_STRING_LIST
+#define CLEANUP_HASH_FREE
+#endif
+
 #define TMP_TEMPLATE_ON_STACK(g,var)                      \
   char *ttos_tmpdir = guestfs_get_tmpdir (g);             \
   char var[strlen (ttos_tmpdir) + 32];                    \
@@ -474,6 +486,13 @@ extern char *guestfs___safe_asprintf (guestfs_h *g, const char *fs, ...)
 #define safe_memdup guestfs___safe_memdup
 #define safe_asprintf guestfs___safe_asprintf
 
+/* These functions are used internally by the CLEANUP_* macros.
+ * Don't call them directly.
+ */
+extern void guestfs___cleanup_free (void *ptr);
+extern void guestfs___cleanup_free_string_list (void *ptr);
+extern void guestfs___cleanup_hash_free (void *ptr);
+
 /* errors.c */
 extern void guestfs___init_error_handler (guestfs_h *g);
 
-- 
1.8.1