[edk2] [Patch] BaseTools: Fix PcdArray issue

Philippe Mathieu-Daudé philmd at redhat.com
Fri Dec 14 03:21:38 PST 2018


Hi Bob,

On 12/14/18 10:55 AM, BobCF wrote:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1390
> 1. support hex number for array index
> 2. support Non-Dynamic Pcd for array data type
> 3. support {} and {CODE()} for array data type
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Bob Feng <bob.c.feng at intel.com>
> Cc: Liming Gao <liming.gao at intel.com>
> Cc: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> ---
>  BaseTools/Source/Python/Common/Misc.py        |  6 ++
>  .../Python/Workspace/BuildClassObject.py      |  3 +-
>  .../Source/Python/Workspace/DscBuildData.py   | 59 ++++++++++++-------
>  3 files changed, 45 insertions(+), 23 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/Common/Misc.py b/BaseTools/Source/Python/Common/Misc.py
> index b063f064fb..ea09f85e70 100644
> --- a/BaseTools/Source/Python/Common/Misc.py
> +++ b/BaseTools/Source/Python/Common/Misc.py
> @@ -2144,10 +2144,16 @@ def CopyDict(ori_dict):
>          if isinstance(ori_dict[key],(dict,OrderedDict)):
>              new_dict[key] = CopyDict(ori_dict[key])
>          else:
>              new_dict[key] = ori_dict[key]
>      return new_dict
> +
> +#
> +# Remove the c/c++ comments: // and /* */
> +#
> +def RemoveCComments(ctext):
> +    return re.sub('//.*?\n|/\*.*?\*/', '\n', ctext, flags=re.S)

What about multi-line comments?

>  ##
>  #
>  # This acts like the main() function for the script, unless it is 'import'ed into another
>  # script.
>  #
> diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py b/BaseTools/Source/Python/Workspace/BuildClassObject.py
> index 008eee1a16..e9a1195fd2 100644
> --- a/BaseTools/Source/Python/Workspace/BuildClassObject.py
> +++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py
> @@ -17,11 +17,11 @@ import collections
>  import re
>  from collections import OrderedDict
>  from Common.Misc import CopyDict
>  import copy
>  StructPattern = re.compile(r'[_a-zA-Z][0-9A-Za-z_\[\]]*$')
> -ArrayIndex = re.compile("\[\s*\d{0,1}\s*\]")
> +ArrayIndex = re.compile("\[\s*[0-9a-fA-FxX]*\s*\]")

These regex changes seems easily unit-testable, is it possible to add
some tests? In particular the RemoveCComments() seems fragile.

>  ## PcdClassObject
>  #
>  # This Class is used for PcdObject
>  #
>  # @param object:             Inherited from object class
> @@ -82,10 +82,11 @@ class PcdClassObject(object):
>          dimension = ArrayIndex.findall(self._DatumType)
>          for item in dimension:
>              maxsize = item.lstrip("[").rstrip("]").strip()
>              if not maxsize:
>                  maxsize = "-1"
> +            maxsize = str(int(maxsize,16)) if maxsize.startswith(("0x","0X")) else maxsize
>              self._Capacity.append(maxsize)
>          if hasattr(self, "SkuOverrideValues"):
>              for sku in self.SkuOverrideValues:
>                  for defaultstore in self.SkuOverrideValues[sku]:
>                      fields = self.SkuOverrideValues[sku][defaultstore]
> diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py b/BaseTools/Source/Python/Workspace/DscBuildData.py
> index b485c75a84..37fb8d56b6 100644
> --- a/BaseTools/Source/Python/Workspace/DscBuildData.py
> +++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
> @@ -31,11 +31,11 @@ from .MetaDataTable import *
>  from .MetaFileTable import *
>  from .MetaFileParser import *
>  
>  from .WorkspaceCommon import GetDeclaredPcd
>  from Common.Misc import AnalyzeDscPcd
> -from Common.Misc import ProcessDuplicatedInf
> +from Common.Misc import ProcessDuplicatedInf,RemoveCComments
>  import re
>  from Common.Parsing import IsValidWord
>  from Common.VariableAttributes import VariableAttributes
>  import Common.GlobalData as GlobalData
>  import subprocess
> @@ -1573,11 +1573,11 @@ class DscBuildData(PlatformBuildClassObject):
>                  mindefaultstorename = DefaultStoreObj.GetMin(PcdDefaultStoreSet)
>                  str_pcd_obj.SkuInfoList[self.SkuIdMgr.SystemSkuId].HiiDefaultValue = str_pcd_obj.SkuInfoList[self.SkuIdMgr.SystemSkuId].DefaultStoreDict[mindefaultstorename]
>  
>              for str_pcd_obj in S_pcd_set.values():
>  
> -                str_pcd_obj.MaxDatumSize = self.GetStructurePcdMaxSize(str_pcd_obj)
> +                str_pcd_obj.MaxDatumSize = DscBuildData.GetStructurePcdMaxSize(str_pcd_obj)

