Re: [Qemu-devel] [PATCH] scripts: provide a script for checking glib symbol usage

[Daniel P. Berrange]

On Fri, Dec 18, 2015 at 01:36:00PM +0100, Paolo Bonzini wrote:
> 
> 
> On 18/12/2015 12:36, Daniel P. Berrange wrote:
> > diff --git a/scripts/Makefile b/scripts/Makefile
> > new file mode 100644
> > index 0000000..162e7e9
> > --- /dev/null
> > +++ b/scripts/Makefile
> > @@ -0,0 +1,24 @@
> > +#
> > +# This makefile runs various style checks across the entire
> > +# source tree.
> > +#
> > +# This is similar in concept of checkpatch.pl, but enforces
> > +# rules across the entire codebase, not just new patches
> > +#
> > +
> > +STYLE_CHECKS = \
> > +   cs-glib-syms
> > +
> > +ALL_FILES = \
> > +   $(shell git ls-tree -r HEAD . | awk '{print $$4}')
> > +
> > +C_CODE_FILES = $(filter %.c %.h, $(ALL_FILES))
> > +
> > +check-style: $(STYLE_CHECKS)
> > +
> > +# Check that we only use glib symbols present in our
> > +# minimum declared glib version
> > +GLIB_SYMS_LIST = scripts/glib-syms.txt
> > +
> > +cs-glib-syms:
> > +   @perl scripts/glib-syms.pl $(GLIB_SYMS_LIST) $(C_CODE_FILES)
> 
> 
> Does this need to be included, or could it be a separate Makefile
> invoked with e.g. make -f scripts/Makefile.style?

Any particular reason to favour that over include ? I did it this
way because QEMU in general seems to be biased towards includes
and not recursive make

> > +# Symbols not present in the release that we depend
> > +# on, but which have wrappers in include/glib-compat.h
> > +my @compatsyms = qw(
> > +    g_get_monotonic_time
> > +
> > +    g_assert_true
> > +    g_assert_false
> > +    g_assert_null
> > +    g_assert_nonnull
> > +    g_assert_cmpmem
> > +
> > +    g_hash_table_add
> > +
> > +    g_cond_clear
> > +    g_cond_init
> > +    g_cond_wait_until
> > +
> > +    g_mutex_init
> > +    g_mutex_clear
> > +
> > +    g_thread_new
> > +
> > +    g_private_replace
> > +    G_PRIVATE_INIT
> > +
> > +    G_TIME_SPAN_SECOND
> > +);
> > +
> > +
> > +# Functions defined inside QEMU which are using the
> > +# the same "g_" function name prefix as glib, so
> > +# get mis-detected as glib symbols
> > +my @blacklist = qw(
> > +    g_to_float64
> > +    g_assert_no_errno
> > +    g_cclosure_new_swap
> > +    g_free_rcu
> > +    g_test_trap_subprocess
> > +    g_poll_fixed
> > +    g_list_insert_sorted_merged
> > +    G_BYTE
> > +);
> > +
> > +# GObject stuff used by gtk frontend
> > +my @gobjectsums = qw(
> > +    g_object_ref
> > +    g_object_unref
> > +    g_object_set_data
> > +    g_signal_connect
> > +    G_CALLBACK
> > +);
> > +
> > +# Functions defined by glib which are strangely
> > +# missing from their docs header index
> > +my @missingindex = qw(
> > +    g_assertion_message
> > +    g_assertion_message_expr
> > +    g_assertion_message_cmpstr
> > +    g_assertion_message_cmpnum
> > +    g_assertion_message_error
> > +);
> > +
> 
> Can we "parse" #.* as comments, and put these in a glib-syms-extra.txt
> file?  Then we can hypothetically do the same tests in checkpatch.pl too.

Yep, that works.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|