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

[Paolo Bonzini <pbonzini@redhat.com>]

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?

> +# 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.

I'm not opposing the scripts/Makefile concept, and I'm not asking you to
patch checkpatch.pl; I just would like to keep the door open for that.

Paolo