[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libguestfs] [PATCH 0/3] Use __attribute__((cleanup(...)))



On Fri, Jan 25, 2013 at 02:32:15PM +0000, Richard W.M. Jones wrote:
> This patch series changes a small part of the library to use
> __attribute__((cleanup(...))) to automatically free memory when
> pointers go out of the current scope.
> 
> In general terms this seems to be a small win although you do have to
> use it carefully.  For functions where you can completely get rid of
> the "exit code paths", it can simplify things.  For a good example,
> see the 'inspect-icon.c:icon_windows_xp' function before and after the
> change.  For most functions there is no change or a minor
> simplification.
> 
> It can however cause problems if you accidentally put a CLEANUP_FREE
> annotation on a pointer which is actually returned from the function,
> or if you explicitly free something.
> 
> If we make this change, then the library will leak memory when
> compiled with something that isn't GCC/LLVM.  It's likely we have
> other dependencies on GCC, since we've never seriously tried compiling
> the library with any other compiler.

I'm inclined to say that you should make configure exit with a
hard error in the non-GCC/LLVM case. The scope of the memory
leaks will be such that libguestfs is effecitively unusable if
compiled with non-GCC/LLVM.

For most libraries I'd be inclined to say that relying on this
feature is unreasonably restrictive from a portability POV - eg
to MicroSoft compilers for a WIndows builds. For libguestfs though
I think it is probably an acceptable tradeoff, because due to the
requirement for KVM, libguestfs is already basically a Linux only
library, and thus you can guarantee that gcc or llvm are present,
or at least available as options.

Regards,
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 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]