util: rewrite auto cleanup macros to use glib's equivalent

To facilitate porting over to glib, this rewrites the auto cleanup
macros to use glib's equivalent.

As a result it is now possible to use g_autoptr/VIR_AUTOPTR, and
g_auto/VIR_AUTOCLEAN, g_autofree/VIR_AUTOFREE interchangably, regardless
of which macros were used to declare the cleanup types.

Within the scope of any single method, code must remain consistent
using either GLib or Libvirt macros, never mixing both. New code
must preferentially use the GLib macros, and old code will be
converted incrementally.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrangé 2019-10-01 10:52:28 +01:00
parent 74d9326795
commit 44e7f02915
5 changed files with 64 additions and 20 deletions

View File

@ -1128,7 +1128,7 @@ sc_prohibit_backslash_alignment:
# Rule to ensure that variables declared using a cleanup macro are # Rule to ensure that variables declared using a cleanup macro are
# always initialized. # always initialized.
sc_require_attribute_cleanup_initialization: sc_require_attribute_cleanup_initialization:
@prohibit='VIR_AUTO((FREE|PTR|UNREF|CLEAN)\(.+\)|CLOSE|STRINGLIST) *[^=]+;' \ @prohibit='((g_auto(ptr|free)?)|(VIR_AUTO((FREE|PTR|UNREF|CLEAN)\(.+\)|CLOSE|STRINGLIST))) *[^=]+;' \
in_vc_files='\.[chx]$$' \ in_vc_files='\.[chx]$$' \
halt='variable declared with a cleanup macro must be initialized' \ halt='variable declared with a cleanup macro must be initialized' \
$(_sc_search_regexp) $(_sc_search_regexp)

View File

@ -1028,6 +1028,24 @@ BAD:
<dd>The GLib APIs g_strdup_printf / g_strdup_vprint should be used <dd>The GLib APIs g_strdup_printf / g_strdup_vprint should be used
instead. Don't use g_vasprintf unless having the string length instead. Don't use g_vasprintf unless having the string length
returned is unavoidable.</dd> returned is unavoidable.</dd>
<dt>VIR_AUTOPTR, VIR_AUTOCLEAN, VIR_AUTOFREE</dt>
<dd>The GLib macros g_autoptr, g_auto and g_autofree must be used
instead in all new code. In existing code, the GLib macros must
never be mixed with libvirt macros within a method, nor should
they be mixed with VIR_FREE. If introducing GLib macros to an
existing method, any use of libvirt macros must be converted
in an independent commit.
</dd>
<dt>VIR_DEFINE_AUTOPTR_FUNC, VIR_DEFINE_AUTOCLEAN_FUNC</dt>
<dd>The GLib macros G_DEFINE_AUTOPTR_CLEANUP_FUNC and
G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC must be used in all
new code. Existing code should be converted to the
new macros where relevant. It is permissible to use
g_autoptr, g_auto on an object whose cleanup function
is declared with the libvirt macros and vice-verca.
</dd>
</dl> </dl>
<h2><a id="file_handling">File handling</a></h2> <h2><a id="file_handling">File handling</a></h2>

View File

