Patchwork [6/7] smbios: Fix -smbios type=0, release=... for big endian hosts

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

Comments

Markus Armbruster - June 6, 2013, 4:27 p.m.
Classic endianness bug due to careless dirty coding: assuming reading
a byte from an int variable gets the least significant byte.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/i386/smbios.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Laszlo Ersek - June 6, 2013, 6:35 p.m.
On 06/06/13 18:27, Markus Armbruster wrote:
> Classic endianness bug due to careless dirty coding: assuming reading
> a byte from an int variable gets the least significant byte.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  hw/i386/smbios.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
> index 322f0a0..68bd6d0 100644
> --- a/hw/i386/smbios.c
> +++ b/hw/i386/smbios.c
> @@ -127,6 +127,7 @@ void smbios_add_field(int type, int offset, const void *data, size_t len)
>  static void smbios_build_type_0_fields(const char *t)
>  {
>      char buf[1024];
> +    unsigned char major, minor;
>  
>      if (get_param_value(buf, sizeof(buf), "vendor", t))
>          smbios_add_field(0, offsetof(struct smbios_type_0, vendor_str),
> @@ -139,8 +140,7 @@ static void smbios_build_type_0_fields(const char *t)
>                                       bios_release_date_str),
>                           buf, strlen(buf) + 1);
>      if (get_param_value(buf, sizeof(buf), "release", t)) {
> -        int major, minor;
> -        sscanf(buf, "%d.%d", &major, &minor);
> +        sscanf(buf, "%hhd.%hhd", &major, &minor);
>          smbios_add_field(0, offsetof(struct smbios_type_0,
>                                       system_bios_major_release),
>                           &major, 1);
> 

Strictly speaking these should be %hhu, if it's not much of a bother.
Otherwise:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

(BTW what was wrong with the definitions being in the narrowest scope? :))
Markus Armbruster - June 6, 2013, 7:55 p.m.
Laszlo Ersek <lersek@redhat.com> writes:

> On 06/06/13 18:27, Markus Armbruster wrote:
>> Classic endianness bug due to careless dirty coding: assuming reading
>> a byte from an int variable gets the least significant byte.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  hw/i386/smbios.c | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
>> index 322f0a0..68bd6d0 100644
>> --- a/hw/i386/smbios.c
>> +++ b/hw/i386/smbios.c
>> @@ -127,6 +127,7 @@ void smbios_add_field(int type, int offset, const void *data, size_t len)
>>  static void smbios_build_type_0_fields(const char *t)
>>  {
>>      char buf[1024];
>> +    unsigned char major, minor;
>>  
>>      if (get_param_value(buf, sizeof(buf), "vendor", t))
>>          smbios_add_field(0, offsetof(struct smbios_type_0, vendor_str),
>> @@ -139,8 +140,7 @@ static void smbios_build_type_0_fields(const char *t)
>>                                       bios_release_date_str),
>>                           buf, strlen(buf) + 1);
>>      if (get_param_value(buf, sizeof(buf), "release", t)) {
>> -        int major, minor;
>> -        sscanf(buf, "%d.%d", &major, &minor);
>> +        sscanf(buf, "%hhd.%hhd", &major, &minor);
>>          smbios_add_field(0, offsetof(struct smbios_type_0,
>>                                       system_bios_major_release),
>>                           &major, 1);
>> 
>
> Strictly speaking these should be %hhu, if it's not much of a bother.

It's not.

> Otherwise:
>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
>
> (BTW what was wrong with the definitions being in the narrowest scope? :))

I don't like spreading declarations all over the function in C.  Not
entirely sure why.  Perhaps it's just habit.  Perhaps it's because
declarations don't stand out visually, unlike in some other languages.

Patch

diff --git a/hw/i386/smbios.c b/hw/i386/smbios.c
index 322f0a0..68bd6d0 100644
--- a/hw/i386/smbios.c
+++ b/hw/i386/smbios.c
@@ -127,6 +127,7 @@  void smbios_add_field(int type, int offset, const void *data, size_t len)
 static void smbios_build_type_0_fields(const char *t)
 {
     char buf[1024];
+    unsigned char major, minor;
 
     if (get_param_value(buf, sizeof(buf), "vendor", t))
         smbios_add_field(0, offsetof(struct smbios_type_0, vendor_str),
@@ -139,8 +140,7 @@  static void smbios_build_type_0_fields(const char *t)
                                      bios_release_date_str),
                          buf, strlen(buf) + 1);
     if (get_param_value(buf, sizeof(buf), "release", t)) {
-        int major, minor;
-        sscanf(buf, "%d.%d", &major, &minor);
+        sscanf(buf, "%hhd.%hhd", &major, &minor);
         smbios_add_field(0, offsetof(struct smbios_type_0,
                                      system_bios_major_release),
                          &major, 1);