4.9.0-rcX can not be built for ARM64

Commit Message

Andrii Anisov May 15, 2017, 8:30 a.m. UTC

Julien,


On 15.05.17 11:12, Julien Grall wrote:
> It looks like you compiler does not (validly?) detect that size will 
> always be initialized when (rc > 0). 

The question is if it should be considered as the XEN sources bug and to 
be patched appropriately.

> Can you try this small patch below:
>
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index db113dbf85..f61aec672b 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -73,6 +73,8 @@ static int libxl__estimate_madt_size(libxl__gc *gc,
>  {
>      int rc = 0;
>
> +    *size = 0;
> +
>      switch (info->arch_arm.gic_version) {
>      case LIBXL_GIC_VERSION_V2:
>          *size = sizeof(struct acpi_table_madt) +

On my table I initialized size before libxl__estimate_madt_size() usage, 
i.e.:


But your code seems to be leaner.

Comments

Julien Grall May 15, 2017, 10:34 a.m. UTC | #1

On 05/15/2017 09:30 AM, Andrii Anisov wrote:
> Julien,

Hi Andrii,

> On 15.05.17 11:12, Julien Grall wrote:
>> It looks like you compiler does not (validly?) detect that size will
>> always be initialized when (rc > 0).
>
> The question is if it should be considered as the XEN sources bug and to
> be patched appropriately.

I would consider it as Xen bug.

>
>> Can you try this small patch below:
>>
>> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
>> index db113dbf85..f61aec672b 100644
>> --- a/tools/libxl/libxl_arm_acpi.c
>> +++ b/tools/libxl/libxl_arm_acpi.c
>> @@ -73,6 +73,8 @@ static int libxl__estimate_madt_size(libxl__gc *gc,
>>  {
>>      int rc = 0;
>>
>> +    *size = 0;
>> +
>>      switch (info->arch_arm.gic_version) {
>>      case LIBXL_GIC_VERSION_V2:
>>          *size = sizeof(struct acpi_table_madt) +
>
> On my table I initialized size before libxl__estimate_madt_size() usage,
> i.e.:
>
> diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
> index db113db..ccc7ebe 100644
> --- a/tools/libxl/libxl_arm_acpi.c
> +++ b/tools/libxl/libxl_arm_acpi.c
> @@ -98,7 +98,7 @@ int libxl__get_acpi_size(libxl__gc *gc,
>                           const libxl_domain_build_info *info,
>                           uint64_t *out)
>  {
> -    uint64_t size;
> +    uint64_t size = 0;
>      int rc = 0;
>
>
> @@ -123,7 +123,7 @@ static int libxl__allocate_acpi_tables(libxl__gc *gc,
>                                         struct acpitable acpitables[])
>  {
>      int rc;
> -    size_t size;
> +    size_t size = 0;
>
>      acpitables[RSDP].addr = GUEST_ACPI_BASE;
>      acpitables[RSDP].size = sizeof(struct acpi_table_rsdp);
>
> But your code seems to be leaner.

Are you planning to submit a patch? I think we can take this for Xen 
4.9. Both version are good for me, it is a matter of taste here I think.

Cheers,

Andrii Anisov May 15, 2017, 1:20 p.m. UTC | #2

Julien,


On 15.05.17 13:34, Julien Grall wrote:
> Are you planning to submit a patch? I think we can take this for Xen 
> 4.9. Both version are good for me, it is a matter of taste here I think.
>
I would take your code, it is safer for possible future usage of 
libxl__estimate_madt_size() function.
I'm going to submit a patch, is it ok for you I put your signed-off?

Patch

diff --git a/tools/libxl/libxl_arm_acpi.c b/tools/libxl/libxl_arm_acpi.c
index db113db..ccc7ebe 100644
--- a/tools/libxl/libxl_arm_acpi.c
+++ b/tools/libxl/libxl_arm_acpi.c
@@ -98,7 +98,7 @@  int libxl__get_acpi_size(libxl__gc *gc,
                           const libxl_domain_build_info *info,
                           uint64_t *out)
  {
-    uint64_t size;
+    uint64_t size = 0;
      int rc = 0;


@@ -123,7 +123,7 @@  static int libxl__allocate_acpi_tables(libxl__gc *gc,
                                         struct acpitable acpitables[])
  {
      int rc;
-    size_t size;
+    size_t size = 0;

      acpitables[RSDP].addr = GUEST_ACPI_BASE;
      acpitables[RSDP].size = sizeof(struct acpi_table_rsdp);