Patchwork [5/7] smbios: Clean up smbios_add_field() parameters

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

Comments

Markus Armbruster - June 6, 2013, 4:27 p.m.
Having size preceed the associated pointer is odd.  Swap them, and fix
up the types.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 arch_init.c              |  2 +-
 hw/i386/smbios.c         | 26 ++++++++++++++------------
 include/hw/i386/smbios.h |  2 +-
 3 files changed, 16 insertions(+), 14 deletions(-)
Laszlo Ersek - June 6, 2013, 6:31 p.m.
On 06/06/13 18:27, Markus Armbruster wrote:
> Having size preceed the associated pointer is odd.  Swap them, and fix
> up the types.

Can you proceed to fix the spelling of "precede"? :)

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  arch_init.c              |  2 +-
>  hw/i386/smbios.c         | 26 ++++++++++++++------------
>  include/hw/i386/smbios.h |  2 +-
>  3 files changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index aa24660..06b65a2 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -1030,7 +1030,7 @@ int qemu_uuid_parse(const char *str, uint8_t *uuid)
>      }
>  #ifdef TARGET_I386
>      smbios_add_field(1, offsetof(struct smbios_type_1, uuid),
> -                     sizeof(uuid), uuid);
> +                     uuid, sizeof(uuid));

Still wrong I think.

