Re: [PATCH 06/27] qapi acpi: Elide redundant has_FOO in generated C

[Igor Mammedov]

On Thu, 15 Sep 2022 22:42:56 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> The has_FOO for pointer-valued FOO are redundant, except for arrays.
> They are also a nuisance to work with.  Recent commit "qapi: Start to
> elide redundant has_FOO in generated C" provided the means to elide
> them step by step.  This is the step for qapi/acpi.py.
> 
> Said commit explains the transformation in more detail.  The invariant
> violations mentioned there do not occur here.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Ani Sinha <ani@anisinha.ca>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/acpi/core.c           | 14 +++++++-------
>  hw/acpi/cpu.c            |  1 -
>  hw/acpi/memory_hotplug.c |  1 -
>  scripts/qapi/schema.py   |  1 -
>  4 files changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index 3e811bf03c..6da275c599 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -185,7 +185,7 @@ static void acpi_table_install(const char unsigned *blob, 
> size_t bloblen,
>      changed_fields = 0;
>      ext_hdr->_length = cpu_to_le16(acpi_payload_size);
>  
> -    if (hdrs->has_sig) {
> +    if (hdrs->sig) {
>          strncpy(ext_hdr->sig, hdrs->sig, sizeof ext_hdr->sig);
>          ++changed_fields;
>      }
> @@ -204,11 +204,11 @@ static void acpi_table_install(const char unsigned 
> *blob, size_t bloblen,
>  
>      ext_hdr->checksum = 0;
>  
> -    if (hdrs->has_oem_id) {
> +    if (hdrs->oem_id) {
>          strncpy(ext_hdr->oem_id, hdrs->oem_id, sizeof ext_hdr->oem_id);
>          ++changed_fields;
>      }
> -    if (hdrs->has_oem_table_id) {
> +    if (hdrs->oem_table_id) {
>          strncpy(ext_hdr->oem_table_id, hdrs->oem_table_id,
>                  sizeof ext_hdr->oem_table_id);
>          ++changed_fields;
> @@ -217,7 +217,7 @@ static void acpi_table_install(const char unsigned *blob, 
> size_t bloblen,
>          ext_hdr->oem_revision = cpu_to_le32(hdrs->oem_rev);
>          ++changed_fields;
>      }
> -    if (hdrs->has_asl_compiler_id) {
> +    if (hdrs->asl_compiler_id) {
>          strncpy(ext_hdr->asl_compiler_id, hdrs->asl_compiler_id,
>                  sizeof ext_hdr->asl_compiler_id);
>          ++changed_fields;
> @@ -255,12 +255,12 @@ void acpi_table_add(const QemuOpts *opts, Error **errp)
>      if (!hdrs) {
>          goto out;
>      }
> -    if (hdrs->has_file == hdrs->has_data) {
> +    if (!hdrs->file == !hdrs->data) {
>          error_setg(errp, "'-acpitable' requires one of 'data' or 'file'");
>          goto out;
>      }
>  
> -    pathnames = g_strsplit(hdrs->has_file ? hdrs->file : hdrs->data, ":", 0);
> +    pathnames = g_strsplit(hdrs->file ?: hdrs->data, ":", 0);
>      if (pathnames == NULL || pathnames[0] == NULL) {
>          error_setg(errp, "'-acpitable' requires at least one pathname");
>          goto out;
> @@ -297,7 +297,7 @@ void acpi_table_add(const QemuOpts *opts, Error **errp)
>          close(fd);
>      }
>  
> -    acpi_table_install(blob, bloblen, hdrs->has_file, hdrs, errp);
> +    acpi_table_install(blob, bloblen, !!hdrs->file, hdrs, errp);
>  
>  out:
>      g_free(blob);
> diff --git a/hw/acpi/cpu.c b/hw/acpi/cpu.c
> index 3646dbfe68..4e580959a2 100644
> --- a/hw/acpi/cpu.c
> +++ b/hw/acpi/cpu.c
> @@ -35,7 +35,6 @@ static ACPIOSTInfo *acpi_cpu_device_status(int idx, 
> AcpiCpuStatus *cdev)
>          DeviceState *dev = DEVICE(cdev->cpu);
>          if (dev->id) {
>              info->device = g_strdup(dev->id);
> -            info->has_device = true;
>          }
>      }
>      return info;
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 0a7e89a13e..a7476330a8 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -44,7 +44,6 @@ static ACPIOSTInfo *acpi_memory_device_status(int slot, 
> MemStatus *mdev)
>          DeviceState *dev = DEVICE(mdev->dimm);
>          if (dev->id) {
>              info->device = g_strdup(dev->id);
> -            info->has_device = true;
>          }
>      }
>      return info;
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 3ae94300c4..5160a659b7 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -759,7 +759,6 @@ def need_has(self):
>          assert self.type
>          # Temporary hack to support dropping the has_FOO in reviewable chunks
>          opt_out = [
> -            'qapi/acpi.json',
>              'qapi/audio.json',
>              'qapi/block-core.json',
>              'qapi/block-export.json',