Re: [PATCH v1 1/1] ShellPkg: acpiview: Remove duplicate ACPI structure size definitions

["Gao, Zhichao" <zhichao.gao@intel.com>]

Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: Krzysztof Koch <krzysztof.koch@arm.com>
> Sent: Thursday, January 30, 2020 7:43 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Gao, Zhichao <zhichao.gao@intel.com>;
> Matteo.Carlini@arm.com; Sami.Mujawar@arm.com; nd@arm.com
> Subject: [PATCH v1 1/1] ShellPkg: acpiview: Remove duplicate ACPI structure
> size definitions
> 
> Remove duplicate Generic Address Structure (GAS) and ACPI table header size
> definitions. Replace any dependencies with the definitions from Acpi63.h.
> 
> AcpiParser.h redefines the size of Generic Address Structure (GAS) and the ACPI
> table header. The motivation is to 'minimise any reference to the EDK2 ACPI
> headers in an attempt to provide cross checking'. While the redefined values are
> unlikely to change, redefinition causes unnecessary maintenance overhead for
> acpiview.
> 
> In addition to this, EDK2 ACPI headers are referenced throughout the entire
> project, so the assumption should be that the definitions in these header files
> are correct.
> 
> Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
> ---
> 
> Changes can be seen at:
> https://github.com/tianocore/edk2/compare/master...KrzysztofKoch1:remove_
> dup_defs_acpiview_v1?expand=1
> 
> Notes:
>     v1:
>     - Remove duplicate definitions [Krzysztof]
> 
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c |  8 +++----
> ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h | 22 +---------------
> ----
>  2 files changed, 5 insertions(+), 25 deletions(-)
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> index
> 2b2ecb93cef9ee28b752e7bf2d920b059dbf7d6b..ed8767da64280d0b02fe42b7d
> 2120690d5573bba 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> @@ -1,7 +1,7 @@
>  /** @file
>    ACPI parser
> 
> -  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
> +  Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
>    SPDX-License-Identifier: BSD-2-Clause-Patent  **/
> 
> @@ -666,7 +666,7 @@ DumpGas (
>    IN UINT8*        Ptr
>    )
>  {
> -  DumpGasStruct (Ptr, 2, GAS_LENGTH);
> +  DumpGasStruct (Ptr, 2, sizeof
> + (EFI_ACPI_6_3_GENERIC_ADDRESS_STRUCTURE));
>  }
> 
>  /**
> @@ -687,7 +687,7 @@ DumpAcpiHeader (
>             0,
>             "ACPI Table Header",
>             Ptr,
> -           ACPI_DESCRIPTION_HEADER_LENGTH,
> +           sizeof (EFI_ACPI_DESCRIPTION_HEADER),
>             PARSER_PARAMS (AcpiHeaderParser)
>             );
>  }
> @@ -721,7 +721,7 @@ ParseAcpiHeader (
>                    0,
>                    NULL,
>                    Ptr,
> -                  ACPI_DESCRIPTION_HEADER_LENGTH,
> +                  sizeof (EFI_ACPI_DESCRIPTION_HEADER),
>                    PARSER_PARAMS (AcpiHeaderParser)
>                    );
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> index
> 6deee3542e09bc18b03fdd18dece5b6f9725839c..f81ccac7e118378aa185db4b6
> 25e5bcd75f78347 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> @@ -1,7 +1,7 @@
>  /** @file
>    Header file for ACPI parser
> 
> -  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
> +  Copyright (c) 2016 - 2020, ARM Limited. All rights reserved.
>    SPDX-License-Identifier: BSD-2-Clause-Patent  **/
> 
> @@ -396,26 +396,6 @@ ParseAcpi (
>    { L"Creator Revision", 4, 32, L"0x%X", NULL,    \
>      (VOID**)&(Info)->CreatorRevision, NULL, NULL }
> 
> -/**
> -  Length of the ACPI GAS structure.
> -
> -  NOTE: This might normally be defined as
> -        sizeof (EFI_ACPI_6_2_GENERIC_ADDRESS_STRUCTURE).
> -        However, we deliberately minimise any reference to the EDK2 ACPI
> -        headers in an attempt to provide cross checking.
> -**/
> -#define GAS_LENGTH                     12
> -
> -/**
> -  Length of the ACPI Header structure.
> -
> -  NOTE: This might normally be defined as
> -        sizeof (EFI_ACPI_DESCRIPTION_HEADER).
> -        However, we deliberately minimise any reference to the EDK2 ACPI
> -        headers in an attempt to provide cross checking.
> -**/
> -#define ACPI_DESCRIPTION_HEADER_LENGTH  36
> -
>  /**
>    This function indents and traces the GAS structure as described by the
> GasParser.
> 
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>