Re: [Qemu-devel] [PATCH for 4.2 0/3] require newer glib2 to enable autofree'ing of stack variables exiting scope

[Stefan Hajnoczi]

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 <address@hidden>

signature.asc
Description: PGP signature