[PATCH v1 0/5] Clean up acpiview input parameters

[PATCH v1 0/5] Clean up acpiview input parameters

[Krzysztof Koch]

The following patches modify the user interface to the Acpiview UEFI
shell tool.

These changes are aimed to simplify interaction with the tool as well
as to remove redundant/non-functional command line parameters.

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

Krzysztof Koch (5):
  ShellPkg: acpiview: Remove '/?' from valid command line flags
  ShellPkg: acpiview: Fix '\n\n' printing in Table Checksum reporting
  ShellPkg: acpiview: Allow disabling consistency checks (-q flag)
  ShellPkg: acpiview: Remove '-v' flag from allowed command line args
  ShellPkg: acpiview: Make '-h' option not require a parameter

 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c                    | 14 ++--
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c               |  6 +-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c                      | 71 ++++++++++----------
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.h                      | 23 ++++++-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni |  9 +--
 5 files changed, 74 insertions(+), 49 deletions(-)

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

[PATCH v1 1/5] ShellPkg: acpiview: Remove '/?' from valid command line flags

[Krzysztof Koch]

Acpiview currently allows displaying help info by specifying both
-? and /? flags. This patch removes /? from valid command line
flags that can be passed to the acpiview tool, as this
flag is not used for such purpose in other UEFI shell commands.

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

Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/commit/55cb307eac7cfc4cc46df0af8c6edf59863518dc

Notes:
    v1:
    - Remove '/?' from allowed acpiview input flags [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
index 25d61158320f5d68023f7187a936d50f2bcd20e7..9bbb00cacafa34760c9d45cb6a9fe15bcd4c100e 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
@@ -1,6 +1,6 @@
 /** @file
 
-  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
 **/
 
@@ -33,7 +33,6 @@ STATIC BOOLEAN            mColourHighlighting;
   An array of acpiview command line parameters.
 **/
 STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
-  {L"/?", TypeFlag},
   {L"-c", TypeFlag},
   {L"-d", TypeFlag},
   {L"-h", TypeValue},
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v1 1/5] ShellPkg: acpiview: Remove '/?' from valid command line flags

[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/5] ShellPkg: acpiview: Fix '\n\n' printing in Table Checksum reporting

[Krzysztof Koch]

Move printing double newline character ('\n\n') from the beginning of
ACPI table checksum validation message to the end of the raw binary
data dump.

This way acpiview table dump looks similar regardless of whether Table
Checksum is validated or not.

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

Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/commit/db22c06817971220875e98ec891ed73359f823de

Notes:
    v1:
    - Unify acpiview output with/without checksum reporting [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index b5965507b498b9fa9bc4d3124b2285f509004c4f..f9dbbd35449e74fdf81f09e627920b10583a9816 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -1,7 +1,7 @@
 /** @file
   ACPI 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
 **/
 
@@ -139,7 +139,7 @@ VerifyChecksum (
                          ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4))
                        );
       }
-      Print (L"\n\nTable Checksum : OK\n\n");
+      Print (L"Table Checksum : OK\n\n");
     } else {
       IncrementErrorCount ();
       if (GetColourHighlighting ()) {
@@ -149,7 +149,7 @@ VerifyChecksum (
                          ((OriginalAttribute&(BIT4|BIT5|BIT6))>>4))
                        );
       }
