[PATCH v1 0/8] Remove redundant forward declarations in acpiview

[PATCH v1 0/8] Remove redundant forward declarations in acpiview

[Krzysztof Koch]

This patch series removes forward static function declarations in the
acpiview table parsers. After repositioning blocks of code, these forward
declarations are no longer needed and they increase the code size.

What is more, some of the existing ACPI table parsers don't have these
duplicate declarations, so this set of patches makes the code structure
consistent across all acpiview parsers.

Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/tree/612_remove_forward_decl_v1

Krzysztof Koch (8):
  ShellPkg: acpiview: RSDP: Remove redundant forward declarations
  ShellPkg: acpiview: FADT: Remove redundant forward declarations
  ShellPkg: acpiview: SPCR: Remove redundant forward declaration
  ShellPkg: acpiview: SRAT: Remove redundant forward declarations
  ShellPkg: acpiview: MADT: Remove redundant forward declarations
  ShellPkg: acpiview: IORT: Remove redundant forward declarations
  ShellPkg: acpiview: GTDT: Remove redundant forward declarations
  ShellPkg: acpiview: DBG2: Remove redundant forward declarations

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c |  62 +++-------
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c | 113 ++++++-------------
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c |  91 ++++++---------
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c |  58 +++-------
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 118 ++++++++------------
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c | 116 +++++++------------
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c |  98 ++++++----------
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c |  64 +++--------
 8 files changed, 236 insertions(+), 484 deletions(-)

--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

[PATCH v1 1/8] ShellPkg: acpiview: RSDP: Remove redundant forward declarations

[Krzysztof Koch]

