diff --git a/build-aux/syntax-check.mk b/build-aux/syntax-check.mk index 8345703b3e..f4712c24c3 100644 --- a/build-aux/syntax-check.mk +++ b/build-aux/syntax-check.mk @@ -1128,7 +1128,7 @@ sc_prohibit_backslash_alignment: # Rule to ensure that variables declared using a cleanup macro are # always initialized. 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]$$' \ halt='variable declared with a cleanup macro must be initialized' \ $(_sc_search_regexp) diff --git a/docs/hacking.html.in b/docs/hacking.html.in index 3f1542b6de..d12b246ef9 100644 --- a/docs/hacking.html.in +++ b/docs/hacking.html.in @@ -1028,6 +1028,24 @@ BAD:
The GLib APIs g_strdup_printf / g_strdup_vprint should be used instead. Don't use g_vasprintf unless having the string length returned is unavoidable.
+ +
VIR_AUTOPTR, VIR_AUTOCLEAN, VIR_AUTOFREE
+
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. +
+ +
VIR_DEFINE_AUTOPTR_FUNC, VIR_DEFINE_AUTOCLEAN_FUNC
+
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. +

File handling

diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 1dbe1abe27..7c86fdd3c6 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -104,6 +104,20 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dontwarn="$dontwarn -Wdouble-promotion" 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 # ideally we'd turn many of them on dontwarn="$dontwarn -Wfloat-equal" @@ -119,6 +133,13 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ # Remove the ones we don't want, blacklisted earlier 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. # Unfortunately, this means you can't simply use '-Wsign-compare' # with gl_MANYWARN_COMPLEMENT diff --git a/src/util/viralloc.h b/src/util/viralloc.h index 517f9aada6..49bf2b86e7 100644 --- a/src/util/viralloc.h +++ b/src/util/viralloc.h @@ -494,8 +494,11 @@ void virDisposeString(char **strptr) * VIR_AUTOFREE: * @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 * the variable declared with it by calling virFree * when the variable goes out of scope. */ -#define VIR_AUTOFREE(type) __attribute__((cleanup(virFree))) type +#define VIR_AUTOFREE(type) g_autofree type diff --git a/src/util/virautoclean.h b/src/util/virautoclean.h index 6da288e67d..71312a2782 100644 --- a/src/util/virautoclean.h +++ b/src/util/virautoclean.h @@ -20,7 +20,21 @@ #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: @@ -31,15 +45,8 @@ * resources allocated to a variable of type @type. This newly * defined function works as a necessary wrapper around @func. */ -#define VIR_DEFINE_AUTOPTR_FUNC(type, func) \ - static inline void VIR_AUTOPTR_FUNC_NAME(type)(type **_ptr) \ - { \ - if (*_ptr) \ - (func)(*_ptr); \ - *_ptr = NULL; \ - } - -#define VIR_AUTOCLEAN_FUNC_NAME(type) type##AutoClean +#define VIR_DEFINE_AUTOPTR_FUNC(t, f) \ + G_DEFINE_AUTOPTR_CLEANUP_FUNC(t, f) /** * VIR_DEFINE_AUTOCLEAN_FUNC: @@ -51,10 +58,7 @@ * take pointer to @type. */ #define VIR_DEFINE_AUTOCLEAN_FUNC(type, func) \ - static inline void VIR_AUTOCLEAN_FUNC_NAME(type)(type *_ptr) \ - { \ - (func)(_ptr); \ - } + G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(type, func) /** * VIR_AUTOPTR: @@ -68,8 +72,7 @@ * Note that this macro must NOT be used with vectors! The freeing function * will not free any elements beyond the first. */ -#define VIR_AUTOPTR(type) \ - __attribute__((cleanup(VIR_AUTOPTR_FUNC_NAME(type)))) type * +#define VIR_AUTOPTR(type) g_autoptr(type) /** * VIR_AUTOCLEAN: @@ -83,5 +86,4 @@ * Note that this macro must NOT be used with vectors! The cleaning function * will not clean any elements beyond the first. */ -#define VIR_AUTOCLEAN(type) \ - __attribute__((cleanup(VIR_AUTOCLEAN_FUNC_NAME(type)))) type +#define VIR_AUTOCLEAN(type) g_auto(type)