Patchwork [4/7] Use sizeof(qemu_uuid) instead of literal 16

login
register
mail settings
Submitter Markus Armbruster
Date June 6, 2013, 4:27 p.m.
Message ID <1370536046-15125-5-git-send-email-armbru@redhat.com>
Download mbox | patch
Permalink /patch/249472/
State New
Headers show

Comments

Markus Armbruster - June 6, 2013, 4:27 p.m.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 arch_init.c             | 3 ++-
 hw/nvram/fw_cfg.c       | 2 +-
 include/sysemu/sysemu.h | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)
Laszlo Ersek - June 6, 2013, 6:26 p.m.
On 06/06/13 18:27, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  arch_init.c             | 3 ++-
>  hw/nvram/fw_cfg.c       | 2 +-
>  include/sysemu/sysemu.h | 2 +-
>  3 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 5d71870..aa24660 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -1029,7 +1029,8 @@ int qemu_uuid_parse(const char *str, uint8_t *uuid)
>          return -1;
>      }
>  #ifdef TARGET_I386
> -    smbios_add_field(1, offsetof(struct smbios_type_1, uuid), 16, uuid);
> +    smbios_add_field(1, offsetof(struct smbios_type_1, uuid),
> +                     sizeof(uuid), uuid);
>  #endif
>      return 0;
>  }

I believe this is wrong, "uuid" is not an array here but a pointer. I
guess you mistyped "sizeof(qemu_uuid)" as "sizeof(uuid)" in the third arg.

> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 3c255ce..f1d3861 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -509,7 +509,7 @@ FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
>          sysbus_mmio_map(d, 1, data_addr);
>      }
>      fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4);
> -    fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
> +    fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, sizeof(qemu_uuid));
>      fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC));
>      fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
>      fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 2fb71af..b969e56 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -15,7 +15,7 @@
>  extern const char *bios_name;
>  
>  extern const char *qemu_name;
> -extern uint8_t qemu_uuid[];
> +extern uint8_t qemu_uuid[16];
>  int qemu_uuid_parse(const char *str, uint8_t *uuid);
>  #define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"
>  
> 

Rest seems OK.

Laszlo
Markus Armbruster - June 6, 2013, 7:52 p.m.
Laszlo Ersek <lersek@redhat.com> writes:

> On 06/06/13 18:27, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  arch_init.c             | 3 ++-
>>  hw/nvram/fw_cfg.c       | 2 +-
>>  include/sysemu/sysemu.h | 2 +-
>>  3 files changed, 4 insertions(+), 3 deletions(-)
>> 
>> diff --git a/arch_init.c b/arch_init.c
>> index 5d71870..aa24660 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -1029,7 +1029,8 @@ int qemu_uuid_parse(const char *str, uint8_t *uuid)
>>          return -1;
>>      }
>>  #ifdef TARGET_I386
>> -    smbios_add_field(1, offsetof(struct smbios_type_1, uuid), 16, uuid);
>> +    smbios_add_field(1, offsetof(struct smbios_type_1, uuid),
>> +                     sizeof(uuid), uuid);
>>  #endif
>>      return 0;
>>  }
>
> I believe this is wrong, "uuid" is not an array here but a pointer. I
> guess you mistyped "sizeof(qemu_uuid)" as "sizeof(uuid)" in the third arg.

Rats!  You're right.  Thanks!

Patch

diff --git a/arch_init.c b/arch_init.c
index 5d71870..aa24660 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1029,7 +1029,8 @@  int qemu_uuid_parse(const char *str, uint8_t *uuid)
         return -1;
     }
 #ifdef TARGET_I386
-    smbios_add_field(1, offsetof(struct smbios_type_1, uuid), 16, uuid);
+    smbios_add_field(1, offsetof(struct smbios_type_1, uuid),
+                     sizeof(uuid), uuid);
 #endif
     return 0;
 }
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index 3c255ce..f1d3861 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -509,7 +509,7 @@  FWCfgState *fw_cfg_init(uint32_t ctl_port, uint32_t data_port,
         sysbus_mmio_map(d, 1, data_addr);
     }
     fw_cfg_add_bytes(s, FW_CFG_SIGNATURE, (char *)"QEMU", 4);
-    fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, 16);
+    fw_cfg_add_bytes(s, FW_CFG_UUID, qemu_uuid, sizeof(qemu_uuid));
     fw_cfg_add_i16(s, FW_CFG_NOGRAPHIC, (uint16_t)(display_type == DT_NOGRAPHIC));
     fw_cfg_add_i16(s, FW_CFG_NB_CPUS, (uint16_t)smp_cpus);
     fw_cfg_add_i16(s, FW_CFG_BOOT_MENU, (uint16_t)boot_menu);
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 2fb71af..b969e56 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -15,7 +15,7 @@ 
 extern const char *bios_name;
 
 extern const char *qemu_name;
-extern uint8_t qemu_uuid[];
+extern uint8_t qemu_uuid[16];
 int qemu_uuid_parse(const char *str, uint8_t *uuid);
 #define UUID_FMT "%02hhx%02hhx%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx-%02hhx%02hhx%02hhx%02hhx%02hhx%02hhx"