Re: [libvirt] [PATCH v3 06/19] util: rewrite auto cleanup macros to use glib's equivalent

2019-10-10 Thread Bjoern Walk
Daniel P. Berrangé  [2019-10-10, 11:54AM +0100]:
> diff --git a/docs/hacking.html.in b/docs/hacking.html.in
> index 3f1542b6de..6e62b2d4ff 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 Gib macros G_DEFINE_AUTOPTR_CLEANUP_FUNC and
  ^
GLib

-- 
IBM Systems
Linux on Z & Virtualization Development
--
IBM Deutschland Research & Development GmbH
Schönaicher Str. 220, 71032 Böblingen
Phone: +49 7031 16 1819
--
Vorsitzende des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


signature.asc
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH v3 06/19] util: rewrite auto cleanup macros to use glib's equivalent

2019-10-10 Thread Daniel P . Berrangé
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 
Signed-off-by: Daniel P. Berrangé 
---
 build-aux/syntax-check.mk   |  2 +-
 docs/hacking.html.in| 18 ++
 m4/virt-compile-warnings.m4 | 21 
 src/util/viralloc.h |  5 -
 src/util/virautoclean.h | 38 +++--
 5 files changed, 64 insertions(+), 20 deletions(-)

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..6e62b2d4ff 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 Gib 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(ty