Skip to content
This repository

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse code

lib: Add CLEANUP_* macros which automatically free things when leavin…

…g scope.

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;
  ...
  • Loading branch information...
commit 98b64650c852ccc9a8eef8b9691052faeb4873c8 1 parent acb85da
rwmjones rwmjones authored

Showing 3 changed files with 88 additions and 0 deletions. Show diff stats Hide diff stats

  1. +42 0 configure.ac
  2. +27 0 src/alloc.c
  3. +19 0 src/guestfs-internal.h
42 configure.ac
@@ -191,6 +191,48 @@ AC_SYS_LARGEFILE
191 191 dnl Check sizeof long.
192 192 AC_CHECK_SIZEOF([long])
193 193
  194 +dnl Check if __attribute__((cleanup(...))) works.
  195 +dnl XXX It would be nice to use AC_COMPILE_IFELSE here, but gcc just
  196 +dnl emits a warning for attributes that it doesn't understand.
  197 +AC_MSG_CHECKING([if __attribute__((cleanup(...))) works with this compiler])
  198 +AC_RUN_IFELSE([
  199 +AC_LANG_SOURCE([[
  200 +#include <stdio.h>
  201 +#include <stdlib.h>
  202 +
  203 +void
  204 +freep (void *ptr)
  205 +{
  206 + exit (0);
  207 +}
  208 +
  209 +void
  210 +test (void)
  211 +{
  212 + __attribute__((cleanup(freep))) char *ptr = malloc (100);
  213 +}
  214 +
  215 +int
  216 +main (int argc, char *argv[])
  217 +{
  218 + test ();
  219 + exit (1);
  220 +}
  221 +]])
  222 + ],[
  223 + AC_MSG_RESULT([yes])
  224 + AC_DEFINE([HAVE_ATTRIBUTE_CLEANUP],[1],[Define to 1 if '__attribute__((cleanup(...)))' works with this compiler.])
  225 + ],[
  226 + AC_MSG_WARN(
  227 +['__attribute__((cleanup(...)))' does not work.
  228 +
  229 +You may not be using a sufficiently recent version of GCC or CLANG, or
  230 +you may be using a C compiler which does not support this attribute,
  231 +or the configure test may be wrong.
  232 +
  233 +The code will still compile, but is likely to leak memory and other
  234 +resources when it runs.])])
  235 +
194 236 dnl Check if dirent (readdir) supports d_type member.
195 237 AC_STRUCT_DIRENT_D_TYPE
196 238
27 src/alloc.c
@@ -26,6 +26,8 @@
26 26 #include "guestfs.h"
27 27 #include "guestfs-internal.h"
28 28
  29 +#include "hash.h"
  30 +
29 31 void *
30 32 guestfs___safe_malloc (guestfs_h *g, size_t nbytes)
31 33 {
@@ -122,3 +124,28 @@ guestfs___safe_asprintf (guestfs_h *g, const char *fs, ...)
122 124
123 125 return msg;
124 126 }
  127 +
  128 +/* These functions are used internally by the CLEANUP_* macros.
  129 + * Don't call them directly.
  130 + */
  131 +
  132 +void
  133 +guestfs___cleanup_free (void *ptr)
  134 +{
  135 + free (* (void **) ptr);
  136 +}
  137 +
  138 +void
  139 +guestfs___cleanup_free_string_list (void *ptr)
  140 +{
  141 + guestfs___free_string_list (* (char ***) ptr);
  142 +}
  143 +
  144 +void
  145 +guestfs___cleanup_hash_free (void *ptr)
  146 +{
  147 + Hash_table *h = * (Hash_table **) ptr;
  148 +
  149 + if (h)
  150 + hash_free (h);
  151 +}
19 src/guestfs-internal.h
@@ -72,6 +72,18 @@
72 72 #define TRACE4(name, arg1, arg2, arg3, arg4)
73 73 #endif
74 74
  75 +#ifdef HAVE_ATTRIBUTE_CLEANUP
  76 +#define CLEANUP_FREE __attribute__((cleanup(guestfs___cleanup_free)))
  77 +#define CLEANUP_FREE_STRING_LIST \
  78 + __attribute__((cleanup(guestfs___cleanup_free_string_list)))
  79 +#define CLEANUP_HASH_FREE \
  80 + __attribute__((cleanup(guestfs___cleanup_hash_free)))
  81 +#else
  82 +#define CLEANUP_FREE
  83 +#define CLEANUP_FREE_STRING_LIST
  84 +#define CLEANUP_HASH_FREE
  85 +#endif
  86 +
75 87 #define TMP_TEMPLATE_ON_STACK(g,var) \
76 88 char *ttos_tmpdir = guestfs_get_tmpdir (g); \
77 89 char var[strlen (ttos_tmpdir) + 32]; \
@@ -474,6 +486,13 @@ extern char *guestfs___safe_asprintf (guestfs_h *g, const char *fs, ...)
474 486 #define safe_memdup guestfs___safe_memdup
475 487 #define safe_asprintf guestfs___safe_asprintf
476 488
  489 +/* These functions are used internally by the CLEANUP_* macros.
  490 + * Don't call them directly.
  491 + */
  492 +extern void guestfs___cleanup_free (void *ptr);
  493 +extern void guestfs___cleanup_free_string_list (void *ptr);
  494 +extern void guestfs___cleanup_hash_free (void *ptr);
  495 +
477 496 /* errors.c */
478 497 extern void guestfs___init_error_handler (guestfs_h *g);
479 498

0 comments on commit 98b6465

Please sign in to comment.
Something went wrong with that request. Please try again.