On Mon, 8 Dec 2014 23:15:32 +0200 "Michael S. Tsirkin" <m...@redhat.com> wrote:
> 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 <imamm...@redhat.com> > > To me just doing it right in callers seems just as easy, but > I guess you disagree :) That means, author MUST know about padding, if he/she doesn't or forget about it that would introduce error usually resulting in BSOD. This patch allows to avoid such mistakes. > > > --- > > 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