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