>  #endif
>      return 0;
>  }
> diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
> index a67a328..322f0a0 100644
> --- a/hw/i386/smbios.c
> +++ b/hw/i386/smbios.c
> @@ -99,7 +99,7 @@ static void smbios_check_collision(int type, int entry)
>      }
>  }
>  
> -void smbios_add_field(int type, int offset, int len, void *data)
> +void smbios_add_field(int type, int offset, const void *data, size_t len)
>  {
>      struct smbios_field *field;
>  
> @@ -130,21 +130,23 @@ static void smbios_build_type_0_fields(const char *t)
>  
>      if (get_param_value(buf, sizeof(buf), "vendor", t))
>          smbios_add_field(0, offsetof(struct smbios_type_0, vendor_str),
> -                         strlen(buf) + 1, buf);
> +                         buf, strlen(buf) + 1);
>      if (get_param_value(buf, sizeof(buf), "version", t))
>          smbios_add_field(0, offsetof(struct smbios_type_0, bios_version_str),
> -                         strlen(buf) + 1, buf);
> +                         buf, strlen(buf) + 1);
>      if (get_param_value(buf, sizeof(buf), "date", t))
>          smbios_add_field(0, offsetof(struct smbios_type_0,
>                                       bios_release_date_str),
> -                                     strlen(buf) + 1, buf);
> +                         buf, strlen(buf) + 1);
>      if (get_param_value(buf, sizeof(buf), "release", t)) {
>          int major, minor;
>          sscanf(buf, "%d.%d", &major, &minor);
>          smbios_add_field(0, offsetof(struct smbios_type_0,
> -                                     system_bios_major_release), 1, &major);
> +                                     system_bios_major_release),
> +                         &major, 1);
>          smbios_add_field(0, offsetof(struct smbios_type_0,
> -                                     system_bios_minor_release), 1, &minor);
> +                                     system_bios_minor_release),
> +                         &minor, 1);
>      }
>  }
>  
> @@ -154,16 +156,16 @@ static void smbios_build_type_1_fields(const char *t)
>  
>      if (get_param_value(buf, sizeof(buf), "manufacturer", t))
>          smbios_add_field(1, offsetof(struct smbios_type_1, manufacturer_str),
> -                         strlen(buf) + 1, buf);
> +                         buf, strlen(buf) + 1);
>      if (get_param_value(buf, sizeof(buf), "product", t))
>          smbios_add_field(1, offsetof(struct smbios_type_1, product_name_str),
> -                         strlen(buf) + 1, buf);
> +                         buf, strlen(buf) + 1);
>      if (get_param_value(buf, sizeof(buf), "version", t))
>          smbios_add_field(1, offsetof(struct smbios_type_1, version_str),
> -                         strlen(buf) + 1, buf);
> +                         buf, strlen(buf) + 1);
>      if (get_param_value(buf, sizeof(buf), "serial", t))
>          smbios_add_field(1, offsetof(struct smbios_type_1, serial_number_str),
> -                         strlen(buf) + 1, buf);
> +                         buf, strlen(buf) + 1);
>      if (get_param_value(buf, sizeof(buf), "uuid", t)) {
>          if (qemu_uuid_parse(buf, qemu_uuid) != 0) {
>              error_report("Invalid UUID");
> @@ -172,10 +174,10 @@ static void smbios_build_type_1_fields(const char *t)
>      }
>      if (get_param_value(buf, sizeof(buf), "sku", t))
>          smbios_add_field(1, offsetof(struct smbios_type_1, sku_number_str),
> -                         strlen(buf) + 1, buf);
> +                         buf, strlen(buf) + 1);
>      if (get_param_value(buf, sizeof(buf), "family", t))
>          smbios_add_field(1, offsetof(struct smbios_type_1, family_str),
> -                         strlen(buf) + 1, buf);
> +                         buf, strlen(buf) + 1);
>  }
>  
>  int smbios_entry_add(const char *t)
> diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
> index 94e3641..9babeaf 100644
> --- a/include/hw/i386/smbios.h
> +++ b/include/hw/i386/smbios.h
> @@ -14,7 +14,7 @@
>   */
>  
>  int smbios_entry_add(const char *t);
> -void smbios_add_field(int type, int offset, int len, void *data);
> +void smbios_add_field(int type, int offset, const void *data, size_t len);
>  uint8_t *smbios_get_table(size_t *length);
>  
>  /*
> 

I trust gcc would have caught any missed calls ("creates pointer from
integer without cast" or some such). So the only problem (I think) is in
qemu_uuid_parse() which should be semi-auto-fixed when you rebase.

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:
>> Having size preceed the associated pointer is odd.  Swap them, and fix
>> up the types.
>
> Can you proceed to fix the spelling of "precede"? :)

Sure.

Patch

diff --git a/arch_init.c b/arch_init.c
index aa24660..06b65a2 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -1030,7 +1030,7 @@  int qemu_uuid_parse(const char *str, uint8_t *uuid)
     }
 #ifdef TARGET_I386
     smbios_add_field(1, offsetof(struct smbios_type_1, uuid),
-                     sizeof(uuid), uuid);
+                     uuid, sizeof(uuid));
 #endif
     return 0;
 }
diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index a67a328..322f0a0 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -99,7 +99,7 @@  static void smbios_check_collision(int type, int entry)
     }
 }
 
-void smbios_add_field(int type, int offset, int len, void *data)
+void smbios_add_field(int type, int offset, const void *data, size_t len)
 {
     struct smbios_field *field;
 
@@ -130,21 +130,23 @@  static void smbios_build_type_0_fields(const char *t)
 
     if (get_param_value(buf, sizeof(buf), "vendor", t))
         smbios_add_field(0, offsetof(struct smbios_type_0, vendor_str),
-                         strlen(buf) + 1, buf);
+                         buf, strlen(buf) + 1);
     if (get_param_value(buf, sizeof(buf), "version", t))
         smbios_add_field(0, offsetof(struct smbios_type_0, bios_version_str),
-                         strlen(buf) + 1, buf);
+                         buf, strlen(buf) + 1);
     if (get_param_value(buf, sizeof(buf), "date", t))
         smbios_add_field(0, offsetof(struct smbios_type_0,
                                      bios_release_date_str),
-                                     strlen(buf) + 1, buf);
+                         buf, strlen(buf) + 1);
     if (get_param_value(buf, sizeof(buf), "release", t)) {
         int major, minor;
         sscanf(buf, "%d.%d", &major, &minor);
         smbios_add_field(0, offsetof(struct smbios_type_0,
-                                     system_bios_major_release), 1, &major);
+                                     system_bios_major_release),
+                         &major, 1);
         smbios_add_field(0, offsetof(struct smbios_type_0,
-                                     system_bios_minor_release), 1, &minor);
+                                     system_bios_minor_release),
+                         &minor, 1);
     }
 }
 
@@ -154,16 +156,16 @@  static void smbios_build_type_1_fields(const char *t)
 
     if (get_param_value(buf, sizeof(buf), "manufacturer", t))
         smbios_add_field(1, offsetof(struct smbios_type_1, manufacturer_str),
-                         strlen(buf) + 1, buf);
+                         buf, strlen(buf) + 1);
     if (get_param_value(buf, sizeof(buf), "product", t))
         smbios_add_field(1, offsetof(struct smbios_type_1, product_name_str),
-                         strlen(buf) + 1, buf);
+                         buf, strlen(buf) + 1);
     if (get_param_value(buf, sizeof(buf), "version", t))
         smbios_add_field(1, offsetof(struct smbios_type_1, version_str),
-                         strlen(buf) + 1, buf);
+                         buf, strlen(buf) + 1);
     if (get_param_value(buf, sizeof(buf), "serial", t))
         smbios_add_field(1, offsetof(struct smbios_type_1, serial_number_str),
-                         strlen(buf) + 1, buf);
+                         buf, strlen(buf) + 1);
     if (get_param_value(buf, sizeof(buf), "uuid", t)) {
         if (qemu_uuid_parse(buf, qemu_uuid) != 0) {
             error_report("Invalid UUID");
@@ -172,10 +174,10 @@  static void smbios_build_type_1_fields(const char *t)
     }
     if (get_param_value(buf, sizeof(buf), "sku", t))
         smbios_add_field(1, offsetof(struct smbios_type_1, sku_number_str),
-                         strlen(buf) + 1, buf);
+                         buf, strlen(buf) + 1);
     if (get_param_value(buf, sizeof(buf), "family", t))
         smbios_add_field(1, offsetof(struct smbios_type_1, family_str),
-                         strlen(buf) + 1, buf);
+                         buf, strlen(buf) + 1);
 }
 
 int smbios_entry_add(const char *t)
diff --git a/include/hw/i386/smbios.h b/include/hw/i386/smbios.h
index 94e3641..9babeaf 100644
--- a/include/hw/i386/smbios.h
+++ b/include/hw/i386/smbios.h
@@ -14,7 +14,7 @@ 
  */
 
 int smbios_entry_add(const char *t);
-void smbios_add_field(int type, int offset, int len, void *data);
+void smbios_add_field(int type, int offset, const void *data, size_t len);
 uint8_t *smbios_get_table(size_t *length);
 
 /*