Re: [Qemu-devel] [PATCH v5 4/5] acpi: drop min-bytes in build_package()

[Marcel Apfelbaum <marcel@redhat.com>]

On 01/21/2015 11:09 AM, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> Reviewed-by: Claudio Fontana <claudio.fontana@huawei.com>
> ---
>   hw/acpi/acpi-build-utils.c         | 14 ++++----------
>   hw/i386/acpi-build.c               | 13 ++++++-------
>   include/hw/acpi/acpi-build-utils.h |  4 ++--
>   3 files changed, 12 insertions(+), 19 deletions(-)
>
> diff --git a/hw/acpi/acpi-build-utils.c b/hw/acpi/acpi-build-utils.c
> index aed9066..602e68c 100644
> --- a/hw/acpi/acpi-build-utils.c
> +++ b/hw/acpi/acpi-build-utils.c
> @@ -167,7 +167,7 @@ enum {
>       PACKAGE_LENGTH_4BYTE_SHIFT = 20,
>   };
>
> -void build_prepend_package_length(GArray *package, unsigned min_bytes)
> +void build_prepend_package_length(GArray *package)
>   {
>       uint8_t byte;
>       unsigned length = package->len;
> @@ -183,11 +183,6 @@ void build_prepend_package_length(GArray *package, unsigned min_bytes)
>           length_bytes = 4;
>       }
>
> -    /* Force length to at least min_bytes.
> -     * This wastes memory but that's how bios did it.
> -     */
> -    length_bytes = MAX(length_bytes, min_bytes);
> -
>       /* PkgLength is the length of the inclusive length of the data. */
>       length += length_bytes;
>
> @@ -220,15 +215,15 @@ void build_prepend_package_length(GArray *package, unsigned min_bytes)
>       build_prepend_byte(package, byte);
>   }
>
> -void build_package(GArray *package, uint8_t op, unsigned min_bytes)
> +void build_package(GArray *package, uint8_t op)
>   {
> -    build_prepend_package_length(package, min_bytes);
> +    build_prepend_package_length(package);
>       build_prepend_byte(package, op);
>   }
>
>   void build_extop_package(GArray *package, uint8_t op)
>   {
> -    build_package(package, op, 1);
> +    build_package(package, op);
>       build_prepend_byte(package, 0x5B); /* ExtOpPrefix */
>   }
>
> @@ -272,4 +267,3 @@ void build_append_int(GArray *table, uint32_t value)
>           build_append_value(table, value, 4);
>       }
>   }
> -
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index e3ac1a14..a010f3b 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -293,7 +293,7 @@ static void build_append_and_cleanup_method(GArray *device, GArray *method)
>   {
>       uint8_t op = 0x14; /* MethodOp */
>
> -    build_package(method, op, 0);
> +    build_package(method, op);
>
>       build_append_array(device, method);
>       build_free_array(method);
> @@ -314,7 +314,7 @@ static void build_append_notify_target_ifequal(GArray *method,
>       build_append_byte(notify, 0x69); /* Arg1Op */
>
>       /* Pack it up */
> -    build_package(notify, op, 1);
> +    build_package(notify, op);
>
>       build_append_array(method, notify);
>
> @@ -827,7 +827,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
>               build_append_byte(notify, 0x69); /* Arg1Op */
>
>               /* Pack it up */
> -            build_package(notify, op, 0);
> +            build_package(notify, op);
>
>               build_append_array(method, notify);
>
> @@ -868,7 +868,7 @@ static void build_pci_bus_end(PCIBus *bus, void *bus_state)
>           if (bus->parent_dev) {
>               build_extop_package(bus_table, op);
>           } else {
> -            build_package(bus_table, op, 0);
> +            build_package(bus_table, op);
>           }
>
>           /* Append our bus description to parent table */
> @@ -1008,7 +1008,7 @@ build_ssdt(GArray *table_data, GArray *linker,
>                   build_append_byte(package, b);
>               }
>
> -            build_package(package, op, 2);
> +            build_package(package, op);
>               build_append_array(sb_scope, package);
>               build_free_array(package);
>           }
> @@ -1056,8 +1056,7 @@ build_ssdt(GArray *table_data, GArray *linker,
>               build_append_array(sb_scope, hotplug_state.device_table);
>               build_pci_bus_state_cleanup(&hotplug_state);
>           }
> -
> -        build_package(sb_scope, op, 3);
> +        build_package(sb_scope, op);
>           build_append_array(table_data, sb_scope);
>           build_free_array(sb_scope);
>       }
> diff --git a/include/hw/acpi/acpi-build-utils.h b/include/hw/acpi/acpi-build-utils.h
> index fd50625..199f003 100644
> --- a/include/hw/acpi/acpi-build-utils.h
> +++ b/include/hw/acpi/acpi-build-utils.h
> @@ -14,8 +14,8 @@ void build_append_array(GArray *array, GArray *val);
>   void GCC_FMT_ATTR(2, 3)
>   build_append_namestring(GArray *array, const char *format, ...);
>
> -void build_prepend_package_length(GArray *package, unsigned min_bytes);
> -void build_package(GArray *package, uint8_t op, unsigned min_bytes);
> +void build_prepend_package_length(GArray *package);
> +void build_package(GArray *package, uint8_t op);
>   void build_append_value(GArray *table, uint32_t value, int size);
>   void build_append_int(GArray *table, uint32_t value);
>   void build_extop_package(GArray *package, uint8_t op);
>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>