From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 83F472094607E for ; Fri, 4 May 2018 11:24:58 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id DC685400ADF7; Fri, 4 May 2018 18:24:57 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-129.rdu2.redhat.com [10.10.120.129]) by smtp.corp.redhat.com (Postfix) with ESMTP id C560E1121301; Fri, 4 May 2018 18:24:56 +0000 (UTC) To: Jaben Carsey , edk2-devel@lists.01.org Cc: Liming Gao , Yonghong Zhu References: From: Laszlo Ersek Message-ID: <12d74035-8a99-3fd1-01e6-6ad990855832@redhat.com> Date: Fri, 4 May 2018 20:24:55 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.3 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Fri, 04 May 2018 18:24:57 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.6]); Fri, 04 May 2018 18:24:57 +0000 (UTC) for IP:'10.11.54.3' DOMAIN:'int-mx03.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH v2 1/1] BaseTools: Ecc - add dict for config file to internal translation X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 04 May 2018 18:24:59 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 > Cc: Yonghong Zhu > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jaben Carsey > --- > 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.
> +# Copyright (c) 2008 - 2018, Intel Corporation. All rights reserved.
> # 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"