From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39913) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xy5eu-0002hq-Rc for qemu-devel@nongnu.org; Mon, 08 Dec 2014 16:16:02 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Xy5ea-000824-6c for qemu-devel@nongnu.org; Mon, 08 Dec 2014 16:15:56 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54661) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Xy5eZ-00081r-M7 for qemu-devel@nongnu.org; Mon, 08 Dec 2014 16:15:35 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sB8LFZL8028441 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Mon, 8 Dec 2014 16:15:35 -0500 Date: Mon, 8 Dec 2014 23:15:32 +0200 From: "Michael S. Tsirkin" Message-ID: <20141208211532.GD30082@redhat.com> References: <1418054888-11310-1-git-send-email-imammedo@redhat.com> <1418054888-11310-5-git-send-email-imammedo@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1418054888-11310-5-git-send-email-imammedo@redhat.com> Subject: Re: [Qemu-devel] [PATCH 4/9] acpi: build_append_nameseg(): add padding if necessary List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: qemu-devel@nongnu.org, marcel.a@redhat.com On Mon, Dec 08, 2014 at 04:08:03PM +0000, Igor Mammedov wrote: > According to ACPI spec NameSeg shorter than 4 characters > must be padded up to 4 characters with "_" symbol. > ACPI 5.0: 20.2.2 "Name Objects Encoding" > > Do it in build_append_nameseg() so that caller shouldn't know > or care about it. > > Signed-off-by: Igor Mammedov To me just doing it right in callers seems just as easy, but I guess you disagree :) > --- > hw/i386/acpi-build.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > index f5ec66a..a8b7a2b 100644 > --- a/hw/i386/acpi-build.c > +++ b/hw/i386/acpi-build.c > @@ -292,6 +292,8 @@ static inline void build_append_array(GArray *array, GArray *val) > g_array_append_vals(array, val->data, val->len); > } > > +#define ACPI_NAMESEG_LEN 4 > + > static void GCC_FMT_ATTR(2, 3) > build_append_nameseg(GArray *array, const char *format, ...) > { > @@ -299,13 +301,19 @@ build_append_nameseg(GArray *array, const char *format, ...) > char s[] = "XXXX"; > int len; > va_list args; > + const char padding = '_'; > > va_start(args, format); > len = vsnprintf(s, sizeof s, format, args); > va_end(args); > > - assert(len == 4); > + g_assert(len <= ACPI_NAMESEG_LEN); I'm not sure when is g_assert preferable to assert. What's the motivation here? > + > g_array_append_vals(array, s, len); > + while (len != ACPI_NAMESEG_LEN) { > + g_array_append_val(array, padding); > + ++len; > + } Easier /* Pad up to 4 characters if necessary. */ g_array_append_vals(array, "____", 4 - len); > } > > /* 5.4 Definition Block Encoding */ > @@ -846,7 +854,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) > > if (bus->parent_dev) { > op = 0x82; /* DeviceOp */ > - build_append_nameseg(bus_table, "S%.02X_", > + build_append_nameseg(bus_table, "S%.02X", > bus->parent_dev->devfn); > build_append_byte(bus_table, 0x08); /* NameOp */ > build_append_nameseg(bus_table, "_SUN"); > @@ -966,7 +974,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) > build_append_int(notify, 0x1U << i); > build_append_byte(notify, 0x00); /* NullName */ > build_append_byte(notify, 0x86); /* NotifyOp */ > - build_append_nameseg(notify, "S%.02X_", PCI_DEVFN(i, 0)); > + build_append_nameseg(notify, "S%.02X", PCI_DEVFN(i, 0)); > build_append_byte(notify, 0x69); /* Arg1Op */ > > /* Pack it up */ > @@ -1023,7 +1031,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state) > if (bus->parent_dev) { > build_append_byte(parent->notify_table, '^'); /* ParentPrefixChar */ > build_append_byte(parent->notify_table, 0x2E); /* DualNamePrefix */ > - build_append_nameseg(parent->notify_table, "S%.02X_", > + build_append_nameseg(parent->notify_table, "S%.02X", > bus->parent_dev->devfn); > build_append_nameseg(parent->notify_table, "PCNT"); > } > @@ -1093,7 +1101,7 @@ build_ssdt(GArray *table_data, GArray *linker, > GArray *sb_scope = build_alloc_array(); > uint8_t op = 0x10; /* ScopeOp */ > > - build_append_nameseg(sb_scope, "_SB_"); > + build_append_nameseg(sb_scope, "_SB"); > > /* build Processor object for each processor */ > for (i = 0; i < acpi_cpus; i++) { > -- > 1.8.3.1