[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 2/2] tests: Make acpid test compile
From: |
Juan Quintela |
Subject: |
Re: [Qemu-devel] [PATCH 2/2] tests: Make acpid test compile |
Date: |
Wed, 30 Aug 2017 13:37:32 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) |
"Daniel P. Berrange" <address@hidden> wrote:
> 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;
I was using this already.
> 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);
>
And here we are, this mail arrived after I sent my new series. Will
wait for more comments and resend later.
Thanks, Juan.
[Qemu-devel] [PATCH 1/2] tests: Use real size for iov tests, Juan Quintela, 2017/08/23