@ -104,6 +104,20 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
dontwarn="$dontwarn -Wdouble-promotion" dontwarn="$dontwarn -Wdouble-promotion"
fi fi
# Clang complains about unused static inline functions
# which are common with G_DEFINE_AUTOPTR_CLEANUP_FUNC
AC_CACHE_CHECK([whether clang gives bogus warnings for -Wunused-function],
[lv_cv_clang_unused_function_broken], [
save_CFLAGS="$CFLAGS"
CFLAGS="-Wunused-function -Werror"
AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[
static inline void foo(void) {}
]], [[
return 0]])],
[lv_cv_clang_unused_function_broken=no],
[lv_cv_clang_unused_function_broken=yes])
CFLAGS="$save_CFLAGS"])
# We might fundamentally need some of these disabled forever, but # We might fundamentally need some of these disabled forever, but
# ideally we'd turn many of them on # ideally we'd turn many of them on
dontwarn="$dontwarn -Wfloat-equal" dontwarn="$dontwarn -Wfloat-equal"
@ -119,6 +133,13 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[
# Remove the ones we don't want, blacklisted earlier # Remove the ones we don't want, blacklisted earlier
gl_MANYWARN_COMPLEMENT([wantwarn], [$maybewarn], [$dontwarn]) gl_MANYWARN_COMPLEMENT([wantwarn], [$maybewarn], [$dontwarn])
# -Wunused-functin is implied by -Wall we must turn it
# off explicitly.
if test "$lv_cv_clang_unused_function_broken" = "yes";
then
wantwarn="$wantwarn -Wno-unused-function"
fi
# GNULIB uses '-W' (aka -Wextra) which includes a bunch of stuff. # GNULIB uses '-W' (aka -Wextra) which includes a bunch of stuff.
# Unfortunately, this means you can't simply use '-Wsign-compare' # Unfortunately, this means you can't simply use '-Wsign-compare'
# with gl_MANYWARN_COMPLEMENT # with gl_MANYWARN_COMPLEMENT

View File

@ -494,8 +494,11 @@ void virDisposeString(char **strptr)
* VIR_AUTOFREE: * VIR_AUTOFREE:
* @type: type of the variable to be freed automatically * @type: type of the variable to be freed automatically
* *
* DEPRECATED: use g_autofree for new code. See HACKING
* for further guidance.
*
* Macro to automatically free the memory allocated to * Macro to automatically free the memory allocated to
* the variable declared with it by calling virFree * the variable declared with it by calling virFree
* when the variable goes out of scope. * when the variable goes out of scope.
*/ */
#define VIR_AUTOFREE(type) __attribute__((cleanup(virFree))) type #define VIR_AUTOFREE(type) g_autofree type

View File

@ -20,7 +20,21 @@
#pragma once #pragma once
#define VIR_AUTOPTR_FUNC_NAME(type) type##AutoPtrFree /**
* DEPRECATION WARNING
*
* The macros in this file should not be used in newly written code.
* Use the equivalent GLib macros instead.
*
* For existing code, use of the libvirt and GLib macros must NEVER
* be mixed within a single method.
*
* The use of the libvirt VIR_FREE macros should also not be mixed
* with GLib auto-free macros and vice-verca.
*
* Existing code should be converted to the new GLib macros and
* g_free APIs as needed.
*/
/** /**
* VIR_DEFINE_AUTOPTR_FUNC: * VIR_DEFINE_AUTOPTR_FUNC:
@ -31,15 +45,8 @@
* resources allocated to a variable of type @type. This newly * resources allocated to a variable of type @type. This newly
* defined function works as a necessary wrapper around @func. * defined function works as a necessary wrapper around @func.
*/ */
#define VIR_DEFINE_AUTOPTR_FUNC(type, func) \ #define VIR_DEFINE_AUTOPTR_FUNC(t, f) \
static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \ G_DEFINE_AUTOPTR_CLEANUP_FUNC(t, f)
{ \
if (*_ptr) \
(func)(*_ptr); \
*_ptr = NULL; \
}
#define VIR_AUTOCLEAN_FUNC_NAME(type) type##AutoClean
/** /**
* VIR_DEFINE_AUTOCLEAN_FUNC: * VIR_DEFINE_AUTOCLEAN_FUNC:
@ -51,10 +58,7 @@
* take pointer to @type. * take pointer to @type.
*/ */
#define VIR_DEFINE_AUTOCLEAN_FUNC(type, func) \ #define VIR_DEFINE_AUTOCLEAN_FUNC(type, func) \
static inline void VIR_AUTOCLEAN_FUNC_NAME(type)(type *_ptr) \ G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(type, func)
{ \
(func)(_ptr); \
}
/** /**
* VIR_AUTOPTR: * VIR_AUTOPTR:
@ -68,8 +72,7 @@
* Note that this macro must NOT be used with vectors! The freeing function * Note that this macro must NOT be used with vectors! The freeing function
* will not free any elements beyond the first. * will not free any elements beyond the first.
*/ */
#define VIR_AUTOPTR(type) \ #define VIR_AUTOPTR(type) g_autoptr(type)
__attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type)))) type *
/** /**
* VIR_AUTOCLEAN: * VIR_AUTOCLEAN:
@ -83,5 +86,4 @@
* Note that this macro must NOT be used with vectors! The cleaning function * Note that this macro must NOT be used with vectors! The cleaning function
* will not clean any elements beyond the first. * will not clean any elements beyond the first.
*/ */
#define VIR_AUTOCLEAN(type) \ #define VIR_AUTOCLEAN(type) g_auto(type)
__attribute__((cleanup(VIR_AUTOCLEAN_FUNC_NAME(type)))) type