This static change isn't described in the commit, if you think this
isn't worth a separate commit, can you add a note about it?

>                  Pcds[str_pcd_obj.TokenCName, str_pcd_obj.TokenSpaceGuidCName] = str_pcd_obj
>                  Pcds[str_pcd_obj.TokenCName, str_pcd_obj.TokenSpaceGuidCName].CustomAttribute['IsStru']=True
>  
>              for pcdkey in Pcds:
>                  pcd = Pcds[pcdkey]
> @@ -1687,13 +1687,14 @@ class DscBuildData(PlatformBuildClassObject):
>                  if SkuName not in Pcds[PcdCName, TokenSpaceGuid].DscRawValue:
>                      Pcds[PcdCName, TokenSpaceGuid].DscRawValue[SkuName] = {}
>                  Pcds[PcdCName, TokenSpaceGuid].DscRawValue[SkuName][TAB_DEFAULT_STORES_DEFAULT] = Settings[0]
>          return Pcds
>  
> -    def GetStructurePcdMaxSize(self, str_pcd):
> +    @staticmethod
> +    def GetStructurePcdMaxSize(str_pcd):
>          pcd_default_value = str_pcd.DefaultValue
> -        sku_values = [skuobj.HiiDefaultValue if str_pcd.Type in [self._PCD_TYPE_STRING_[MODEL_PCD_DYNAMIC_HII], self._PCD_TYPE_STRING_[MODEL_PCD_DYNAMIC_EX_HII]] else skuobj.DefaultValue for skuobj in str_pcd.SkuInfoList.values()]
> +        sku_values = [skuobj.HiiDefaultValue if str_pcd.Type in [DscBuildData._PCD_TYPE_STRING_[MODEL_PCD_DYNAMIC_HII], DscBuildData._PCD_TYPE_STRING_[MODEL_PCD_DYNAMIC_EX_HII]] else skuobj.DefaultValue for skuobj in str_pcd.SkuInfoList.values()]
>          sku_values.append(pcd_default_value)
>  
>          def get_length(value):
>              Value = value.strip()
>              if len(value) > 1:
> @@ -1701,11 +1702,14 @@ class DscBuildData(PlatformBuildClassObject):
>                      return 16
>                  if Value.startswith('L"') and Value.endswith('"'):
>                      return len(Value[2:-1])
>                  if Value[0] == '"' and Value[-1] == '"':
>                      return len(Value) - 2
> -                if Value[0] == '{' and Value[-1] == '}':
> +                if Value.strip().startswith("{CODE("):
> +                    tmpValue = RemoveCComments(Value)
> +                    return len(tmpValue.split(","))
> +                if (Value[0] == '{' and Value[-1] == '}'):

OK.