-      Print (L"\n\nTable Checksum : FAILED (0x%X)\n\n", Checksum);
+      Print (L"Table Checksum : FAILED (0x%X)\n\n", Checksum);
     }
     if (GetColourHighlighting ()) {
       gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
@@ -219,7 +219,7 @@ DumpRaw (
 
   // Print ASCII data for the final line.
   AsciiBuffer[AsciiBufferIndex] = '\0';
-  Print (L"  %a", AsciiBuffer);
+  Print (L"  %a\n\n", AsciiBuffer);
 }
 
 /**
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v1 2/5] ShellPkg: acpiview: Fix '\n\n' printing in Table Checksum reporting

[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/5] ShellPkg: acpiview: Allow disabling consistency checks (-q flag)

[Krzysztof Koch]

The current documentation for the acpiview UEFI shell tool states
that the '-c' flag enables consistency checks on ACPI table data.
However, these checks are enabled anyway by default.

This patch keeps ACPI table validation as a default option, but it
makes it possible to turn ACPI table validation off by setting the
newly-introduced '-q' flag. Consequently, the '-c' flag is removed.

The remaining code changes in this patch make a number of consistency
checks optional (but enabled by default):
1. ACPI table field offset mismatch.
2. ACPI table field validation functions provided in the ACPI_PARSER
   arrays.
3. Table checksum computation.

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

Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/commit/fa016236b0955d19db70bf83fe4e5048e3b858f4

Notes:
    v1:
    - make table consistency checks optional where applicable [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c                    |  6 ++--
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c               |  6 ++--
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c                      | 35 ++++++++++++++++++--
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.h                      | 23 ++++++++++++-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni |  4 +--
 5 files changed, 65 insertions(+), 9 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
index f9dbbd35449e74fdf81f09e627920b10583a9816..8b3153516d2b7d9b920ab2de0344c17798ac572c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
@@ -506,7 +506,8 @@ ParseAcpi (
       break;
     }
 
-    if (Offset != Parser[Index].Offset) {
+    if (GetConsistencyChecking () &&
+        (Offset != Parser[Index].Offset)) {
       IncrementErrorCount ();
       Print (
         L"\nERROR: %a: Offset Mismatch for %s\n"
@@ -549,7 +550,8 @@ ParseAcpi (
 
         // Validating only makes sense if we are tracing
         // the parsed table entries, to report by table name.
-        if (Parser[Index].FieldValidator != NULL) {
+        if (GetConsistencyChecking () &&
+            (Parser[Index].FieldValidator != NULL)) {
           Parser[Index].FieldValidator (Ptr, Parser[Index].Context);
         }
       }
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
index 504c81e3f1405095afd0d77fee64a02fd06a64d2..d5500bcb2b4a55c7a69f45444aa49d36d2c1694f 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
@@ -1,7 +1,7 @@
 /** @file
   ACPI 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
 **/
 
@@ -193,7 +193,9 @@ ProcessAcpiTable (
 
   if (Trace) {
     DumpRaw (Ptr, *AcpiTableLength);
-    VerifyChecksum (TRUE, Ptr, *AcpiTableLength);
+    if (GetConsistencyChecking ()) {
+      VerifyChecksum (TRUE, Ptr, *AcpiTableLength);
+    }
   }
 
   Status = GetParser (*AcpiTableSignature, &ParserProc);
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
index 9bbb00cacafa34760c9d45cb6a9fe15bcd4c100e..e99537acf36616783103e1e9d154278f8dedb78e 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
@@ -33,7 +33,7 @@ STATIC BOOLEAN            mColourHighlighting;
   An array of acpiview command line parameters.
 **/
 STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
-  {L"-c", TypeFlag},
+  {L"-q", TypeFlag},
   {L"-d", TypeFlag},
   {L"-h", TypeValue},
   {L"-l", TypeFlag},
@@ -69,6 +69,33 @@ SetColourHighlighting (
   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 report options.
 
@@ -380,7 +407,8 @@ AcpiView (
          (ReportDumpBinFile == ReportOption)) &&
         (!mSelectedAcpiTableFound)) {
       Print (L"\nRequested ACPI Table not found.\n");
-    } else if (ReportDumpBinFile != ReportOption) {
+    } else if (GetConsistencyChecking () &&
+               (ReportDumpBinFile != ReportOption)) {
       OriginalAttribute = gST->ConOut->Mode->Attribute;
 
       Print (L"\nTable Statistics:\n");
@@ -554,6 +582,9 @@ ShellCommandRunAcpiView (
         }
       }
 
+      // Surpress consistency checking if requested
+      SetConsistencyChecking (!ShellCommandLineGetFlag (Package, L"-q"));
+
       if (ShellCommandLineGetFlag (Package, L"-l")) {
         mReportType = ReportTableList;
       } else {
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.h
index 1b26d8fb1771eb4adb3f381e04234932480b6110..b5cb274ecbf77f7ccb81d78f852caa0f50854312 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.h
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.h
@@ -1,7 +1,7 @@
 /** @file
   Header file for AcpiView
 
-  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
 **/
 
@@ -91,6 +91,27 @@ SetColourHighlighting (
   BOOLEAN Highlight
   );
 
+/**
+  This function returns the consistency checking status.
+
+  @retval TRUE if consistency checking is enabled.
+**/
+BOOLEAN
+GetConsistencyChecking (
+  VOID
+  );
+
+/**
+  This function sets the consistency checking status.
+
+  @param  ConsistencyChecking   The consistency checking status.
+
+**/
+VOID
+SetConsistencyChecking (
+  BOOLEAN ConsistencyChecking
+  );
+
 /**
   This function processes the table reporting options for the ACPI table.
 
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni
index 4c63143cbef85262793c287dfeb8ebb394394416..f0610870505d5c5d89e6516122fc9e782479df0c 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni
@@ -30,7 +30,7 @@
 "Display ACPI Table information.\r\n"
 ".SH SYNOPSIS\r\n"
 " \r\n"
-"ACPIVIEW [[-?] | [[-l] | [-s AcpiTable [-d]]] [-c] [-v] [-h Highlight]]\r\n"
+"ACPIVIEW [[-?] | [[-l] | [-s AcpiTable [-d]]] [-q] [-v] [-h Highlight]]\r\n"
 " \r\n"
 ".SH OPTIONS\r\n"
 " \r\n"
@@ -39,7 +39,7 @@
 "       invocation option.\r\n"
 "         AcpiTable    : The required ACPI Table type.\r\n"
 "  -d - Generate a binary file dump of the specified AcpiTable.\r\n"
-"  -c - Consistency checking (enabled by default).\r\n"
+"  -q - Quiet. Suppress errors and warnings. Disables consistency checks.\r\n"
 "  -v - Display verbose data (enabled by default).\r\n"
 "  -h - Enable/Disable Colour Highlighting.\r\n"
 "         Highlight    : TRUE/ON enables highlighting;\r\n"
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v1 3/5] ShellPkg: acpiview: Allow disabling consistency checks (-q flag)

[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/5] ShellPkg: acpiview: Remove '-v' flag from allowed command line args

[Krzysztof Koch]

According to the acpiview documentation, the -v flag enables verbose
output and it is set on default.

Moreover, the acpiview UEFI shell tool dumps the same output with
and without this flag set.

Therefore this patch removes the superfluous -v flag from allowed
command line parameters.

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

Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/commit/6400c6323f78ac89e790e57c07e9cca2e979ef05

Notes:
    v1:
    - Remove the unused '-v' flag [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c                      | 3 ---
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni | 3 +--
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
index e99537acf36616783103e1e9d154278f8dedb78e..911b2da3dd9f19010d0f4993bcf017749cafb749 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
@@ -25,7 +25,6 @@ STATIC BOOLEAN            mSelectedAcpiTableFound;
 STATIC EREPORT_OPTION     mReportType;
 STATIC UINT32             mTableCount;
 STATIC UINT32             mBinTableCount;
-STATIC BOOLEAN            mVerbose;
 STATIC BOOLEAN            mConsistencyCheck;
 STATIC BOOLEAN            mColourHighlighting;
 
@@ -38,7 +37,6 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
   {L"-h", TypeValue},
   {L"-l", TypeFlag},
   {L"-s", TypeValue},
-  {L"-v", TypeFlag},
   {NULL, TypeMax}
 };
 
@@ -472,7 +470,6 @@ ShellCommandRunAcpiView (
   mSelectedAcpiTable = 0;
   mSelectedAcpiTableName = NULL;
   mSelectedAcpiTableFound = FALSE;
-  mVerbose = TRUE;
   mConsistencyCheck = TRUE;
 
   ShellStatus = SHELL_SUCCESS;
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni
index f0610870505d5c5d89e6516122fc9e782479df0c..0d10b5debe470cb096cf414d0a0144fd3bde03ee 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni
@@ -30,7 +30,7 @@
 "Display ACPI Table information.\r\n"
 ".SH SYNOPSIS\r\n"
 " \r\n"
-"ACPIVIEW [[-?] | [[-l] | [-s AcpiTable [-d]]] [-q] [-v] [-h Highlight]]\r\n"
+"ACPIVIEW [[-?] | [[-l] | [-s AcpiTable [-d]]] [-q] [-h Highlight]]\r\n"
 " \r\n"
 ".SH OPTIONS\r\n"
 " \r\n"
@@ -40,7 +40,6 @@
 "         AcpiTable    : The required ACPI Table type.\r\n"
 "  -d - Generate a binary file dump of the specified AcpiTable.\r\n"
 "  -q - Quiet. Suppress errors and warnings. Disables consistency checks.\r\n"
-"  -v - Display verbose data (enabled by default).\r\n"
 "  -h - Enable/Disable Colour Highlighting.\r\n"
 "         Highlight    : TRUE/ON enables highlighting;\r\n"
 "                        FALSE/OFF (default) disables highlighting.\r\n"
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v1 4/5] ShellPkg: acpiview: Remove '-v' flag from allowed command line args

[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/5] ShellPkg: acpiview: Make '-h' option not require a parameter

[Krzysztof Koch]

Make the -h command line option a binary flag. Now, colour
highlighting is enabled whenever this flag is set (stateless),
instead of being dependent on previous acpiview command
invocations.

By removing the parameter required with the -h flag the command
line parsing logic becomes simpler.

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

Changes can be seen at: https://github.com/KrzysztofKoch1/edk2/commit/c3e78ab9d47f51e4c6a786a8813d1fa01685f0c9

Notes:
    v1:
    - turn the '-h' acpiview input option into a flag [Krzysztof]

 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c                      | 30 ++------------------
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni |  6 ++--
 2 files changed, 5 insertions(+), 31 deletions(-)

diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
index 911b2da3dd9f19010d0f4993bcf017749cafb749..9feb2df2078f5aff41e1d39081fbb8cbc0bc32dd 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
@@ -34,7 +34,7 @@ STATIC BOOLEAN            mColourHighlighting;
 STATIC CONST SHELL_PARAM_ITEM ParamList[] = {
   {L"-q", TypeFlag},
   {L"-d", TypeFlag},
-  {L"-h", TypeValue},
+  {L"-h", TypeFlag},
   {L"-l", TypeFlag},
   {L"-s", TypeValue},
   {NULL, TypeMax}
@@ -459,8 +459,6 @@ ShellCommandRunAcpiView (
   SHELL_STATUS       ShellStatus;
   LIST_ENTRY*        Package;
   CHAR16*            ProblemParam;
-  CONST CHAR16*      Temp;
-  CHAR8              ColourOption[8];
   SHELL_FILE_HANDLE  TmpDumpFileHandle;
 
   // Set Defaults
@@ -540,18 +538,6 @@ ShellCommandRunAcpiView (
         L"acpiview"
         );
       ShellStatus = SHELL_INVALID_PARAMETER;
-    } else if (ShellCommandLineGetFlag (Package, L"-h") &&
-               ShellCommandLineGetValue (Package, L"-h") == NULL) {
-        ShellPrintHiiEx (
-          -1,
-          -1,
-          NULL,
-          STRING_TOKEN (STR_GEN_NO_VALUE),
-          gShellAcpiViewHiiHandle,
-          L"acpiview",
-          L"-h"
-          );
-        ShellStatus = SHELL_INVALID_PARAMETER;
     } else if (ShellCommandLineGetFlag (Package, L"-d") &&
                !ShellCommandLineGetFlag (Package, L"-s")) {
         ShellPrintHiiEx (
@@ -566,18 +552,8 @@ ShellCommandRunAcpiView (
           );
         ShellStatus = SHELL_INVALID_PARAMETER;
     } else {
-      // Check if the colour option is set
-      Temp = ShellCommandLineGetValue (Package, L"-h");
-      if (Temp != NULL) {
-        UnicodeStrToAsciiStrS (Temp, ColourOption, sizeof (ColourOption));
-        if ((AsciiStriCmp (ColourOption, "ON") == 0) ||
-            (AsciiStriCmp (ColourOption, "TRUE") == 0)) {
-          SetColourHighlighting (TRUE);
-        } else if ((AsciiStriCmp (ColourOption, "OFF") == 0) ||
-                   (AsciiStriCmp (ColourOption, "FALSE") == 0)) {
-          SetColourHighlighting (FALSE);
-        }
-      }
+      // Turn on colour highlighting if requested
+      SetColourHighlighting (ShellCommandLineGetFlag (Package, L"-h"));
 
       // Surpress consistency checking if requested
       SetConsistencyChecking (!ShellCommandLineGetFlag (Package, L"-q"));
diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni
index 0d10b5debe470cb096cf414d0a0144fd3bde03ee..1f07b7ae20d474be67a433ff3774a508a5289318 100644
--- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni
+++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni
@@ -30,7 +30,7 @@
 "Display ACPI Table information.\r\n"
 ".SH SYNOPSIS\r\n"
 " \r\n"
-"ACPIVIEW [[-?] | [[-l] | [-s AcpiTable [-d]]] [-q] [-h Highlight]]\r\n"
+"ACPIVIEW [[-?] | [[-l] | [-s AcpiTable [-d]]] [-q] [-h]]\r\n"
 " \r\n"
 ".SH OPTIONS\r\n"
 " \r\n"
@@ -40,9 +40,7 @@
 "         AcpiTable    : The required ACPI Table type.\r\n"
 "  -d - Generate a binary file dump of the specified AcpiTable.\r\n"
 "  -q - Quiet. Suppress errors and warnings. Disables consistency checks.\r\n"
-"  -h - Enable/Disable Colour Highlighting.\r\n"
-"         Highlight    : TRUE/ON enables highlighting;\r\n"
-"                        FALSE/OFF (default) disables highlighting.\r\n"
+"  -h - Enable colour highlighting.\r\n"
 "  -? - Show help.\r\n"
 " \r\n"
 ".SH DESCRIPTION\r\n"
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'

Re: [edk2-devel] [PATCH v1 5/5] ShellPkg: acpiview: Make '-h' option not require a parameter

[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: [edk2-devel] [PATCH v1 0/5] Clean up acpiview input parameters

[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: [edk2-devel] [PATCH v1 0/5] Clean up acpiview input parameters

[Gao, Zhichao]

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

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Krzysztof Koch
> Sent: Friday, June 28, 2019 4:57 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: [edk2-devel] [PATCH v1 0/5] Clean up acpiview input parameters
> 
> The following patches modify the user interface to the Acpiview UEFI shell
> tool.
> 
> These changes are aimed to simplify interaction with the tool as well as to
> remove redundant/non-functional command line parameters.
> 
> Changes can be seen at:
> https://github.com/KrzysztofKoch1/edk2/tree/612_acpiview_flags_fix_v1
> 
> Krzysztof Koch (5):
>   ShellPkg: acpiview: Remove '/?' from valid command line flags
>   ShellPkg: acpiview: Fix '\n\n' printing in Table Checksum reporting
>   ShellPkg: acpiview: Allow disabling consistency checks (-q flag)
>   ShellPkg: acpiview: Remove '-v' flag from allowed command line args
>   ShellPkg: acpiview: Make '-h' option not require a parameter
> 
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c                    | 14
> ++--
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c               |
> 6 +-
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c                      | 71
> ++++++++++----------
>  ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.h                      | 23
> ++++++-
> 
> ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewComman
> dLib.uni |  9 +--
>  5 files changed, 74 insertions(+), 49 deletions(-)
> 
> --
> 'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
> 
> 
> 
> 

Re: [PATCH v1 0/5] Clean up acpiview input parameters

[Sami Mujawar]

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

-----Original Message-----
From: Krzysztof Koch <krzysztof.koch@arm.com> 
Sent: 28 June 2019 09:57 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/5] Clean up acpiview input parameters

The following patches modify the user interface to the Acpiview UEFI shell tool.

These changes are aimed to simplify interaction with the tool as well as to remove redundant/non-functional command line parameters.

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

Krzysztof Koch (5):
  ShellPkg: acpiview: Remove '/?' from valid command line flags
  ShellPkg: acpiview: Fix '\n\n' printing in Table Checksum reporting
  ShellPkg: acpiview: Allow disabling consistency checks (-q flag)
  ShellPkg: acpiview: Remove '-v' flag from allowed command line args
  ShellPkg: acpiview: Make '-h' option not require a parameter

 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c                    | 14 ++--
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c               |  6 +-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c                      | 71 ++++++++++----------
 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.h                      | 23 ++++++-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.uni |  9 +--
 5 files changed, 74 insertions(+), 49 deletions(-)

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