On Tue, Jul 23, 2019 at 04:48:53PM +0100, Daniel P. Berrangé wrote: > Both GCC and CLang support a C extension attribute((cleanup)) which > allows you to define a function that is invoked when a stack variable > exits scope. This typically used to free the memory allocated to it, > though you're not restricted to this. For example it could be used to > unlock a mutex. > > We could use that functionality now, but the syntax is a bit ugly in > plain C. Since version 2.44 of GLib, there have been a few macros to > make it more friendly to use - g_autofree, g_autoptr and > G_DEFINE_AUTOPTR_CLEANUP_FUNC. > > https://developer.gnome.org/glib/stable/glib-Miscellaneous-Macros.html > > https://blogs.gnome.org/desrt/2015/01/30/g_autoptr/ > > The key selling point is that it simplifies the cleanup code paths, > often eliminating the need to goto cleanup labels. This improves > the readability of the code and makes it less likely that you'll > leak memory accidentally. > > Inspired by seeing it added to glib, and used in systemd, Libvirt > finally got around to adopting this in Feb 2019. Overall our > experience with it has been favourable/positive, with the code > simplification being very nice. > > The main caveats with it are > > - Only works with GCC or CLang. We don't care as those are > the only two compilers we declare support for. > > - You must always initialize the variables when declared > to ensure predictable behaviour when they leave scope. > Chances are most methods with goto jumps for cleanup > are doing this already > > - You must not directly return the value that's assigned > to a auto-cleaned variable. You must steal the pointer > in some way. ie > > BAD: > g_autofree char *wibble = g_strdup("wibble") > .... > return wibble; > > GOOD: > g_autofree char *wibble = g_strdup("wibble") > ... > return g_steal_pointer(wibble); > > g_steal_pointer is an inline function which simply copies > the pointer to a new variable, and sets the original variable > to NULL, thus avoiding cleanup. > > I've illustrated the usage by converting a bunch of the crypto code in > QEMU to use auto cleanup. > > Daniel P. Berrangé (3): > glib: bump min required glib library version to 2.48 > crypto: define cleanup functions for use with g_autoptr > crypto: use auto cleanup for many stack variables > > configure | 2 +- > crypto/afsplit.c | 28 ++++---------- > crypto/block-luks.c | 74 +++++++++++-------------------------- > crypto/block.c | 15 +++----- > crypto/hmac-glib.c | 5 --- > crypto/pbkdf.c | 5 +-- > crypto/secret.c | 9 ++--- > crypto/tlscredsanon.c | 16 +++----- > crypto/tlscredspsk.c | 5 +-- > crypto/tlscredsx509.c | 16 +++----- > include/crypto/block.h | 2 + > include/crypto/cipher.h | 2 + > include/crypto/hmac.h | 2 + > include/crypto/ivgen.h | 2 + > include/crypto/tlssession.h | 2 + > include/glib-compat.h | 42 +-------------------- > 16 files changed, 66 insertions(+), 161 deletions(-) > > -- > 2.21.0 > >
Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com>
signature.asc
Description: PGP signature