Remove redundant forward function declarations by repositioning
blocks of code. This way the code structure is consistent across
ACPI table parsers and the code becomes more concise.

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
    v1:
    - remove redundant forward function declarations [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c | 116 ++++++++------------
 1 file changed, 43 insertions(+), 73 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
index 586de7cbfb12f856c0c735b6e295c1cc32eb2ceb..4bf928139a507d0b8f203ed0cbf0863cc2ec5de5 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
@@ -1,7 +1,7 @@
 /** @file
   RSDP table parser
 
-  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
+  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
@@ -28,7 +28,27 @@ EFIAPI
 ValidateRsdtAddress (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+  )
+{
+#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+  // Reference: Server Base Boot Requirements System Software on ARM Platforms
+  // Section: 4.2.1.1 RSDP
+  // Root System Description Pointer (RSDP), ACPI ? 5.2.5.
+  //   - Within the RSDP, the RsdtAddress field must be null (zero) and the
+  //     XsdtAddresss MUST be a valid, non-null, 64-bit value.
+  UINT32 RsdtAddr;
+
+  RsdtAddr = *(UINT32*)Ptr;
+
+  if (RsdtAddr != 0) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: Rsdt Address = 0x%p. This must be NULL on ARM Platforms.",
+      RsdtAddr
+      );
+  }
+#endif
+}
 
 /**
   This function validates the XSDT Address.
@@ -43,7 +63,27 @@ EFIAPI
 ValidateXsdtAddress (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+  )
+{
+#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+  // Reference: Server Base Boot Requirements System Software on ARM Platforms
+  // Section: 4.2.1.1 RSDP
+  // Root System Description Pointer (RSDP), ACPI ? 5.2.5.
+  //   - Within the RSDP, the RsdtAddress field must be null (zero) and the
+  //     XsdtAddresss MUST be a valid, non-null, 64-bit value.
+  UINT64 XsdtAddr;
+
+  XsdtAddr = *(UINT64*)Ptr;
+
+  if (XsdtAddr == 0) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: Xsdt Address = 0x%p. This must not be NULL on ARM Platforms.",
+      XsdtAddr
+      );
+  }
+#endif
+}
 
 /**
   An array describing the ACPI RSDP Table.
@@ -61,76 +101,6 @@ STATIC CONST ACPI_PARSER RsdpParser[] = {
   {L"Reserved", 3, 33, L"%x %x %x", Dump3Chars, NULL, NULL, NULL}
 };
 
-/**
-  This function validates the RSDT Address.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateRsdtAddress (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-  )
-{
-#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  // Reference: Server Base Boot Requirements System Software on ARM Platforms
-  // Section: 4.2.1.1 RSDP
-  // Root System Description Pointer (RSDP), ACPI ? 5.2.5.
-  //   - Within the RSDP, the RsdtAddress field must be null (zero) and the
-  //     XsdtAddresss MUST be a valid, non-null, 64-bit value.
-  UINT32 RsdtAddr;
-
-  RsdtAddr = *(UINT32*)Ptr;
-
-  if (RsdtAddr != 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: Rsdt Address = 0x%p. This must be NULL on ARM Platforms.",
-      RsdtAddr
-      );
-  }
-#endif
-}
-
-/**
-  This function validates the XSDT Address.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateXsdtAddress (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-  )
-{
-#if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  // Reference: Server Base Boot Requirements System Software on ARM Platforms
-  // Section: 4.2.1.1 RSDP
-  // Root System Description Pointer (RSDP), ACPI ? 5.2.5.
-  //   - Within the RSDP, the RsdtAddress field must be null (zero) and the
-  //     XsdtAddresss MUST be a valid, non-null, 64-bit value.
-  UINT64 XsdtAddr;
-
-  XsdtAddr = *(UINT64*)Ptr;
-
-  if (XsdtAddr == 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: Xsdt Address = 0x%p. This must not be NULL on ARM Platforms.",
-      XsdtAddr
-      );
-  }
-#endif
-}
-
 /**
   This function parses the ACPI RSDP table.
 
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v1 1/8] ShellPkg: acpiview: RSDP: Remove redundant forward declarations

[Alexei Fedorov]

[-- Attachment #1: Type: text/plain, Size: 54 bytes --]

Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>

[-- Attachment #2: Type: text/html, Size: 60 bytes --]

[PATCH v1 2/8] ShellPkg: acpiview: FADT: Remove redundant forward declarations

[Krzysztof Koch]

Remove redundant forward function declarations by repositioning
blocks of code. This way the code structure is consistent across
ACPI table parsers and the code becomes more concise.

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
    v1:
    - remove redundant forward function declarations [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c | 113 ++++++--------------
 1 file changed, 34 insertions(+), 79 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
index cee7ee0770433da96d6042d2f5d687903f4b5495..e40c9ef8ee4b3285faf8c6edf3cb6236ee367397 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
@@ -1,7 +1,7 @@
 /** @file
   FADT table parser
 
-  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
+  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
@@ -46,7 +46,17 @@ EFIAPI
 ValidateFirmwareCtrl (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+)
+{
+#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+  if (*(UINT32*)Ptr != 0) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: Firmware Control must be zero for ARM platforms."
+    );
+  }
+#endif
+}
 
 /**
   This function validates the X_Firmware Control Field.
@@ -61,7 +71,17 @@ EFIAPI
 ValidateXFirmwareCtrl (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+)
+{
+#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+  if (*(UINT64*)Ptr != 0) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: X Firmware Control must be zero for ARM platforms."
+    );
+  }
+#endif
+}
 
 /**
   This function validates the flags.
@@ -76,7 +96,17 @@ EFIAPI
 ValidateFlags (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+)
+{
+#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+  if (((*(UINT32*)Ptr) & HW_REDUCED_ACPI) == 0) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: HW_REDUCED_ACPI flag must be set for ARM platforms."
+    );
+  }
+#endif
+}
 
 /**
   An ACPI_PARSER array describing the ACPI FADT Table.
@@ -142,81 +172,6 @@ STATIC CONST ACPI_PARSER FadtParser[] = {
   {L"Hypervisor VendorIdentity", 8, 268, L"%lx", NULL, NULL, NULL, NULL}
 };
 
-/**
-  This function validates the Firmware Control Field.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateFirmwareCtrl (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-)
-{
-#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  if (*(UINT32*)Ptr != 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: Firmware Control must be zero for ARM platforms."
-    );
-  }
-#endif
-}
-
-/**
-  This function validates the X_Firmware Control Field.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateXFirmwareCtrl (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-)
-{
-#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  if (*(UINT64*)Ptr != 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: X Firmware Control must be zero for ARM platforms."
-    );
-  }
-#endif
-}
-
-/**
-  This function validates the flags.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateFlags (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-)
-{
-#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  if (((*(UINT32*)Ptr) & HW_REDUCED_ACPI) == 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: HW_REDUCED_ACPI flag must be set for ARM platforms."
-    );
-  }
-#endif
-}
-
 /**
   This function parses the ACPI FADT table.
   This function parses the FADT table and optionally traces the ACPI table fields.
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v1 2/8] ShellPkg: acpiview: FADT: Remove redundant forward declarations

[Alexei Fedorov]

[-- Attachment #1: Type: text/plain, Size: 54 bytes --]

Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>

[-- Attachment #2: Type: text/html, Size: 60 bytes --]

[PATCH v1 3/8] ShellPkg: acpiview: SPCR: Remove redundant forward declaration

[Krzysztof Koch]

Reposition blocks of code to remove redundant forward function
declarations in order to reduce the code size.

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
    v1:
    - remove redundant forward function declarations [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c | 98 +++++++-------------
 1 file changed, 34 insertions(+), 64 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c
index 1974a9c046e4a3bccccc55cf758184af097b2420..3b06b05dee8c056c6e009b9e485ccd35d4194e95 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c
@@ -1,7 +1,7 @@
 /** @file
   SPCR table parser
 
-  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
+  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
@@ -31,7 +31,23 @@ EFIAPI
 ValidateInterruptType (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+  )
+{
+#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+  UINT8 InterruptType;
+
+  InterruptType = *Ptr;
+
+  if (InterruptType !=
+        EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: InterruptType = %d. This must be 8 on ARM Platforms",
+      InterruptType
+      );
+  }
+#endif
+}
 
 /**
   This function validates the Irq.
@@ -46,7 +62,22 @@ EFIAPI
 ValidateIrq (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+  )
+{
+#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
+  UINT8 Irq;
+
+  Irq = *Ptr;
+
+  if (Irq != 0) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: Irq = %d. This must be zero on ARM Platforms\n",
+      Irq
+      );
+  }
+#endif
+}
 
 /**
   An ACPI_PARSER array describing the ACPI SPCR Table.
@@ -76,67 +107,6 @@ STATIC CONST ACPI_PARSER SpcrParser[] = {
   {L"Reserved", 4, 76, L"%x", NULL, NULL, NULL, NULL}
 };
 
-/**
-  This function validates the Interrupt Type.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateInterruptType (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-  )
-{
-#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  UINT8 InterruptType;
-
-  InterruptType = *Ptr;
-
-  if (InterruptType !=
-        EFI_ACPI_SERIAL_PORT_CONSOLE_REDIRECTION_TABLE_INTERRUPT_TYPE_GIC) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: InterruptType = %d. This must be 8 on ARM Platforms",
-      InterruptType
-      );
-  }
-#endif
-}
-
-/**
-  This function validates the Irq.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateIrq (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-  )
-{
-#if defined (MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
-  UINT8 Irq;
-
-  Irq = *Ptr;
-
-  if (Irq != 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: Irq = %d. This must be zero on ARM Platforms\n",
-      Irq
-      );
-  }
-#endif
-}
-
 /**
   This function parses the ACPI SPCR table.
   When trace is enabled this function parses the SPCR table and
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v1 3/8] ShellPkg: acpiview: SPCR: Remove redundant forward declaration

[Alexei Fedorov]

[-- Attachment #1: Type: text/plain, Size: 54 bytes --]

Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>

[-- Attachment #2: Type: text/html, Size: 60 bytes --]

[PATCH v1 4/8] ShellPkg: acpiview: SRAT: Remove redundant forward declarations

[Krzysztof Koch]

Remove redundant forward function declarations by repositioning
blocks of code. This way the code structure is consistent across
ACPI table parsers and the code becomes more concise.

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
    v1:
    - remove redundant forward function declarations [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c | 64 ++++++--------------
 1 file changed, 18 insertions(+), 46 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
index 075ff2a141a82b522e8aaedb7ad79249aaf5eaac..03d28d52842040e4872e204fc10ae3dba7861936 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c
@@ -1,7 +1,7 @@
 /** @file
   SRAT table parser
 
-  Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.
+  Copyright (c) 2016 - 2019, ARM Limited. All rights reserved.
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
   @par Reference(s):
@@ -32,7 +32,13 @@ EFIAPI
 ValidateSratReserved (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+  )
+{
+  if (*(UINT32*)Ptr != 1) {
+    IncrementErrorCount ();
+    Print (L"\nERROR: Reserved should be 1 for backward compatibility.\n");
+  }
+}
 
 /**
   This function traces the APIC Proximity Domain field.
@@ -44,9 +50,16 @@ STATIC
 VOID
 EFIAPI
 DumpSratApicProximity (
-  IN  CONST CHAR16*  Format,
-  IN  UINT8*         Ptr
-  );
+ IN CONST CHAR16* Format,
+ IN UINT8*        Ptr
+ )
+{
+  UINT32 ProximityDomain;
+
+  ProximityDomain = Ptr[0] | (Ptr[1] << 8) | (Ptr[2] << 16);
+
+  Print (Format, ProximityDomain);
+}
 
 /**
   An ACPI_PARSER array describing the SRAT Table.
@@ -139,47 +152,6 @@ STATIC CONST ACPI_PARSER SratX2ApciAffinityParser[] = {
   {L"Reserved", 4, 20, L"0x%x", NULL, NULL, NULL, NULL}
 };
 
-/** This function validates the Reserved field in the SRAT table header.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateSratReserved (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-  )
-{
-  if (*(UINT32*)Ptr != 1) {
-    IncrementErrorCount ();
-    Print (L"\nERROR: Reserved should be 1 for backward compatibility.\n");
-  }
-}
-
-/**
-  This function traces the APIC Proximity Domain field.
-
-  @param [in] Format  Format string for tracing the data.
-  @param [in] Ptr     Pointer to the start of the buffer.
-**/
-STATIC
-VOID
-EFIAPI
-DumpSratApicProximity (
- IN CONST CHAR16* Format,
- IN UINT8*        Ptr
- )
-{
-  UINT32 ProximityDomain;
-
-  ProximityDomain = Ptr[0] | (Ptr[1] << 8) | (Ptr[2] << 16);
-
-  Print (Format, ProximityDomain);
-}
-
 /**
   This function parses the ACPI SRAT table.
   When trace is enabled this function parses the SRAT table and
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v1 4/8] ShellPkg: acpiview: SRAT: Remove redundant forward declarations

[Alexei Fedorov]

[-- Attachment #1: Type: text/plain, Size: 54 bytes --]

Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>

[-- Attachment #2: Type: text/html, Size: 60 bytes --]

[PATCH v1 5/8] ShellPkg: acpiview: MADT: Remove redundant forward declarations

[Krzysztof Koch]

Remove redundant forward function declarations by repositioning
blocks of code. This way the code structure is consistent across
ACPI table parsers and the code becomes more concise.

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
    v1:
    - remove redundant forward function declarations [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 118 ++++++++------------
 1 file changed, 44 insertions(+), 74 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
index 59c3df0cc8a080497b517baf36fc63f1e4ab866f..338295d30e35c366a60505225cf57145a8e73d93 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c
@@ -35,7 +35,15 @@ EFIAPI
 ValidateGICDSystemVectorBase (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+)
+{
+  if (*(UINT32*)Ptr != 0) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: System Vector Base must be zero."
+    );
+  }
+}
 
 /**
   This function validates the SPE Overflow Interrupt in the GICC.
@@ -50,7 +58,41 @@ EFIAPI
 ValidateSpeOverflowInterrupt (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+  )
+{
+  UINT16 SpeOverflowInterrupt;
+
+  SpeOverflowInterrupt = *(UINT16*)Ptr;
+
+  // SPE not supported by this processor
+  if (SpeOverflowInterrupt == 0) {
+    return;
+  }
+
+  if ((SpeOverflowInterrupt < ARM_PPI_ID_MIN) ||
+      ((SpeOverflowInterrupt > ARM_PPI_ID_MAX) &&
+       (SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MIN)) ||
+      (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MAX)) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: SPE Overflow Interrupt ID of %d is not in the allowed PPI ID "
+        L"ranges of %d-%d or %d-%d (for GICv3.1 or later).",
+      SpeOverflowInterrupt,
+      ARM_PPI_ID_MIN,
+      ARM_PPI_ID_MAX,
+      ARM_PPI_ID_EXTENDED_MIN,
+      ARM_PPI_ID_EXTENDED_MAX
+    );
+  } else if (SpeOverflowInterrupt != ARM_PPI_ID_PMBIRQ) {
+    IncrementWarningCount();
+    Print (
+      L"\nWARNING: SPE Overflow Interrupt ID of %d is not compliant with SBSA "
+        L"Level 3 PPI ID assignment: %d.",
+      SpeOverflowInterrupt,
+      ARM_PPI_ID_PMBIRQ
+    );
+  }
+}
 
 /**
   An ACPI_PARSER array describing the GICC Interrupt Controller Structure.
@@ -158,78 +200,6 @@ STATIC CONST ACPI_PARSER MadtInterruptControllerHeaderParser[] = {
   {L"Reserved", 2, 2, NULL, NULL, NULL, NULL, NULL}
 };
 
-/**
-  This function validates the System Vector Base in the GICD.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateGICDSystemVectorBase (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-)
-{
-  if (*(UINT32*)Ptr != 0) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: System Vector Base must be zero."
-    );
-  }
-}
-
-/**
-  This function validates the SPE Overflow Interrupt in the GICC.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateSpeOverflowInterrupt (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-  )
-{
-  UINT16 SpeOverflowInterrupt;
-
-  SpeOverflowInterrupt = *(UINT16*)Ptr;
-
-  // SPE not supported by this processor
-  if (SpeOverflowInterrupt == 0) {
-    return;
-  }
-
-  if ((SpeOverflowInterrupt < ARM_PPI_ID_MIN) ||
-      ((SpeOverflowInterrupt > ARM_PPI_ID_MAX) &&
-       (SpeOverflowInterrupt < ARM_PPI_ID_EXTENDED_MIN)) ||
-      (SpeOverflowInterrupt > ARM_PPI_ID_EXTENDED_MAX)) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: SPE Overflow Interrupt ID of %d is not in the allowed PPI ID "
-        L"ranges of %d-%d or %d-%d (for GICv3.1 or later).",
-      SpeOverflowInterrupt,
-      ARM_PPI_ID_MIN,
-      ARM_PPI_ID_MAX,
-      ARM_PPI_ID_EXTENDED_MIN,
-      ARM_PPI_ID_EXTENDED_MAX
-    );
-  } else if (SpeOverflowInterrupt != ARM_PPI_ID_PMBIRQ) {
-    IncrementWarningCount();
-    Print (
-      L"\nWARNING: SPE Overflow Interrupt ID of %d is not compliant with SBSA "
-        L"Level 3 PPI ID assignment: %d.",
-      SpeOverflowInterrupt,
-      ARM_PPI_ID_PMBIRQ
-    );
-  }
-}
-
 /**
   This function parses the ACPI MADT table.
   When trace is enabled this function parses the MADT table and
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v1 5/8] ShellPkg: acpiview: MADT: Remove redundant forward declarations

[Alexei Fedorov]

[-- Attachment #1: Type: text/plain, Size: 54 bytes --]

Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>

[-- Attachment #2: Type: text/html, Size: 60 bytes --]

[PATCH v1 6/8] ShellPkg: acpiview: IORT: Remove redundant forward declarations

[Krzysztof Koch]

Remove redundant forward function declarations by repositioning
blocks of code. This way the code structure is consistent across
ACPI table parsers and the code becomes more concise.

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
    v1:
    - remove redundant forward function declarations [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | 58 +++++---------------
 1 file changed, 14 insertions(+), 44 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
index 93f78e1a9786ed53f6b5529f478b72a220b4f8df..0461205b4f8bc02aa11ab1db1d0deb73bce053b7 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
@@ -45,7 +45,13 @@ EFIAPI
 ValidateItsIdMappingCount (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+  )
+{
+  if (*(UINT32*)Ptr != 0) {
+    IncrementErrorCount ();
+    Print (L"\nERROR: IORT ID Mapping count must be zero.");
+  }
+}
 
 /**
   This function validates the ID Mapping array offset for the ITS node.
@@ -60,7 +66,13 @@ EFIAPI
 ValidateItsIdArrayReference (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+  )
+{
+  if (*(UINT32*)Ptr != 0) {
+    IncrementErrorCount ();
+    Print (L"\nERROR: IORT ID Mapping offset must be zero.");
+  }
+}
 
 /**
   Helper Macro for populating the IORT Node header in the ACPI_PARSER array.
@@ -210,48 +222,6 @@ STATIC CONST ACPI_PARSER IortNodePmcgParser[] = {
   {L"Node reference", 4, 28, L"0x%x", NULL, NULL, NULL, NULL},
 };
 
-/**
-  This function validates the ID Mapping array count for the ITS node.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateItsIdMappingCount (
-  IN UINT8* Ptr,
-  IN VOID*     Context
-  )
-{
-  if (*(UINT32*)Ptr != 0) {
-    IncrementErrorCount ();
-    Print (L"\nERROR: IORT ID Mapping count must be zero.");
-  }
-}
-
-/**
-  This function validates the ID Mapping array offset for the ITS node.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateItsIdArrayReference (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-  )
-{
-  if (*(UINT32*)Ptr != 0) {
-    IncrementErrorCount ();
-    Print (L"\nERROR: IORT ID Mapping offset must be zero.");
-  }
-}
-
 /**
   This function parses the IORT Node Id Mapping array.
 
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v1 6/8] ShellPkg: acpiview: IORT: Remove redundant forward declarations

[Alexei Fedorov]

[-- Attachment #1: Type: text/plain, Size: 54 bytes --]

Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>

[-- Attachment #2: Type: text/html, Size: 60 bytes --]

[PATCH v1 7/8] ShellPkg: acpiview: GTDT: Remove redundant forward declarations

[Krzysztof Koch]

Remove redundant forward function declarations by repositioning
blocks of code. This way the code structure is consistent across
ACPI table parsers and the code becomes more concise.

Replace multple use of literal values for GT Block Timer Frame
count/number validation with a macro definition.

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
    v1:
    - remove redundant forward function declarations [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 91 +++++++-------------
 1 file changed, 33 insertions(+), 58 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
index 3b05ff3015d4a3af62dd9fab057c32369a456267..1e5b5764f50a2d29aa904c889bc89af5bdc3af5c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
@@ -13,6 +13,9 @@
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
 
+// "The number of GT Block Timers must be less than or equal to 8"
+#define GT_BLOCK_TIMER_COUNT_MAX 8
+
 // Local variables
 STATIC CONST UINT32* GtdtPlatformTimerCount;
 STATIC CONST UINT32* GtdtPlatformTimerOffset;
@@ -36,7 +39,21 @@ EFIAPI
 ValidateGtBlockTimerCount (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+  )
+{
+  UINT32 BlockTimerCount;
+
+  BlockTimerCount = *(UINT32*)Ptr;
+
+  if (BlockTimerCount > GT_BLOCK_TIMER_COUNT_MAX) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: Timer Count = %d. Max Timer Count is %d.",
+      BlockTimerCount,
+      GT_BLOCK_TIMER_COUNT_MAX
+      );
+  }
+}
 
 /**
   This function validates the GT Frame Number.
@@ -51,7 +68,21 @@ EFIAPI
 ValidateGtFrameNumber (
   IN UINT8* Ptr,
   IN VOID*  Context
-  );
+  )
+{
+  UINT8 FrameNumber;
+
+  FrameNumber = *(UINT8*)Ptr;
+
+  if (FrameNumber >= GT_BLOCK_TIMER_COUNT_MAX) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: GT Frame Number = %d. GT Frame Number must be in range 0-%d.",
+      FrameNumber,
+      GT_BLOCK_TIMER_COUNT_MAX - 1
+      );
+  }
+}
 
 /**
   An ACPI_PARSER array describing the ACPI GTDT Table.
@@ -134,62 +165,6 @@ STATIC CONST ACPI_PARSER SBSAGenericWatchdogParser[] = {
   {L"Watchdog Timer Flags", 4, 24, L"0x%x", NULL, NULL, NULL, NULL}
 };
 
-/**
-  This function validates the GT Block timer count.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateGtBlockTimerCount (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-  )
-{
-  UINT32 BlockTimerCount;
-
-  BlockTimerCount = *(UINT32*)Ptr;
-
-  if (BlockTimerCount > 8) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: Timer Count = %d. Max Timer Count is 8.",
-      BlockTimerCount
-      );
-  }
-}
-
-/**
-  This function validates the GT Frame Number.
-
-  @param [in] Ptr     Pointer to the start of the field data.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateGtFrameNumber (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-  )
-{
-  UINT8 FrameNumber;
-
-  FrameNumber = *(UINT8*)Ptr;
-
-  if (FrameNumber > 7) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: GT Frame Number = %d. GT Frame Number must be in range 0-7.",
-      FrameNumber
-      );
-  }
-}
-
 /**
   This function parses the Platform GT Block.
 
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v1 7/8] ShellPkg: acpiview: GTDT: Remove redundant forward declarations

[Alexei Fedorov]

[-- Attachment #1: Type: text/plain, Size: 54 bytes --]

Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>

[-- Attachment #2: Type: text/html, Size: 60 bytes --]

[PATCH v1 8/8] ShellPkg: acpiview: DBG2: Remove redundant forward declarations

[Krzysztof Koch]

Remove redundant forward function declarations by repositioning
blocks of code. This way the code structure is consistent across
ACPI table parsers and the code becomes more concise.

Signed-off-by: Krzysztof Koch <krzysztof.koch@arm.com>
---

Notes:
    v1:
    - remove redundant forward function declarations [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c | 62 +++++---------------
 1 file changed, 16 insertions(+), 46 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
index 8de5ebf74775bab8e765849cba6ef4eb6f659a5a..1efcbd40f86efdabed2152540a415db8a950fb71 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
@@ -27,7 +27,7 @@ STATIC CONST UINT16* AddrSizeOffset;
 STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
 
 /**
-  This function Validates the NameSpace string length.
+  This function validates the NameSpace string length.
 
   @param [in] Ptr     Pointer to the start of the buffer.
   @param [in] Context Pointer to context specific information e.g. this
@@ -37,24 +37,23 @@ STATIC
 VOID
 EFIAPI
 ValidateNameSpaceStrLen (
-  IN  UINT8* Ptr,
-  IN  VOID*  Context
-  );
+  IN UINT8* Ptr,
+  IN VOID*  Context
+  )
+{
+  UINT16 NameSpaceStrLen;
 
-/**
-  This function parses the debug device information structure.
+  NameSpaceStrLen = *(UINT16*)Ptr;
 
-  @param [in]  Ptr     Pointer to the start of the buffer.
-  @param [out] Length  Pointer in which the length of the debug
-                       device information is returned.
-**/
-STATIC
-VOID
-EFIAPI
-DumpDbgDeviceInfo (
-  IN  UINT8*  Ptr,
-  OUT UINT32* Length
-  );
+  if (NameSpaceStrLen < 2) {
+    IncrementErrorCount ();
+    Print (
+      L"\nERROR: NamespaceString Length = %d. If no Namespace device exists, " \
+        L"NamespaceString[] must contain a period '.'",
+      NameSpaceStrLen
+      );
+  }
+}
 
 /// An ACPI_PARSER array describing the ACPI DBG2 table.
 STATIC CONST ACPI_PARSER Dbg2Parser[] = {
@@ -91,35 +90,6 @@ STATIC CONST ACPI_PARSER DbgDevInfoParser[] = {
    (VOID**)&AddrSizeOffset, NULL, NULL}
 };
 
