Remove accessor method bloat by creating a configuration struct that is linked using an extern symbol in the config header file. Directly reference the config struct for all read and write accesses in the entire module.
Rationalise the remaining methods in the config header and source. Cc: Ray Ni <ray...@intel.com> Cc: Zhichao Gao <zhichao....@intel.com> Signed-off-by: Tomas Pilar <tomas.pi...@arm.com> --- .../UefiShellAcpiViewCommandLib/AcpiParser.c | 18 +- .../AcpiTableParser.c | 4 +- .../UefiShellAcpiViewCommandLib/AcpiView.c | 67 +++---- .../AcpiViewConfig.c | 180 ++---------------- .../AcpiViewConfig.h | 138 ++------------ .../UefiShellAcpiViewCommandLib.c | 18 +- 6 files changed, 70 insertions(+), 355 deletions(-) diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c index 02f6d771c7e1..3a029b01cc20 100644 --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c @@ -137,7 +137,7 @@ VerifyChecksum ( if (Log) { OriginalAttribute = gST->ConOut->Mode->Attribute; if (Checksum == 0) { - if (GetColourHighlighting ()) { + if (mConfig.ColourHighlighting) { gST->ConOut->SetAttribute ( gST->ConOut, EFI_TEXT_ATTR (EFI_GREEN, @@ -147,7 +147,7 @@ VerifyChecksum ( Print (L"Table Checksum : OK\n\n"); } else { IncrementErrorCount (); - if (GetColourHighlighting ()) { + if (mConfig.ColourHighlighting) { gST->ConOut->SetAttribute ( gST->ConOut, EFI_TEXT_ATTR (EFI_RED, @@ -156,7 +156,7 @@ VerifyChecksum ( } Print (L"Table Checksum : FAILED (0x%X)\n\n", Checksum); } - if (GetColourHighlighting ()) { + if (mConfig.ColourHighlighting) { gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute); } } @@ -507,7 +507,6 @@ ParseAcpi ( { UINT32 Index; UINT32 Offset; - BOOLEAN HighLight; UINTN OriginalAttribute; // @@ -520,9 +519,8 @@ ParseAcpi ( gIndent += Indent; if (Trace && (AsciiName != NULL)){ - HighLight = GetColourHighlighting (); - if (HighLight) { + if (mConfig.ColourHighlighting) { OriginalAttribute = gST->ConOut->Mode->Attribute; gST->ConOut->SetAttribute ( gST->ConOut, @@ -537,7 +535,7 @@ ParseAcpi ( (OUTPUT_FIELD_COLUMN_WIDTH - gIndent), AsciiName ); - if (HighLight) { + if (mConfig.ColourHighlighting) { gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute); } } @@ -555,8 +553,7 @@ ParseAcpi ( continue; } - if (GetConsistencyChecking () && - (Offset != Parser[Index].Offset)) { + if (mConfig.ConsistencyCheck && (Offset != Parser[Index].Offset)) { IncrementErrorCount (); Print ( L"\nERROR: %a: Offset Mismatch for %s\n" @@ -599,8 +596,7 @@ ParseAcpi ( // Validating only makes sense if we are tracing // the parsed table entries, to report by table name. - if (GetConsistencyChecking () && - (Parser[Index].FieldValidator != NULL)) { + if (mConfig.ConsistencyCheck && (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 4b618f131eac..526cb8cb7cad 100644 --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c @@ -222,13 +222,13 @@ ProcessAcpiTable ( return; } - if (GetConsistencyChecking ()) { + if (mConfig.ConsistencyCheck) { VerifyChecksum (TRUE, Ptr, *AcpiTableLength); } } #if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64) - if (GetMandatoryTableValidate ()) { + if (mConfig.MandatoryTableValidate) { ArmSbbrIncrementTableCount (*AcpiTableSignature); } #endif diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c index 9a5b013fb234..d0a0edc45f3b 100644 --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c @@ -48,15 +48,12 @@ DumpAcpiTableToFile ( { CHAR16 FileNameBuffer[MAX_FILE_NAME_LEN]; UINTN TransferBytes; - SELECTED_ACPI_TABLE *SelectedTable; - - GetSelectedAcpiTable (&SelectedTable); UnicodeSPrint ( FileNameBuffer, sizeof (FileNameBuffer), L".\\%s%04d.bin", - SelectedTable->Name, + mSelectedAcpiTable.Name, mBinTableCount++ ); @@ -82,11 +79,9 @@ ProcessTableReportOptions ( IN CONST UINT32 Length ) { - UINTN OriginalAttribute; - UINT8 *SignaturePtr; - BOOLEAN Log; - BOOLEAN HighLight; - SELECTED_ACPI_TABLE *SelectedTable; + UINTN OriginalAttribute; + UINT8 *SignaturePtr; + BOOLEAN Log; // // set local variables to suppress incorrect compiler/analyzer warnings @@ -94,22 +89,20 @@ ProcessTableReportOptions ( OriginalAttribute = 0; SignaturePtr = (UINT8*)(UINTN)&Signature; Log = FALSE; - HighLight = GetColourHighlighting (); - GetSelectedAcpiTable (&SelectedTable); - switch (GetReportOption ()) { + switch (mConfig.ReportType) { case ReportAll: Log = TRUE; break; case ReportSelected: - if (Signature == SelectedTable->Type) { + if (Signature == mSelectedAcpiTable.Type) { Log = TRUE; - SelectedTable->Found = TRUE; + mSelectedAcpiTable.Found = TRUE; } break; case ReportTableList: if (mTableCount == 0) { - if (HighLight) { + if (mConfig.ColourHighlighting) { OriginalAttribute = gST->ConOut->Mode->Attribute; gST->ConOut->SetAttribute ( gST->ConOut, @@ -118,7 +111,7 @@ ProcessTableReportOptions ( ); } Print (L"\nInstalled Table(s):\n"); - if (HighLight) { + if (mConfig.ColourHighlighting) { gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute); } } @@ -132,8 +125,8 @@ ProcessTableReportOptions ( ); break; case ReportDumpBinFile: - if (Signature == SelectedTable->Type) { - SelectedTable->Found = TRUE; + if (Signature == mSelectedAcpiTable.Type) { + mSelectedAcpiTable.Found = TRUE; DumpAcpiTableToFile (TablePtr, Length); } break; @@ -144,7 +137,7 @@ ProcessTableReportOptions ( } // switch if (Log) { - if (HighLight) { + if (mConfig.ColourHighlighting) { OriginalAttribute = gST->ConOut->Mode->Attribute; gST->ConOut->SetAttribute ( gST->ConOut, @@ -159,7 +152,7 @@ ProcessTableReportOptions ( SignaturePtr[2], SignaturePtr[3] ); - if (HighLight) { + if (mConfig.ColourHighlighting) { gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute); } } @@ -191,18 +184,12 @@ AcpiView ( BOOLEAN FoundAcpiTable; UINTN OriginalAttribute; UINTN PrintAttribute; - EREPORT_OPTION ReportOption; UINT8* RsdpPtr; UINT32 RsdpLength; UINT8 RsdpRevision; PARSE_ACPI_TABLE_PROC RsdpParserProc; BOOLEAN Trace; - SELECTED_ACPI_TABLE *SelectedTable; - // - // set local variables to suppress incorrect compiler/analyzer warnings - // - EfiConfigurationTable = NULL; OriginalAttribute = 0; // Reset Table counts @@ -213,9 +200,6 @@ AcpiView ( ResetErrorCount (); ResetWarningCount (); - // Retrieve the user selection of ACPI table to process - GetSelectedAcpiTable (&SelectedTable); - // Search the table for an entry that matches the ACPI Table Guid FoundAcpiTable = FALSE; for (Index = 0; Index < SystemTable->NumberOfTableEntries; Index++) { @@ -241,7 +225,7 @@ AcpiView ( } #if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64) - if (GetMandatoryTableValidate ()) { + if (mConfig.MandatoryTableValidate) { ArmSbbrResetTableCounts (); } #endif @@ -275,24 +259,23 @@ AcpiView ( } #if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64) - if (GetMandatoryTableValidate ()) { - ArmSbbrReqsValidate ((ARM_SBBR_VERSION)GetMandatoryTableSpec ()); + if (mConfig.MandatoryTableValidate) { + ArmSbbrReqsValidate ((ARM_SBBR_VERSION) mConfig.MandatoryTableSpec); } #endif - ReportOption = GetReportOption (); - if (ReportTableList != ReportOption) { - if (((ReportSelected == ReportOption) || - (ReportDumpBinFile == ReportOption)) && - (!SelectedTable->Found)) { + if (ReportTableList != mConfig.ReportType) { + if (((ReportSelected == mConfig.ReportType) || + (ReportDumpBinFile == mConfig.ReportType)) && + (!mSelectedAcpiTable.Found)) { Print (L"\nRequested ACPI Table not found.\n"); - } else if (GetConsistencyChecking () && - (ReportDumpBinFile != ReportOption)) { + } else if (mConfig.ConsistencyCheck && + (ReportDumpBinFile != mConfig.ReportType)) { OriginalAttribute = gST->ConOut->Mode->Attribute; Print (L"\nTable Statistics:\n"); - if (GetColourHighlighting ()) { + if (mConfig.ColourHighlighting) { PrintAttribute = (GetErrorCount () > 0) ? EFI_TEXT_ATTR ( EFI_RED, @@ -303,7 +286,7 @@ AcpiView ( } Print (L"\t%d Error(s)\n", GetErrorCount ()); - if (GetColourHighlighting ()) { + if (mConfig.ColourHighlighting) { PrintAttribute = (GetWarningCount () > 0) ? EFI_TEXT_ATTR ( EFI_RED, @@ -315,7 +298,7 @@ AcpiView ( } Print (L"\t%d Warning(s)\n", GetWarningCount ()); - if (GetColourHighlighting ()) { + if (mConfig.ColourHighlighting) { gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute); } } diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewConfig.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewConfig.c index 759a915928c7..a0a392085355 100644 --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewConfig.c +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewConfig.c @@ -10,12 +10,8 @@ #include "AcpiViewConfig.h" -// Report variables -STATIC BOOLEAN mConsistencyCheck; -STATIC BOOLEAN mColourHighlighting; -STATIC EREPORT_OPTION mReportType; -STATIC BOOLEAN mMandatoryTableValidate; -STATIC UINTN mMandatoryTableSpec; +// Main configuration structure +ACPI_VIEW_CONFIG mConfig; // User selection of which ACPI table should be checked SELECTED_ACPI_TABLE mSelectedAcpiTable; @@ -25,17 +21,21 @@ SELECTED_ACPI_TABLE mSelectedAcpiTable; **/ VOID EFIAPI -AcpiConfigSetDefaults ( +AcpiViewConfigSetDefaults ( VOID ) { - mReportType = ReportAll; + // Generic defaults + mConfig.ReportType = ReportAll; + mConfig.ConsistencyCheck = TRUE; + mConfig.MandatoryTableValidate = FALSE; + mConfig.MandatoryTableSpec = 0; + mConfig.ColourHighlighting = FALSE; + + // Unselect acpi table mSelectedAcpiTable.Type = 0; mSelectedAcpiTable.Name = NULL; mSelectedAcpiTable.Found = FALSE; - mConsistencyCheck = TRUE; - mMandatoryTableValidate = FALSE; - mMandatoryTableSpec = 0; } /** @@ -79,7 +79,7 @@ ConvertStrToAcpiSignature ( **/ VOID EFIAPI -SelectAcpiTable ( +AcpiViewConfigSelectTable ( IN CONST CHAR16 *TableName ) { @@ -88,159 +88,3 @@ SelectAcpiTable ( mSelectedAcpiTable.Name = TableName; mSelectedAcpiTable.Type = ConvertStrToAcpiSignature (mSelectedAcpiTable.Name); } - -/** - This function returns the selected ACPI table. - - @param [out] SelectedAcpiTable Pointer that will contain the returned struct. -**/ -VOID -EFIAPI -GetSelectedAcpiTable ( - OUT SELECTED_ACPI_TABLE **SelectedAcpiTable - ) -{ - *SelectedAcpiTable = &mSelectedAcpiTable; -} - -/** - This function returns the colour highlighting status. - - @retval TRUE Colour highlighting is enabled. -**/ -BOOLEAN -EFIAPI -GetColourHighlighting ( - VOID - ) -{ - return mColourHighlighting; -} - -/** - This function sets the colour highlighting status. - - @param [in] Highlight The highlight status. -**/ -VOID -EFIAPI -SetColourHighlighting ( - BOOLEAN Highlight - ) -{ - mColourHighlighting = Highlight; -} - -/** - This function returns the consistency checking status. - - @retval TRUE Consistency checking is enabled. -**/ -BOOLEAN -EFIAPI -GetConsistencyChecking ( - VOID - ) -{ - return mConsistencyCheck; -} - -/** - This function sets the consistency checking status. - - @param [in] ConsistencyChecking The consistency checking status. -**/ -VOID -EFIAPI -SetConsistencyChecking ( - BOOLEAN ConsistencyChecking - ) -{ - mConsistencyCheck = ConsistencyChecking; -} - -/** - This function returns the report options. - - @return The current report option. -**/ -EREPORT_OPTION -EFIAPI -GetReportOption ( - VOID - ) -{ - return mReportType; -} - -/** - This function sets the report options. - - @param [in] ReportType The report option to set. -**/ -VOID -EFIAPI -SetReportOption ( - EREPORT_OPTION ReportType - ) -{ - mReportType = ReportType; -} - -/** - This function returns the ACPI table requirements validation flag. - - @retval TRUE Check for mandatory table presence should be performed. -**/ -BOOLEAN -EFIAPI -GetMandatoryTableValidate ( - VOID - ) -{ - return mMandatoryTableValidate; -} - -/** - This function sets the ACPI table requirements validation flag. - - @param [in] Validate Enable/Disable ACPI table requirements validation. -**/ -VOID -EFIAPI -SetMandatoryTableValidate ( - BOOLEAN Validate - ) -{ - mMandatoryTableValidate = Validate; -} - -/** - This function returns the identifier of specification to validate ACPI table - requirements against. - - @return ID of specification listing mandatory tables. -**/ -UINTN -EFIAPI -GetMandatoryTableSpec ( - VOID - ) -{ - return mMandatoryTableSpec; -} - -/** - This function sets the identifier of specification to validate ACPI table - requirements against. - - @param [in] Spec ID of specification listing mandatory tables. -**/ -VOID -EFIAPI -SetMandatoryTableSpec ( - UINTN Spec - ) -{ - mMandatoryTableSpec = Spec; -} diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewConfig.h b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewConfig.h index 2db4a65415d8..1883145d04a4 100644 --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewConfig.h +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewConfig.h @@ -8,96 +8,6 @@ #ifndef ACPI_VIEW_CONFIG_H_ #define ACPI_VIEW_CONFIG_H_ -/** - This function returns the colour highlighting status. - - @retval TRUE Colour highlighting is enabled. -**/ -BOOLEAN -EFIAPI -GetColourHighlighting ( - VOID - ); - -/** - This function sets the colour highlighting status. - - @param [in] Highlight The highlight status. -**/ -VOID -EFIAPI -SetColourHighlighting ( - BOOLEAN Highlight - ); - -/** - This function returns the consistency checking status. - - @retval TRUE Consistency checking is enabled. -**/ -BOOLEAN -EFIAPI -GetConsistencyChecking ( - VOID - ); - -/** - This function sets the consistency checking status. - - @param [in] ConsistencyChecking The consistency checking status. -**/ -VOID -EFIAPI -SetConsistencyChecking ( - BOOLEAN ConsistencyChecking - ); - -/** - This function returns the ACPI table requirements validation flag. - - @retval TRUE Check for mandatory table presence should be performed. -**/ -BOOLEAN -EFIAPI -GetMandatoryTableValidate ( - VOID - ); - -/** - This function sets the ACPI table requirements validation flag. - - @param [in] Validate Enable/Disable ACPI table requirements validation. -**/ -VOID -EFIAPI -SetMandatoryTableValidate ( - BOOLEAN Validate - ); - -/** - This function returns the identifier of specification to validate ACPI table - requirements against. - - @return ID of specification listing mandatory tables. -**/ -UINTN -EFIAPI -GetMandatoryTableSpec ( - VOID - ); - -/** - This function sets the identifier of specification to validate ACPI table - requirements against. - - @param [in] Spec ID of specification listing mandatory tables. -**/ -VOID -EFIAPI -SetMandatoryTableSpec ( - UINTN Spec - ); - /** The EREPORT_OPTION enum describes ACPI table Reporting options. **/ @@ -109,28 +19,6 @@ typedef enum { ReportMax, } EREPORT_OPTION; -/** - This function returns the report options. - - @return The current report option. -**/ -EREPORT_OPTION -EFIAPI -GetReportOption ( - VOID - ); - -/** - This function sets the report options. - - @param [in] ReportType The report option to set. -**/ -VOID -EFIAPI -SetReportOption ( - EREPORT_OPTION ReportType - ); - /** A structure holding the user selection detailing which ACPI table is to be examined by the AcpiView code. @@ -142,15 +30,15 @@ typedef struct { } SELECTED_ACPI_TABLE; /** - This function returns the selected ACPI table. - - @param [out] SelectedAcpiTable Pointer that will contain the returned struct. + Configuration struct for the acpiview command. **/ -VOID -EFIAPI -GetSelectedAcpiTable ( - OUT SELECTED_ACPI_TABLE** SelectedAcpiTable - ); +typedef struct { + BOOLEAN ConsistencyCheck; ///< Enable consistency checks when processing tables + BOOLEAN ColourHighlighting; ///< Enable UEFI shell colour codes in output + EREPORT_OPTION ReportType; ///< Type of report to create + BOOLEAN MandatoryTableValidate; ///< Validate presence of mandatory tables + UINTN MandatoryTableSpec; ///< Specification of which tables are mandatory +} ACPI_VIEW_CONFIG; /** This function selects an ACPI table in current context. @@ -161,8 +49,8 @@ GetSelectedAcpiTable ( **/ VOID EFIAPI -SelectAcpiTable ( - CONST CHAR16* TableName +AcpiViewConfigSelectTable ( + CONST CHAR16 *TableName ); /** @@ -170,8 +58,12 @@ SelectAcpiTable ( **/ VOID EFIAPI -AcpiConfigSetDefaults ( +AcpiViewConfigSetDefaults ( VOID ); +extern ACPI_VIEW_CONFIG mConfig; +extern SELECTED_ACPI_TABLE mSelectedAcpiTable; + #endif // ACPI_VIEW_CONFIG_H_ + diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c index d2f26ff89f12..9613c16f5703 100644 --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/UefiShellAcpiViewCommandLib.c @@ -197,7 +197,7 @@ ShellCommandRunAcpiView ( CONST CHAR16* SelectedTableName; // Set configuration defaults - AcpiConfigSetDefaults (); + AcpiViewConfigSetDefaults (); ShellStatus = SHELL_SUCCESS; Package = NULL; @@ -290,31 +290,31 @@ ShellCommandRunAcpiView ( ShellStatus = SHELL_INVALID_PARAMETER; } else { // Turn on colour highlighting if requested - SetColourHighlighting (ShellCommandLineGetFlag (Package, L"-h")); + mConfig.ColourHighlighting = ShellCommandLineGetFlag (Package, L"-h"); // Surpress consistency checking if requested - SetConsistencyChecking (!ShellCommandLineGetFlag (Package, L"-q")); + mConfig.ConsistencyCheck = !ShellCommandLineGetFlag (Package, L"-q"); // Evaluate the parameters for mandatory ACPI table presence checks - SetMandatoryTableValidate (ShellCommandLineGetFlag (Package, L"-r")); + mConfig.MandatoryTableValidate = ShellCommandLineGetFlag (Package, L"-r"); MandatoryTableSpecStr = ShellCommandLineGetValue (Package, L"-r"); if (MandatoryTableSpecStr != NULL) { - SetMandatoryTableSpec (ShellHexStrToUintn (MandatoryTableSpecStr)); + mConfig.MandatoryTableSpec = ShellHexStrToUintn (MandatoryTableSpecStr); } if (ShellCommandLineGetFlag (Package, L"-l")) { - SetReportOption (ReportTableList); + mConfig.ReportType = ReportTableList; } else { SelectedTableName = ShellCommandLineGetValue (Package, L"-s"); if (SelectedTableName != NULL) { - SelectAcpiTable (SelectedTableName); - SetReportOption (ReportSelected); + AcpiViewConfigSelectTable (SelectedTableName); + mConfig.ReportType = ReportSelected; if (ShellCommandLineGetFlag (Package, L"-d")) { // Create a temporary file to check if the media is writable. CHAR16 FileNameBuffer[MAX_FILE_NAME_LEN]; - SetReportOption (ReportDumpBinFile); + mConfig.ReportType = ReportDumpBinFile; UnicodeSPrint ( FileNameBuffer, -- 2.24.1.windows.2 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62553): https://edk2.groups.io/g/devel/message/62553 Mute This Topic: https://groups.io/mt/75504254/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-