Re: [Qemu-devel] [PATCH] checkpatch: Disallow glib asserts in main code

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3070 bytes --]

On 04/28/2017 08:34 AM, Markus Armbruster wrote:

>>
>> Ban most of the glib assertion functions (basically everything except
>> g_assert and g_assert_not_reached) except in tests/

You'll also want to exclude scripts/ and possible include/glib-compat.h...


> If these are screwy enough to warrant rejecting them in new code,
> they're probably screwy enough to purge them from existing code:
> 
>     $ git-grep -E 'g_assert_cmpstr|g_assert_cmpint|g_assert_cmpuint|g_assert_cmphex|g_assert_cmpfloat|g_assert_true|g_assert_false|g_assert_nonnull|g_assert_null|g_assert_no_error|g_assert_error|g_test_assert_expected_messages|g_test_trap_assert_passed|g_test_trap_assert_stdout|g_test_trap_assert_stdout_unmatched|g_test_trap_assert_stderr|g_test_trap_assert_stderr_unmatched' | grep -v ^tests/
>     hw/ide/ahci.c:        g_assert_cmpint(size, >, 1);
>     hw/ppc/spapr_ovec.c:    g_assert_cmpint(bitnr, <, OV_MAXBITS);
>     hw/ppc/spapr_ovec.c:    g_assert_cmpint(bitnr, <, OV_MAXBITS);
>     hw/ppc/spapr_ovec.c:    g_assert_cmpint(bitnr, <, OV_MAXBITS);
>     hw/ppc/spapr_ovec.c:    g_assert_cmpint(vector, >=, 1); /* vector numbering starts at 1 */
>     hw/ppc/spapr_ovec.c:    g_assert_cmpint(vector_len, <=, OV_MAXBYTES);
>     hw/ppc/spapr_ovec.c:    g_assert_cmpint(vec_len, <=, OV_MAXBYTES);

Those should go.

>     include/glib-compat.h:#ifndef g_assert_true
>     include/glib-compat.h:#define g_assert_true(expr)                                                    \
>     include/glib-compat.h:#ifndef g_assert_false
>     include/glib-compat.h:#define g_assert_false(expr)                                                   \
>     include/glib-compat.h:#ifndef g_assert_null
>     include/glib-compat.h:#define g_assert_null(expr)                                                    \
>     include/glib-compat.h:#ifndef g_assert_nonnull
>     include/glib-compat.h:#define g_assert_nonnull(expr)                                                 \

We still need these until we can require glib 2.38 or even 2.40.

>     qom/object.c:        g_assert_cmpint(parent->class_size, <=, ti->class_size);
>     qom/object.c:    g_assert_cmpint(type->instance_size, >=, sizeof(Object));
>     qom/object.c:    g_assert_cmpint(size, >=, type->instance_size);
>     qom/object.c:    g_assert_cmpint(obj->ref, ==, 0);
>     qom/object.c:    g_assert_cmpint(obj->ref, >, 0);

These should go.

>     scripts/cocci-macro-file.h:#define g_assert_cmpint(a, op, b)   g_assert(a op b)
>     scripts/cocci-macro-file.h:#define g_assert_cmpuint(a, op, b)   g_assert(a op b)
>     scripts/cocci-macro-file.h:#define g_assert_cmphex(a, op, b)   g_assert(a op b)
>     scripts/cocci-macro-file.h:#define g_assert_cmpstr(a, op, b)   g_assert(strcmp(a, b) op 0)

These must stay permanently.

>     util/qht.c:    g_assert_cmpuint(new->n_buckets, !=, old->n_buckets);
> 
> Volunteers?
> 
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]