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

On Thu, Jan 24, 2013 at 05:13:11PM +0000, Richard W.M. Jones wrote: > From: "Richard W.M. Jones" <rjones(a)redhat.com&gt; > > 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.