>                      return len(Value.split(","))
>                  if Value.startswith("L'") and Value.endswith("'") and len(list(Value[2:-1])) > 1:
>                      return  len(list(Value[2:-1]))
>                  if Value[0] == "'" and Value[-1] == "'" and len(list(Value[1:-1])) > 1:
>                      return len(Value) - 2
> @@ -1864,11 +1868,12 @@ class DscBuildData(PlatformBuildClassObject):
>              for dem in Pcd.Capacity:
>                  if dem == '0':
>                      r_datatype.append("[1]")
>                  else:
>                      r_datatype.append("[" + dem + "]")
> -            CApp = '  Size = sizeof(%s);\n' % ("".join(r_datatype))
> +            sizebasevalue = "(%s / sizeof(%s) + 1)" % ((DscBuildData.GetStructurePcdMaxSize(Pcd), Pcd.BaseDatumType))
> +            CApp = '  Size = sizeof(%s) > %s?sizeof(%s) : %s ;\n' % ( ("".join(r_datatype), sizebasevalue, "".join(r_datatype), sizebasevalue)  )
>          else:
>              CApp = '  Size = sizeof(%s);\n' % (Pcd.DatumType)
>          CApp = CApp + '  Cal_%s_%s_Size(&Size);\n' % (Pcd.TokenSpaceGuidCName, Pcd.TokenCName)
>          return CApp
>  
> @@ -2246,11 +2251,11 @@ class DscBuildData(PlatformBuildClassObject):
>  
>              CApp = CApp + '\n'
>  
>              PcdDefaultValue = StringToArray(Pcd.DefaultValueFromDec.strip())
>  
> -            InitByteValue += '%s.%s.%s.%s|%s|%s\n' % (SkuName, DefaultStoreName, Pcd.TokenSpaceGuidCName, Pcd.TokenCName, Pcd.BaseDatumType, PcdDefaultValue)
> +            InitByteValue += '%s.%s.%s.%s|%s|%s\n' % (SkuName, DefaultStoreName, Pcd.TokenSpaceGuidCName, Pcd.TokenCName, Pcd.DatumType, PcdDefaultValue)
>  
>              #
>              # Get current PCD value and size
>              #
>              CApp = CApp + '  OriginalPcd = PcdGetPtr (%s, %s, %s, %s, &OriginalSize);\n' % (SkuName, DefaultStoreName, Pcd.TokenSpaceGuidCName, Pcd.TokenCName)
> @@ -2316,33 +2321,43 @@ class DscBuildData(PlatformBuildClassObject):
>              return CApp
>          if not Pcd.IsArray():
>              return CApp
>          Demesion = ""
>          for d in Pcd.Capacity:
> -            if d == "0":
> -                Demesion += "[]"
> -            else:
> -                Demesion += "["+d+"]"
> +            Demesion += "[]"
>  
>          Value = Pcd.DefaultValueFromDec
>          if "{CODE(" in Pcd.DefaultValueFromDec:
>              realvalue = Pcd.DefaultValueFromDec.strip()[6:-2] # "{CODE(").rstrip(")}"
> -            CApp += "static %s %s_%s_INIT_Value%s = %s;\n" % (Pcd.BaseDatumType,Pcd.TokenSpaceGuidCName,Pcd.TokenCName,Demesion,realvalue)
> +        else:
> +            realvalue = Pcd.DefaultValueFromDec.strip()
> +        CApp += "static %s %s_%s_INIT_Value%s = %s;\n" % (Pcd.BaseDatumType,Pcd.TokenSpaceGuidCName,Pcd.TokenCName,Demesion,realvalue)
>  
> -        for skuname in Pcd.SkuInfoList:
> -            skuinfo = Pcd.SkuInfoList[skuname]
> -            if skuinfo.VariableName:
> -                for defaultstore in skuinfo.DefaultStoreDict:
> -                    Value = skuinfo[defaultstore]
> +        if Pcd.Type in PCD_DYNAMIC_TYPE_SET | PCD_DYNAMIC_EX_TYPE_SET:
> +            for skuname in Pcd.SkuInfoList:
> +                skuinfo = Pcd.SkuInfoList[skuname]
> +                if skuinfo.VariableName:
> +                    for defaultstore in skuinfo.DefaultStoreDict:
> +                        Value = skuinfo[defaultstore]
> +                        if "{CODE(" in Value:
> +                            realvalue = Value.strip()[6:-2] # "{CODE(").rstrip(")}"
> +                        else:
> +                            realvalue = Value.strip()
> +                        CApp += "static %s %s_%s_%s_%s_Value%s = %s;\n" % (Pcd.BaseDatumType,Pcd.TokenSpaceGuidCName,Pcd.TokenCName,skuname,defaultstore,Demesion,realvalue)
> +                else:
> +                    Value = skuinfo.DefaultValue
>                      if "{CODE(" in Value:
>                          realvalue = Value.strip()[6:-2] # "{CODE(").rstrip(")}"
> -                        CApp += "static %s %s_%s_%s_%s_Value%s = %s;\n" % (Pcd.BaseDatumType,Pcd.TokenSpaceGuidCName,Pcd.TokenCName,skuname,defaultstore,Demesion,realvalue)
> -            else:
> -                Value = skuinfo.DefaultValue
> -                if "{CODE(" in Value:
> -                    realvalue = Value.strip()[6:-2] # "{CODE(").rstrip(")}"
> +                    else:
> +                        realvalue = Value.strip()
>                      CApp += "static %s %s_%s_%s_%s_Value%s = %s;\n" % (Pcd.BaseDatumType,Pcd.TokenSpaceGuidCName,Pcd.TokenCName,skuname,TAB_DEFAULT_STORES_DEFAULT,Demesion,realvalue)
> +        else:
> +            if "{CODE(" in Pcd.DefaultValue:
> +                realvalue = Pcd.DefaultValue.strip()[6:-2] # "{CODE(").rstrip(")}"
> +            else:
> +                realvalue = Pcd.DefaultValue.strip()
> +            CApp += "static %s %s_%s_DEFAULT_STANDARD_Value%s = %s;\n" % (Pcd.BaseDatumType,Pcd.TokenSpaceGuidCName,Pcd.TokenCName,Demesion,realvalue)
>          return CApp
>      def SkuOverrideValuesEmpty(self,OverrideValues):
>          if not OverrideValues:
>              return True
>          for key in OverrideValues:
> 

Rest looks OK.

Regards,

Phil.


More information about the edk2-devel mailing list