Re: [libvirt] RFC: Use __attribute__ ((cleanup) in libvirt ?

On Mon, Jan 09, 2017 at 04:58:49PM +0000, Daniel P. Berrange wrote: > For those who don't already know, GCC and CLang both implement a C language > extension that enables automatic free'ing of resources when variables go > out of scope. This is done by annotating the variable with the "cleanup" > attribute, pointing to a function the compiler will wire up a call to when > unwinding the stack. Since the annotation points to an arbitrary user > defined function, you're not limited to simple free() like semantics. The > cleanup function could unlock a mutex, or decrement a reference count, etc > > This annotation is used extensively by systemd, and libguestfs, amongst > other projects. This obviously doesn't bring full garbage collection to > C, but it does enable the code to be simplified. By removing the need to > put in many free() (or equiv) calls to cleanup state, the "interesting" > logic in the code stands out more, not being obscured by cleanup calls > and goto jumps. > > I'm wondering what people think of making use of this in libvirt ? > > To my mind the only real reason to *not* use it, would be to maintain > code portability to non-GCC/non-CLang compilers. OS-X, *BSD and *Linux > all use GCC or CLang or both, so its a non-issue there. So the only place > this could cause pain is people building libvirt on Win32, who are using > the Microsoft compilers instead og GCC. > > IMHO, it is perfectly valid for us to declare that MSVC is unsupported > with Libvirt and users must use GCC to build on Windows, either natively > via cygwin, or cross-build from Linux hosts. >From the libguestfs POV it's absolutely been a great thing. Of course we also don't care about MSVC, but that is a possible concern for libvirt as you say. I wonder if MSVC offers some non-standard support for this? As it's really a C++ compiler, maybe it's possible to use that? There are many many examples of how this simplifies code in libguestfs. Even very complex functions can often be written with no "exit path" at all. I'll just point to a few: https://github.com/libguestfs/libguestfs/blob/master/src/inspect-fs.c#L34... https://github.com/libguestfs/libguestfs/blob/master/src/launch-libvirt.c... https://github.com/libguestfs/libguestfs/blob/master/src/appliance-kcmdli... https://github.com/libguestfs/libguestfs/blob/master/src/filearch.c#L131-... BTW you can use it for a lot more than just free(). We also have macros for deleting temporary files and cleaning up complex objects. Now it's also worth saying there are some catches: (1) You must not CLEANUP_FREE any objects which you will return from the function. This often means you still need to explicitly free such objects on error return paths. (2) You must not write code like: fn () { CLEANUP_FREE char *v; // uninitialized if (some error condition) { return -1; } ... } because that will call free (v) on the uninitialized variable. Sometimes GCC can spot this. In libguestfs we tend to initialize every CLEANUP_* variable to either an explicit value or NULL. GCC optimizes away calls to free (NULL).