Re: [Qemu-devel] [PATCH] tests/bios-tables-test: Compiler warning fix

[Daniel P. Berrange]

On Thu, Jul 20, 2017 at 05:35:36PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <address@hidden>
> 
> gcc 7.1.1 in fedora 26 moans about the:
>    tables = g_new0(uint32_t, tables_nr)
> 
> because it can't convince itself that tables_nr is positive.
> This is fallout from g_assert_cmpint no longer necessarily being
> no-return;  replace it with a plain g_assert.
> 
> Signed-off-by: Dr. David Alan Gilbert <address@hidden>
> ---
>  tests/bios-tables-test.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
> index 63da978f0b..564da45f65 100644
> --- a/tests/bios-tables-test.c
> +++ b/tests/bios-tables-test.c
> @@ -108,7 +108,7 @@ static void test_acpi_rsdt_table(test_data *data)
>      /* compute the table entries in rsdt */
>      tables_nr = (rsdt_table->length - sizeof(AcpiRsdtDescriptorRev1)) /
>                  sizeof(uint32_t);
> -    g_assert_cmpint(tables_nr, >, 0);
> +    g_assert(tables_nr > 0);

IMHO your original patch was better - rsdt_table->length is an
uint32_t, and sizeof() evaluates to size_t, but we're assigning
to a local  tables_nr that is a signed int. So tables_nr would
be better declared as size_t.  That would mean the assert can
just be removed entirely, or replaced by an assert that
rsdt_table->length > sizeof(AcpiRsdtDescriptorRev1) if we're
concerned about that


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 :|