[Libguestfs] [PATCH 1/2] lib: Add CLEANUP_FREE macro which automatically calls 'free' when leaving scope.

[Daniel P. Berrange]

On Thu, Jan 24, 2013 at 05:13:11PM +0000, Richard W.M. Jones wrote:
> 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, strdup (bar)); /* char *foo = strdup (bar) */
>     ...
>     // no need to call free (foo)!
>   }
> 
> On GCC and LLVM, this is implemented using __attribute__((cleanup(...))).
> 
> On other compilers, we fall back to a less efficient implementation
> which saves up the memory allocations and frees them all when the
> handle is closed.
> ---
>  configure.ac           | 35 +++++++++++++++++++++++++++++++++++
>  src/alloc.c            | 19 +++++++++++++++++++
>  src/guestfs-internal.h | 33 +++++++++++++++++++++++++++++++++
>  src/handle.c           | 15 +++++++++++++++
>  4 files changed, 102 insertions(+)
> 

> +#ifndef HAVE_ATTRIBUTE_CLEANUP
> +void
> +guestfs___defer_free (guestfs_h *g, void (*freefn) (void *), void *data)
> +{
> +  struct deferred_free *p = safe_malloc (g, sizeof *p);
> +
> +  p->freefn = freefn;
> +  p->data = data;
> +  p->next = g->deferred_frees;
> +  g->deferred_frees = p;
> +}
> +#endif

...

>  guestfs_close (guestfs_h *g)
>  {
>    struct qemu_param *qp, *qp_next;
> +#ifndef HAVE_ATTRIBUTE_CLEANUP
> +  struct deferred_free *dfp, *dfp_next;
> +#endif
>    guestfs_h **gg;
>  
>    if (g->state == NO_HANDLE) {
> @@ -324,6 +327,18 @@ guestfs_close (guestfs_h *g)
>    while (g->error_cb_stack)
>      guestfs_pop_error_handler (g);
>  
> +#ifndef HAVE_ATTRIBUTE_CLEANUP
> +  /* For compilers that don't support __attribute__((cleanup(...))),
> +   * free any temporary data that we allocated in CLEANUP_* macros
> +   * here.
> +   */
> +  for (dfp = g->deferred_frees; dfp; dfp = dfp_next) {
> +    dfp->freefn (&dfp->data);
> +    dfp_next = dfp->next;
> +    free (dfp);
> +  }
> +#endif
> +

So if I'm understanding correctly, in the fallback case, this means that
no memory is free'd until guest_close() is invoked. This may work if you
have a fairly short lived  guestfs_h * handle, but if the application
opens a handle and uses it for alot of operations for a very long time
without closing, I think this design is effectively a memory leak, or
least leads to ever increasing memory usage for a long time.

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|