-/**
-  This function validates the NameSpace string length.
-
-  @param [in] Ptr     Pointer to the start of the buffer.
-  @param [in] Context Pointer to context specific information e.g. this
-                      could be a pointer to the ACPI table header.
-**/
-STATIC
-VOID
-EFIAPI
-ValidateNameSpaceStrLen (
-  IN UINT8* Ptr,
-  IN VOID*  Context
-  )
-{
-  UINT16 NameSpaceStrLen;
-
-  NameSpaceStrLen = *(UINT16*)Ptr;
-
-  if (NameSpaceStrLen < 2) {
-    IncrementErrorCount ();
-    Print (
-      L"\nERROR: NamespaceString Length = %d. If no Namespace device exists,\n"
-       L"    then NamespaceString[] must contain a period '.'",
-      NameSpaceStrLen
-      );
-  }
-}
-
 /**
   This function parses the debug device information structure.
 
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v1 8/8] ShellPkg: acpiview: DBG2: Remove redundant forward declarations

[Alexei Fedorov]

[-- Attachment #1: Type: text/plain, Size: 54 bytes --]

Reviewed-by: Alexei Fedorov <Alexei.Fedorov@arm.com>

[-- Attachment #2: Type: text/html, Size: 60 bytes --]

Re: [PATCH v1 0/8] Remove redundant forward declarations in acpiview

[Sami Mujawar]

Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>


-----Original Message-----
From: Krzysztof Koch <krzysztof.koch@arm.com> 
Sent: 18 July 2019 11:05 AM
To: devel@edk2.groups.io
Cc: jaben.carsey@intel.com; ray.ni@intel.com; zhichao.gao@intel.com; Sami Mujawar <Sami.Mujawar@arm.com>; Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>
Subject: [PATCH v1 0/8] Remove redundant forward declarations in acpiview

This patch series removes forward static function declarations in the acpiview table parsers. After repositioning blocks of code, these forward declarations are no longer needed and they increase the code size.

What is more, some of the existing ACPI table parsers don't have these duplicate declarations, so this set of patches makes the code structure consistent across all acpiview parsers.

Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/tree/612_remove_forward_decl_v1

Krzysztof Koch (8):
  ShellPkg: acpiview: RSDP: Remove redundant forward declarations
  ShellPkg: acpiview: FADT: Remove redundant forward declarations
  ShellPkg: acpiview: SPCR: Remove redundant forward declaration
  ShellPkg: acpiview: SRAT: Remove redundant forward declarations
  ShellPkg: acpiview: MADT: Remove redundant forward declarations
  ShellPkg: acpiview: IORT: Remove redundant forward declarations
  ShellPkg: acpiview: GTDT: Remove redundant forward declarations
  ShellPkg: acpiview: DBG2: Remove redundant forward declarations

 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c |  62 +++-------  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c | 113 ++++++-------------  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c |  91 ++++++---------  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c |  58 +++-------  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 118 ++++++++------------  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c | 116 +++++++------------  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c |  98 ++++++----------  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c |  64 +++--------
 8 files changed, 236 insertions(+), 484 deletions(-)

--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [PATCH v1 0/8] Remove redundant forward declarations in acpiview

[Carsey, Jaben]

Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>

Thanks
-Jaben

> -----Original Message-----
> From: Krzysztof Koch [mailto:krzysztof.koch@arm.com]
> Sent: Thursday, July 18, 2019 3:05 AM
> To: devel@edk2.groups.io
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Gao, Zhichao <zhichao.gao@intel.com>; Sami.Mujawar@arm.com;
> Matteo.Carlini@arm.com; nd@arm.com
> Subject: [PATCH v1 0/8] Remove redundant forward declarations in acpiview
> 
> This patch series removes forward static function declarations in the
> acpiview table parsers. After repositioning blocks of code, these forward
> declarations are no longer needed and they increase the code size.
> 
> What is more, some of the existing ACPI table parsers don't have these
> duplicate declarations, so this set of patches makes the code structure
> consistent across all acpiview parsers.
> 
> Changes can be seen at:
> https://github.com/KrzysztofKoch1/edk2/tree/612_remove_forward_decl_
> v1
> 
> Krzysztof Koch (8):
>   ShellPkg: acpiview: RSDP: Remove redundant forward declarations
>   ShellPkg: acpiview: FADT: Remove redundant forward declarations
>   ShellPkg: acpiview: SPCR: Remove redundant forward declaration
>   ShellPkg: acpiview: SRAT: Remove redundant forward declarations
>   ShellPkg: acpiview: MADT: Remove redundant forward declarations
>   ShellPkg: acpiview: IORT: Remove redundant forward declarations
>   ShellPkg: acpiview: GTDT: Remove redundant forward declarations
>   ShellPkg: acpiview: DBG2: Remove redundant forward declarations
> 
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> |  62 +++-------
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> | 113 ++++++-------------
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> |  91 ++++++---------
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c |
> 58 +++-------
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> c | 118 ++++++++------------
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
> | 116 +++++++------------
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c
> |  98 ++++++----------
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c |
> 64 +++--------
>  8 files changed, 236 insertions(+), 484 deletions(-)
> 
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 

Re: [PATCH v1 0/8] Remove redundant forward declarations in acpiview

[Gao, Zhichao]

I am reviewing your previous patch "Add security checks in the Acpiview table parsers". That takes times and I didn't finish reviewing them yet.
You do a pretty good separation. That makes the review more easier. I would give some comments on your previous one 10/10 patches to help your further work after I finish reviewing them.
This patch set is good for me. 
Reviewed-by: Zhichao Gao <zhichao.gao@intel.com>

Thanks,
Zhichao

> -----Original Message-----
> From: Krzysztof Koch [mailto:krzysztof.koch@arm.com]
> Sent: Thursday, July 18, 2019 6:05 PM
> To: devel@edk2.groups.io
> Cc: Carsey, Jaben <jaben.carsey@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Gao, Zhichao <zhichao.gao@intel.com>; Sami.Mujawar@arm.com;
> Matteo.Carlini@arm.com; nd@arm.com
> Subject: [PATCH v1 0/8] Remove redundant forward declarations in acpiview
> 
> This patch series removes forward static function declarations in the acpiview
> table parsers. After repositioning blocks of code, these forward declarations
> are no longer needed and they increase the code size.
> 
> What is more, some of the existing ACPI table parsers don't have these
> duplicate declarations, so this set of patches makes the code structure
> consistent across all acpiview parsers.
> 
> Changes can be seen at:
> https://github.com/KrzysztofKoch1/edk2/tree/612_remove_forward_decl_
> v1
> 
> Krzysztof Koch (8):
>   ShellPkg: acpiview: RSDP: Remove redundant forward declarations
>   ShellPkg: acpiview: FADT: Remove redundant forward declarations
>   ShellPkg: acpiview: SPCR: Remove redundant forward declaration
>   ShellPkg: acpiview: SRAT: Remove redundant forward declarations
>   ShellPkg: acpiview: MADT: Remove redundant forward declarations
>   ShellPkg: acpiview: IORT: Remove redundant forward declarations
>   ShellPkg: acpiview: GTDT: Remove redundant forward declarations
>   ShellPkg: acpiview: DBG2: Remove redundant forward declarations
> 
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c
> |  62 +++-------
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c
> | 113 ++++++-------------
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c
> |  91 ++++++---------
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c |
> 58 +++-------
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.
> c | 118 ++++++++------------
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c
> | 116 +++++++------------
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Spcr/SpcrParser.c |
> 98 ++++++----------
> ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c |
> 64 +++--------
>  8 files changed, 236 insertions(+), 484 deletions(-)
> 
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
>