Re: [edk2-devel] [PATCH v3 2/7] ShellPkg/AcpiView: Refactor configuration

2020-06-22 Thread Gao, Zhichao
See below.

> -Original Message-
> From: Tomas Pilar 
> Sent: Monday, June 15, 2020 10:04 PM
> To: devel@edk2.groups.io
> Cc: n...@arm.com; Ni, Ray ; Gao, Zhichao
> 
> Subject: [PATCH v3 2/7] ShellPkg/AcpiView: Refactor configuration
> 
> A new file and header (AcpiViewConfig.[ch]) is created that houses the user
> configuration. This declutters the core code and improves modularity of the
> design.
> 
> The module level symbols for verbosity, table selection, and highlighting are
> refactored into the new file.
> 
> Cc: Ray Ni 
> Cc: Zhichao Gao 
> Signed-off-by: Tomas Pilar 
> ---
>  .../UefiShellAcpiViewCommandLib/AcpiParser.c  |   1 +
>  .../AcpiTableParser.c |   1 +
>  .../UefiShellAcpiViewCommandLib/AcpiView.c| 237 +++--
>  .../UefiShellAcpiViewCommandLib/AcpiView.h|  95 ---
>  .../AcpiViewConfig.c  | 246 ++
>  .../AcpiViewConfig.h  | 177 +
>  .../Parsers/Gtdt/GtdtParser.c |   1 +
>  .../Parsers/Iort/IortParser.c |   1 +
>  .../Parsers/Madt/MadtParser.c |   1 +
>  .../Parsers/Pptt/PpttParser.c |   1 +
>  .../Parsers/Srat/SratParser.c |   1 +
>  .../UefiShellAcpiViewCommandLib.inf   |  32 +--
>  12 files changed, 479 insertions(+), 315 deletions(-)  create mode 100644
> ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewConfig.c
>  create mode 100644
> ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewConfig.h
> 
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> index 3f12a33050a4..02f6d771c7e1 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> @@ -10,6 +10,7 @@
>  #include 
>  #include "AcpiParser.h"
>  #include "AcpiView.h"
> +#include "AcpiViewConfig.h"
> 
>  STATIC UINT32   gIndent;
>  STATIC UINT32   mTableErrorCount;
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> index d5b9eee52323..4b618f131eac 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> @@ -17,6 +17,7 @@
>  #include "AcpiParser.h"
>  #include "AcpiTableParser.h"
>  #include "AcpiView.h"
> +#include "AcpiViewConfig.h"
> 
>  #if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)  #include
> "Arm/SbbrValidator.h"
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> index f1a95b7b8f03..390e378e9a6c 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> @@ -20,6 +20,7 @@
>  #include "AcpiParser.h"
>  #include "AcpiTableParser.h"
>  #include "AcpiView.h"
> +#include "AcpiViewConfig.h"
>  #include "UefiShellAcpiViewCommandLib.h"
> 
>  #if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64) @@ -28,17 +29,8
> @@
> 
>  EFI_HII_HANDLE gShellAcpiViewHiiHandle = NULL;
> 
> -// Report variables
> -STATIC UINT32 mSelectedAcpiTable;
> -STATIC CONST CHAR16*  mSelectedAcpiTableName;
> -STATIC BOOLEANmSelectedAcpiTableFound;
> -STATIC EREPORT_OPTION mReportType;
>  STATIC UINT32 mTableCount;
>  STATIC UINT32 mBinTableCount;
> -STATIC BOOLEANmConsistencyCheck;
> -STATIC BOOLEANmColourHighlighting;
> -STATIC BOOLEANmMandatoryTableValidate;
> -STATIC UINTN  mMandatoryTableSpec;
> 
>  /**
>An array of acpiview command line parameters.
> @@ -53,142 +45,6 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
>{NULL, TypeMax}
>  };
> 
> -/**
> -  This function returns the colour highlighting status.
> -
> -  @retval TRUE if colour highlighting is enabled.
> -**/
> -BOOLEAN
> -GetColourHighlighting (
> -  VOID
> -  )
> -{
> -  return mColourHighlighting;
> -}
> -
> -/**
> -  This function sets the colour highlighting status.
> -
> -  @param  Highlight   The Highlight status.
> -
> -**/
> -VOID
> -SetColourHighlighting (
> -  BOOLEAN Highlight
> -  )
> -{
> -  mColourHighlighting = Highlight;
> -}
> -
> -/**
> -  This function returns the consistency checking status.
> -
> -  @retval TRUE if consistency checking is enabled.
> -**/
> -BOOLEAN
> -GetConsistencyChecking (
> -  VOID
> -  )
> -{
> -  return mConsistencyCheck;
> -}
> -
> -/**
> -  This function sets the consistency checking status.
> -
> -  @param  ConsistencyChecking   The consistency checking status.
> -
> -**/
> -VOID
> -SetConsistencyChecking (
> -  BOOLEAN ConsistencyChecking
> -  )
> -{
> -  mConsistencyCheck = ConsistencyChecking; -}
> -
> -/**
> -  This function returns the ACPI table requirements validation flag.
> -
> -  @retval TRUE if 

[edk2-devel] [PATCH v3 2/7] ShellPkg/AcpiView: Refactor configuration

2020-06-15 Thread Tomas Pilar (tpilar)
A new file and header (AcpiViewConfig.[ch]) is created
that houses the user configuration. This declutters the
core code and improves modularity of the design.

The module level symbols for verbosity, table selection, and
highlighting are refactored into the new file.

Cc: Ray Ni 
Cc: Zhichao Gao 
Signed-off-by: Tomas Pilar 
---
 .../UefiShellAcpiViewCommandLib/AcpiParser.c  |   1 +
 .../AcpiTableParser.c |   1 +
 .../UefiShellAcpiViewCommandLib/AcpiView.c| 237 +++--
 .../UefiShellAcpiViewCommandLib/AcpiView.h|  95 ---
 .../AcpiViewConfig.c  | 246 ++
 .../AcpiViewConfig.h  | 177 +
 .../Parsers/Gtdt/GtdtParser.c |   1 +
 .../Parsers/Iort/IortParser.c |   1 +
 .../Parsers/Madt/MadtParser.c |   1 +
 .../Parsers/Pptt/PpttParser.c |   1 +
 .../Parsers/Srat/SratParser.c |   1 +
 .../UefiShellAcpiViewCommandLib.inf   |  32 +--
 12 files changed, 479 insertions(+), 315 deletions(-)
 create mode 100644 
ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewConfig.c
 create mode 100644 
ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewConfig.h

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index 3f12a33050a4..02f6d771c7e1 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -10,6 +10,7 @@
 #include 
 #include "AcpiParser.h"
 #include "AcpiView.h"
+#include "AcpiViewConfig.h"
 
 STATIC UINT32   gIndent;
 STATIC UINT32   mTableErrorCount;
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
index d5b9eee52323..4b618f131eac 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
@@ -17,6 +17,7 @@
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
 #include "AcpiView.h"
+#include "AcpiViewConfig.h"
 
 #if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
 #include "Arm/SbbrValidator.h"
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c 
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
index f1a95b7b8f03..390e378e9a6c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
@@ -20,6 +20,7 @@
 #include "AcpiParser.h"
 #include "AcpiTableParser.h"
 #include "AcpiView.h"
+#include "AcpiViewConfig.h"
 #include "UefiShellAcpiViewCommandLib.h"
 
 #if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
@@ -28,17 +29,8 @@
 
 EFI_HII_HANDLE gShellAcpiViewHiiHandle = NULL;
 
-// Report variables
-STATIC UINT32 mSelectedAcpiTable;
-STATIC CONST CHAR16*  mSelectedAcpiTableName;
-STATIC BOOLEANmSelectedAcpiTableFound;
-STATIC EREPORT_OPTION mReportType;
 STATIC UINT32 mTableCount;
 STATIC UINT32 mBinTableCount;
-STATIC BOOLEANmConsistencyCheck;
-STATIC BOOLEANmColourHighlighting;
-STATIC BOOLEANmMandatoryTableValidate;
-STATIC UINTN  mMandatoryTableSpec;
 
 /**
   An array of acpiview command line parameters.
@@ -53,142 +45,6 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
   {NULL, TypeMax}
 };
 
-/**
-  This function returns the colour highlighting status.
-
-  @retval TRUE if colour highlighting is enabled.
-**/
-BOOLEAN
-GetColourHighlighting (
-  VOID
-  )
-{
-  return mColourHighlighting;
-}
-
-/**
-  This function sets the colour highlighting status.
-
-  @param  Highlight   The Highlight status.
-
-**/
-VOID
-SetColourHighlighting (
-  BOOLEAN Highlight
-  )
-{
-  mColourHighlighting = Highlight;
-}
-
-/**
-  This function returns the consistency checking status.
-
-  @retval TRUE if consistency checking is enabled.
-**/
-BOOLEAN
-GetConsistencyChecking (
-  VOID
-  )
-{
-  return mConsistencyCheck;
-}
-
-/**
-  This function sets the consistency checking status.
-
-  @param  ConsistencyChecking   The consistency checking status.
-
-**/
-VOID
-SetConsistencyChecking (
-  BOOLEAN ConsistencyChecking
-  )
-{
-  mConsistencyCheck = ConsistencyChecking;
-}
-
-/**
-  This function returns the ACPI table requirements validation flag.
-
-  @retval TRUE if check for mandatory table presence should be performed.
-**/
-BOOLEAN
-GetMandatoryTableValidate (
-  VOID
-  )
-{
-  return mMandatoryTableValidate;
-}
-
-/**
-  This function sets the ACPI table requirements validation flag.
-
-  @param  ValidateEnable/Disable ACPI table requirements validation.
-**/
-VOID
-SetMandatoryTableValidate (
-  BOOLEAN Validate
-  )
-{
-  mMandatoryTableValidate = Validate;
-}
-
-/**
-  This function returns the identifier of specification to validate ACPI table
-  requirements against.