From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42515) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ukf06-0001zf-A6 for qemu-devel@nongnu.org; Thu, 06 Jun 2013 14:33:32 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Ukf03-0002DT-9N for qemu-devel@nongnu.org; Thu, 06 Jun 2013 14:33:30 -0400 Received: from mx1.redhat.com ([209.132.183.28]:14935) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Ukf03-0002D1-06 for qemu-devel@nongnu.org; Thu, 06 Jun 2013 14:33:27 -0400 Message-ID: <51B0D684.2070509@redhat.com> Date: Thu, 06 Jun 2013 20:35:48 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <1370536046-15125-1-git-send-email-armbru@redhat.com> <1370536046-15125-7-git-send-email-armbru@redhat.com> In-Reply-To: <1370536046-15125-7-git-send-email-armbru@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 6/7] smbios: Fix -smbios type=0, release=... for big endian hosts List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org 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 > --- > 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 (BTW what was wrong with the definitions being in the narrowest scope? :))