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 :)
> ---
> 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
