[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] tests: Make acpid test compile
From: |
Daniel P. Berrange |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] tests: Make acpid test compile |
Date: |
Wed, 30 Aug 2017 11:45:54 +0100 |
User-agent: |
Mutt/1.8.3 (2017-05-23) |
On Tue, Aug 29, 2017 at 03:17:25PM -0500, Eric Blake wrote:
> On 08/28/2017 09:41 AM, Cédric Le Goater wrote:
> > On 08/23/2017 01:53 PM, Dr. David Alan Gilbert wrote:
> >> * Juan Quintela (address@hidden) wrote:
> >>> Compiler gets confused with the size of the struct, so move form
> >>> g_new0() to g_malloc0().
> >>>
> >>> I *think* that the problem is in gcc (or glib for that matter), but
> >>> the documentation of the g_new0 states that 1sts first argument is an
> >>> struct type, and uint32_t is not an struct type.
> >>>
> >>> Signed-off-by: Juan Quintela <address@hidden>
> >>> ---
>
> >>>
> >>> /* get the addresses of the tables pointed by rsdt */
> >>> - tables = g_new0(uint32_t, tables_nr);
> >>> + tables = g_malloc0(sizeof(uint32_t) * tables_nr);
> >>
>
> > I fixed that one with :
> >
> > @@ -40,7 +40,7 @@ static uint32_t acpi_find_vgia(void)
> > AcpiRsdpDescriptor rsdp_table;
> > uint32_t rsdt;
> > AcpiRsdtDescriptorRev1 rsdt_table;
> > - int tables_nr;
> > + uint32_t tables_nr;
>
> I like this one better (multiplication in g_malloc0() makes me worry
> about overflow; using unsigned math to avoid the problem is nicer). Are
> we going to see a v2 of this patch series?
It should really be size_t, because it is assigned from the result of
a size_t calculation, but you then also need to change a later assert
which was relying on it being signed:
@@ -40,7 +40,7 @@ static uint32_t acpi_find_vgia(void)
AcpiRsdpDescriptor rsdp_table;
uint32_t rsdt;
AcpiRsdtDescriptorRev1 rsdt_table;
- int tables_nr;
+ size_t tables_nr;
uint32_t *tables;
AcpiTableHeader ssdt_table;
VgidTable vgid_table;
@@ -62,9 +62,9 @@ static uint32_t acpi_find_vgia(void)
ACPI_ASSERT_CMP(rsdt_table.signature, "RSDT");
/* compute the table entries in rsdt */
+ g_assert_cmpint(rsdt_table.length, >, sizeof(AcpiRsdtDescriptorRev1));
tables_nr = (rsdt_table.length - sizeof(AcpiRsdtDescriptorRev1)) /
sizeof(uint32_t);
- g_assert_cmpint(tables_nr, >, 0);
/* get the addresses of the tables pointed by rsdt */
tables = g_new0(uint32_t, tables_nr);
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 :|
[Qemu-devel] [PATCH 1/2] tests: Use real size for iov tests, Juan Quintela, 2017/08/23