On 05/04/18 18:46, Jaben Carsey wrote:
> Commit eece4292acc80 changed a variable name, which was tied directly to a
> config file entry. this seperates the itnernal variable names from the
> config file entries by having the internal dict accessed through a translation
> of key words.
>
> Cc: Liming Gao <liming....@intel.com>
> Cc: Yonghong Zhu <yonghong....@intel.com>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jaben Carsey <jaben.car...@intel.com>
> ---
> BaseTools/Source/Python/Ecc/Configuration.py | 101 +++++++++++++++++++-
> 1 file changed, 98 insertions(+), 3 deletions(-)
>
> diff --git a/BaseTools/Source/Python/Ecc/Configuration.py
> b/BaseTools/Source/Python/Ecc/Configuration.py
> index b5b583be8c4a..72377070f831 100644
> --- a/BaseTools/Source/Python/Ecc/Configuration.py
> +++ b/BaseTools/Source/Python/Ecc/Configuration.py
> @@ -1,7 +1,7 @@
> ## @file
> # This file is used to define class Configuration
> #
> -# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.<BR>
> # This program and the accompanying materials
> # are licensed and made available under the terms and conditions of the BSD
> License
> # which accompanies this distribution. The full text of the license may be
> found at
> @@ -20,6 +20,101 @@ from Common.DataType import *
> from Common.String import *
> from Common.LongFilePathSupport import OpenLongFilePath as open
>
> +_ConfigFileToInternalTranslation = {
> + # not same
> + "ModifierList":"ModifierSet",
> +
> + # same
> + "Version":"Version",
> + "CheckAll":"CheckAll",
> + "AutoCorrect":"AutoCorrect",
> + "GeneralCheckAll":"GeneralCheckAll",
> + "GeneralCheckNoTab":"GeneralCheckNoTab",
> + "GeneralCheckTabWidth":"GeneralCheckTabWidth",
> + "GeneralCheckIndentation":"GeneralCheckIndentation",
> + "GeneralCheckIndentationWidth":"GeneralCheckIndentationWidth",
> + "GeneralCheckLine":"GeneralCheckLine",
> + "GeneralCheckLineWidth":"GeneralCheckLineWidth",
> + "GeneralCheckNo_Asm":"GeneralCheckNo_Asm",
> + "GeneralCheckNoProgma":"GeneralCheckNoProgma",
> + "GeneralCheckCarriageReturn":"GeneralCheckCarriageReturn",
> + "GeneralCheckFileExistence":"GeneralCheckFileExistence",
> + "GeneralCheckNonAcsii":"GeneralCheckNonAcsii",
> + "GeneralCheckUni":"GeneralCheckUni",
> + "SpaceCheckAll":"SpaceCheckAll",
> + "PredicateExpressionCheckAll":"PredicateExpressionCheckAll",
> +
> "PredicateExpressionCheckBooleanValue":"PredicateExpressionCheckBooleanValue",
> +
> "PredicateExpressionCheckNonBooleanOperator":"PredicateExpressionCheckNonBooleanOperator",
> +
> "PredicateExpressionCheckComparisonNullType":"PredicateExpressionCheckComparisonNullType",
> + "HeaderCheckAll":"HeaderCheckAll",
> + "HeaderCheckFile":"HeaderCheckFile",
> + "HeaderCheckFunction":"HeaderCheckFunction",
> + "HeaderCheckFileCommentEnd":"HeaderCheckFileCommentEnd",
> +
> "HeaderCheckCFileCommentStartSpacesNum":"HeaderCheckCFileCommentStartSpacesNum",
> +
> "HeaderCheckCFileCommentReferenceFormat":"HeaderCheckCFileCommentReferenceFormat",
> +
> "HeaderCheckCFileCommentLicenseFormat":"HeaderCheckCFileCommentLicenseFormat",
> + "CFunctionLayoutCheckAll":"CFunctionLayoutCheckAll",
> + "CFunctionLayoutCheckReturnType":"CFunctionLayoutCheckReturnType",
> +
> "CFunctionLayoutCheckOptionalFunctionalModifier":"CFunctionLayoutCheckOptionalFunctionalModifier",
> + "CFunctionLayoutCheckFunctionName":"CFunctionLayoutCheckFunctionName",
> +
> "CFunctionLayoutCheckFunctionPrototype":"CFunctionLayoutCheckFunctionPrototype",
> + "CFunctionLayoutCheckFunctionBody":"CFunctionLayoutCheckFunctionBody",
> +
> "CFunctionLayoutCheckDataDeclaration":"CFunctionLayoutCheckDataDeclaration",
> +
> "CFunctionLayoutCheckNoInitOfVariable":"CFunctionLayoutCheckNoInitOfVariable",
> + "CFunctionLayoutCheckNoStatic":"CFunctionLayoutCheckNoStatic",
> + "IncludeFileCheckAll":"IncludeFileCheckAll",
> + "IncludeFileCheckSameName":"IncludeFileCheckSameName",
> + "IncludeFileCheckIfndefStatement":"IncludeFileCheckIfndefStatement",
> + "IncludeFileCheckData":"IncludeFileCheckData",
> + "DeclarationDataTypeCheckAll":"DeclarationDataTypeCheckAll",
> +
> "DeclarationDataTypeCheckNoUseCType":"DeclarationDataTypeCheckNoUseCType",
> +
> "DeclarationDataTypeCheckInOutModifier":"DeclarationDataTypeCheckInOutModifier",
> +
> "DeclarationDataTypeCheckEFIAPIModifier":"DeclarationDataTypeCheckEFIAPIModifier",
> +
> "DeclarationDataTypeCheckEnumeratedType":"DeclarationDataTypeCheckEnumeratedType",
> +
> "DeclarationDataTypeCheckStructureDeclaration":"DeclarationDataTypeCheckStructureDeclaration",
> +
> "DeclarationDataTypeCheckSameStructure":"DeclarationDataTypeCheckSameStructure",
> + "DeclarationDataTypeCheckUnionType":"DeclarationDataTypeCheckUnionType",
> + "NamingConventionCheckAll":"NamingConventionCheckAll",
> +
> "NamingConventionCheckDefineStatement":"NamingConventionCheckDefineStatement",
> +
> "NamingConventionCheckTypedefStatement":"NamingConventionCheckTypedefStatement",
> +
> "NamingConventionCheckIfndefStatement":"NamingConventionCheckIfndefStatement",
> + "NamingConventionCheckPathName":"NamingConventionCheckPathName",
> + "NamingConventionCheckVariableName":"NamingConventionCheckVariableName",
> + "NamingConventionCheckFunctionName":"NamingConventionCheckFunctionName",
> +
> "NamingConventionCheckSingleCharacterVariable":"NamingConventionCheckSingleCharacterVariable",
> + "DoxygenCheckAll":"DoxygenCheckAll",
> + "DoxygenCheckFileHeader":"DoxygenCheckFileHeader",
> + "DoxygenCheckFunctionHeader":"DoxygenCheckFunctionHeader",
> + "DoxygenCheckCommentDescription":"DoxygenCheckCommentDescription",
> + "DoxygenCheckCommentFormat":"DoxygenCheckCommentFormat",
> + "DoxygenCheckCommand":"DoxygenCheckCommand",
> + "MetaDataFileCheckAll":"MetaDataFileCheckAll",
> + "MetaDataFileCheckPathName":"MetaDataFileCheckPathName",
> + "MetaDataFileCheckGenerateFileList":"MetaDataFileCheckGenerateFileList",
> +
> "MetaDataFileCheckPathOfGenerateFileList":"MetaDataFileCheckPathOfGenerateFileList",
> + "MetaDataFileCheckLibraryInstance":"MetaDataFileCheckLibraryInstance",
> +
> "MetaDataFileCheckLibraryInstanceDependent":"MetaDataFileCheckLibraryInstanceDependent",
> +
> "MetaDataFileCheckLibraryInstanceOrder":"MetaDataFileCheckLibraryInstanceOrder",
> + "MetaDataFileCheckLibraryNoUse":"MetaDataFileCheckLibraryNoUse",
> +
> "MetaDataFileCheckLibraryDefinedInDec":"MetaDataFileCheckLibraryDefinedInDec",
> + "MetaDataFileCheckBinaryInfInFdf":"MetaDataFileCheckBinaryInfInFdf",
> + "MetaDataFileCheckPcdDuplicate":"MetaDataFileCheckPcdDuplicate",
> + "MetaDataFileCheckPcdFlash":"MetaDataFileCheckPcdFlash",
> + "MetaDataFileCheckPcdNoUse":"MetaDataFileCheckPcdNoUse",
> + "MetaDataFileCheckGuidDuplicate":"MetaDataFileCheckGuidDuplicate",
> + "MetaDataFileCheckModuleFileNoUse":"MetaDataFileCheckModuleFileNoUse",
> + "MetaDataFileCheckPcdType":"MetaDataFileCheckPcdType",
> +
> "MetaDataFileCheckModuleFileGuidDuplication":"MetaDataFileCheckModuleFileGuidDuplication",
> + "UniCheckAll":"UniCheckAll",
> + "UniCheckHelpInfo":"UniCheckHelpInfo",
> + "UniCheckPCDInfo":"UniCheckPCDInfo",
> + "GeneralCheckUni":"GeneralCheckUni",
> + "SmmCommParaCheckAll":"SmmCommParaCheckAll",
> + "SmmCommParaCheckBufferType":"SmmCommParaCheckBufferType",
> + "BinaryExtList":"BinaryExtList",
> + "ScanOnlyDirList":"ScanOnlyDirList"
> + }
> +
> ## Configuration
> #
> # This class is used to define all items in configuration file
> @@ -297,7 +392,7 @@ class Configuration(object):
> Line = CleanString(Line)
> if Line != '':
> List = GetSplitValueList(Line, TAB_EQUAL_SPLIT)
> - if List[0] not in self.__dict__:
> + if _ConfigFileToInternalTranslation[List[0]] not in
> self.__dict__:
> ErrorMsg = "Invalid configuration option '%s' was found"
> % List[0]
> EdkLogger.error("Ecc", EdkLogger.ECC_ERROR, ErrorMsg,
> File = Filepath, Line = LineNo)
> if List[0] == 'ModifierList':
> @@ -312,7 +407,7 @@ class Configuration(object):
> List[1] = GetSplitValueList(List[1], TAB_COMMA_SPLIT)
> if List[0] == 'Copyright':
> List[1] = GetSplitValueList(List[1], TAB_COMMA_SPLIT)
> - self.__dict__[List[0]] = List[1]
> + self.__dict__[_ConfigFileToInternalTranslation[List[0]]] =
> List[1]
>
> def ShowMe(self):
> print self.Filename
>
Ouch, I didn't think the dictionary was this large. :/
Anyway, I still think this is the right approach. Three comments:
* The commit message is too wide. It should be 74 chars or so.
* Can we keep the list sorted (in the source code)? Because, at the
moment, the "GeneralCheckUni":"GeneralCheckUni" entry is duplicated.
That's easier to notice if the entries are sorted.
Maybe add a comment too about keeping the lists sorted.
* If the config file contains an error (typo or completely bogus option
name), now we will not get a nice error message. Because, before we
get to evaluate "translated_name in self.__dict__", the translation
lookup will itself fail. I think that throws a KeyError exception.
I think we should:
- report "invalid config option" if the explicit translation fails,
- use an additional assertion that the translated option name is in
self.__dict__ (it should be an assertion because if it fails, it
indicates a bug in Ecc).
To save you some time, I'll paste the uniquely sorted list of "same"
entries at the end (87 key:value pairs).
Thanks!
Laszlo
"AutoCorrect":"AutoCorrect",
"BinaryExtList":"BinaryExtList",
"CFunctionLayoutCheckAll":"CFunctionLayoutCheckAll",
"CFunctionLayoutCheckDataDeclaration":"CFunctionLayoutCheckDataDeclaration",
"CFunctionLayoutCheckFunctionBody":"CFunctionLayoutCheckFunctionBody",
"CFunctionLayoutCheckFunctionName":"CFunctionLayoutCheckFunctionName",
"CFunctionLayoutCheckFunctionPrototype":"CFunctionLayoutCheckFunctionPrototype",
"CFunctionLayoutCheckNoInitOfVariable":"CFunctionLayoutCheckNoInitOfVariable",
"CFunctionLayoutCheckNoStatic":"CFunctionLayoutCheckNoStatic",
"CFunctionLayoutCheckOptionalFunctionalModifier":"CFunctionLayoutCheckOptionalFunctionalModifier",
"CFunctionLayoutCheckReturnType":"CFunctionLayoutCheckReturnType",
"CheckAll":"CheckAll",
"DeclarationDataTypeCheckAll":"DeclarationDataTypeCheckAll",
"DeclarationDataTypeCheckEFIAPIModifier":"DeclarationDataTypeCheckEFIAPIModifier",
"DeclarationDataTypeCheckEnumeratedType":"DeclarationDataTypeCheckEnumeratedType",
"DeclarationDataTypeCheckInOutModifier":"DeclarationDataTypeCheckInOutModifier",
"DeclarationDataTypeCheckNoUseCType":"DeclarationDataTypeCheckNoUseCType",
"DeclarationDataTypeCheckSameStructure":"DeclarationDataTypeCheckSameStructure",
"DeclarationDataTypeCheckStructureDeclaration":"DeclarationDataTypeCheckStructureDeclaration",
"DeclarationDataTypeCheckUnionType":"DeclarationDataTypeCheckUnionType",
"DoxygenCheckAll":"DoxygenCheckAll",
"DoxygenCheckCommand":"DoxygenCheckCommand",
"DoxygenCheckCommentDescription":"DoxygenCheckCommentDescription",
"DoxygenCheckCommentFormat":"DoxygenCheckCommentFormat",
"DoxygenCheckFileHeader":"DoxygenCheckFileHeader",
"DoxygenCheckFunctionHeader":"DoxygenCheckFunctionHeader",
"GeneralCheckAll":"GeneralCheckAll",
"GeneralCheckCarriageReturn":"GeneralCheckCarriageReturn",
"GeneralCheckFileExistence":"GeneralCheckFileExistence",
"GeneralCheckIndentation":"GeneralCheckIndentation",
"GeneralCheckIndentationWidth":"GeneralCheckIndentationWidth",
"GeneralCheckLine":"GeneralCheckLine",
"GeneralCheckLineWidth":"GeneralCheckLineWidth",
"GeneralCheckNoProgma":"GeneralCheckNoProgma",
"GeneralCheckNoTab":"GeneralCheckNoTab",
"GeneralCheckNo_Asm":"GeneralCheckNo_Asm",
"GeneralCheckNonAcsii":"GeneralCheckNonAcsii",
"GeneralCheckTabWidth":"GeneralCheckTabWidth",
"GeneralCheckUni":"GeneralCheckUni",
"HeaderCheckAll":"HeaderCheckAll",
"HeaderCheckCFileCommentLicenseFormat":"HeaderCheckCFileCommentLicenseFormat",
"HeaderCheckCFileCommentReferenceFormat":"HeaderCheckCFileCommentReferenceFormat",
"HeaderCheckCFileCommentStartSpacesNum":"HeaderCheckCFileCommentStartSpacesNum",
"HeaderCheckFile":"HeaderCheckFile",
"HeaderCheckFileCommentEnd":"HeaderCheckFileCommentEnd",
"HeaderCheckFunction":"HeaderCheckFunction",
"IncludeFileCheckAll":"IncludeFileCheckAll",
"IncludeFileCheckData":"IncludeFileCheckData",
"IncludeFileCheckIfndefStatement":"IncludeFileCheckIfndefStatement",
"IncludeFileCheckSameName":"IncludeFileCheckSameName",
"MetaDataFileCheckAll":"MetaDataFileCheckAll",
"MetaDataFileCheckBinaryInfInFdf":"MetaDataFileCheckBinaryInfInFdf",
"MetaDataFileCheckGenerateFileList":"MetaDataFileCheckGenerateFileList",
"MetaDataFileCheckGuidDuplicate":"MetaDataFileCheckGuidDuplicate",
"MetaDataFileCheckLibraryDefinedInDec":"MetaDataFileCheckLibraryDefinedInDec",
"MetaDataFileCheckLibraryInstance":"MetaDataFileCheckLibraryInstance",
"MetaDataFileCheckLibraryInstanceDependent":"MetaDataFileCheckLibraryInstanceDependent",
"MetaDataFileCheckLibraryInstanceOrder":"MetaDataFileCheckLibraryInstanceOrder",
"MetaDataFileCheckLibraryNoUse":"MetaDataFileCheckLibraryNoUse",
"MetaDataFileCheckModuleFileGuidDuplication":"MetaDataFileCheckModuleFileGuidDuplication",
"MetaDataFileCheckModuleFileNoUse":"MetaDataFileCheckModuleFileNoUse",
"MetaDataFileCheckPathName":"MetaDataFileCheckPathName",
"MetaDataFileCheckPathOfGenerateFileList":"MetaDataFileCheckPathOfGenerateFileList",
"MetaDataFileCheckPcdDuplicate":"MetaDataFileCheckPcdDuplicate",
"MetaDataFileCheckPcdFlash":"MetaDataFileCheckPcdFlash",
"MetaDataFileCheckPcdNoUse":"MetaDataFileCheckPcdNoUse",
"MetaDataFileCheckPcdType":"MetaDataFileCheckPcdType",
"NamingConventionCheckAll":"NamingConventionCheckAll",
"NamingConventionCheckDefineStatement":"NamingConventionCheckDefineStatement",
"NamingConventionCheckFunctionName":"NamingConventionCheckFunctionName",
"NamingConventionCheckIfndefStatement":"NamingConventionCheckIfndefStatement",
"NamingConventionCheckPathName":"NamingConventionCheckPathName",
"NamingConventionCheckSingleCharacterVariable":"NamingConventionCheckSingleCharacterVariable",
"NamingConventionCheckTypedefStatement":"NamingConventionCheckTypedefStatement",
"NamingConventionCheckVariableName":"NamingConventionCheckVariableName",
"PredicateExpressionCheckAll":"PredicateExpressionCheckAll",
"PredicateExpressionCheckBooleanValue":"PredicateExpressionCheckBooleanValue",
"PredicateExpressionCheckComparisonNullType":"PredicateExpressionCheckComparisonNullType",
"PredicateExpressionCheckNonBooleanOperator":"PredicateExpressionCheckNonBooleanOperator",
"ScanOnlyDirList":"ScanOnlyDirList",
"SmmCommParaCheckAll":"SmmCommParaCheckAll",
"SmmCommParaCheckBufferType":"SmmCommParaCheckBufferType",
"SpaceCheckAll":"SpaceCheckAll",
"UniCheckAll":"UniCheckAll",
"UniCheckHelpInfo":"UniCheckHelpInfo",
"UniCheckPCDInfo":"UniCheckPCDInfo",
"Version":"Version"
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
