On Tue, Mar 17, 2020 at 09:40:00AM -0500, Eric Blake wrote:
On 3/17/20 9:00 AM, Peter Maydell wrote:
On Tue, 17 Mar 2020 at 04:38, John Snow <jsnow@redhat.com> wrote:
block/qcow2-bitmap: use bdrv_dirty_bitmap_next_dirty (2020-03-12 16:36:46 -0400)
---------------------------------------------------------------- Pull request
---------------------------------------------------------------
Hi; this fails to compile with clang:
/home/petmay01/linaro/qemu-for-merges/nbd/server.c:1937:1: error: unused function 'glib_listautoptr_cleanup_NBDExtentArray' [-Werror,-Wunused-function] G_DEFINE_AUTOPTR_CLEANUP_FUNC(NBDExtentArray, nbd_extent_array_free); ^ /usr/include/glib-2.0/glib/gmacros.h:462:22: note: expanded from macro 'G_DEFINE_AUTOPTR_CLEANUP_FUNC' static inline void _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) (GList **_l) { g_list_free_full (*_l, (GDestroyNotify) func); } \ ^ /usr/include/glib-2.0/glib/gmacros.h:443:48: note: expanded from macro '_GLIB_AUTOPTR_LIST_FUNC_NAME' #define _GLIB_AUTOPTR_LIST_FUNC_NAME(TypeName) glib_listautoptr_cleanup_##TypeName ^ <scratch space>:49:1: note: expanded from here glib_listautoptr_cleanup_NBDExtentArray ^
Should we add -Wno-unused-function to CFLAGS when dealing with a version of clang that complains about that version of glib's headers? Is it fixed in a newer version of glib, where we could just backport a newer definition of G_DEFINE_AUTOPTR_CLEANUP_FUNC() that adds whatever annotations are needed to shut the compiler up?
On IRC, danpb pointed me to libvirt's solution: https://libvirt.org/git/?p=libvirt.git;a=commit;h=44e7f029
Maybe we just write our own macro wrapper around G_DEFINE_AUTOPTR_CLEANUP_FUNC which takes care of adding necessary annotations and use that instead (and our macro name might be shorter...)
My preference is to stick with regular glib functions/macros whereever practical, rather than inventing QEMU replacements which add a knowledge burden for contributors. That's why we moved to -Wno-unused-function in libvirt. I don't feel like -Wno-unused-function looses anything significant, as the GCC builds will still be reporting unused functions which will catch majority of cases. Possibly we could figure out a patch for glib upstream that uses pragma push/pop to squelch the warning ? They are quite receptive to patches IME. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|