public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Gao, Liming" <liming.gao@intel.com>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>,
	"Feng, Bob C" <bob.c.feng@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: [Patch V2] BaseTools: Fix PcdArray issue
Date: Tue, 18 Dec 2018 01:47:53 +0000	[thread overview]
Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E38DC6F@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <91b38ee2-7680-9276-6904-ac3a1e6ebfdc@redhat.com>

Reviewed-by: Liming Gao <liming.gao@intel.com>

>-----Original Message-----
>From: Philippe Mathieu-Daudé [mailto:philmd@redhat.com]
>Sent: Monday, December 17, 2018 6:05 PM
>To: Feng, Bob C <bob.c.feng@intel.com>; edk2-devel@lists.01.org
>Cc: Gao, Liming <liming.gao@intel.com>; Ard Biesheuvel
><ard.biesheuvel@linaro.org>
>Subject: Re: [Patch V2] BaseTools: Fix PcdArray issue
>
>On 12/14/18 5:15 PM, BobCF wrote:
>> From: "Feng, Bob C" <bob.c.feng@intel.com>
>>
>> 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
>> 4. Change GetStructurePcdMaxSize to be a static function since it need to
>> be called in another static function. And this function does not depend on
>> it's class instance.
>> 5. Add unittest for RemoveCComments function and
>> ArrayIndex regular expression.
>
>Thanks for this Bob!
>
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Bob Feng <bob.c.feng@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
>
>Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
>> ---
>>  BaseTools/Source/Python/Common/Misc.py        |  6 ++
>>  .../Python/Workspace/BuildClassObject.py      |  3 +-
>>  .../Source/Python/Workspace/DscBuildData.py   | 59 ++++++++++++-------
>>  BaseTools/Tests/TestRegularExpression.py      | 54 +++++++++++++++++
>>  4 files changed, 99 insertions(+), 23 deletions(-)
>>  create mode 100644 BaseTools/Tests/TestRegularExpression.py
>>
>> 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)
>>  ##
>>  #
>>  # 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*\]")
>>  ## 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[minde
>faultstorename]
>>
>>              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)
>>                  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] == '}'):
>>                      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,Demesi
>on,realvalue)
>> +        else:
>> +            realvalue = Pcd.DefaultValueFromDec.strip()
>> +        CApp += "static %s %s_%s_INIT_Value%s = %s;\n" %
>(Pcd.BaseDatumType,Pcd.TokenSpaceGuidCName,Pcd.TokenCName,Demesi
>on,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,skunam
>e,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,skunam
>e,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,skunam
>e,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,Demesi
>on,realvalue)
>>          return CApp
>>      def SkuOverrideValuesEmpty(self,OverrideValues):
>>          if not OverrideValues:
>>              return True
>>          for key in OverrideValues:
>> diff --git a/BaseTools/Tests/TestRegularExpression.py
>b/BaseTools/Tests/TestRegularExpression.py
>> new file mode 100644
>> index 0000000000..07143d6a76
>> --- /dev/null
>> +++ b/BaseTools/Tests/TestRegularExpression.py
>> @@ -0,0 +1,54 @@
>> +## @file
>> +# Routines for generating Pcd Database
>> +#
>> +# Copyright (c) 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
>> +# http://opensource.org/licenses/bsd-license.php
>> +#
>> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
>BASIS,
>> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
>EXPRESS OR IMPLIED.
>> +
>> +import unittest
>> +from Common.Misc import RemoveCComments
>> +from Workspace.BuildClassObject import ArrayIndex
>> +
>> +class TestRe(unittest.TestCase):
>> +    def test_ccomments(self):
>> +        TestStr1 = """ {0x01,0x02} """
>> +        self.assertEquals(TestStr1, RemoveCComments(TestStr1))
>> +
>> +        TestStr2 = """ L'TestString' """
>> +        self.assertEquals(TestStr2, RemoveCComments(TestStr2))
>> +
>> +        TestStr3 = """ 'TestString' """
>> +        self.assertEquals(TestStr3, RemoveCComments(TestStr3))
>> +
>> +        TestStr4 = """
>> +            {CODE({
>> +              {0x01, {0x02, 0x03, 0x04 }},// Data comment
>> +              {0x01, {0x02, 0x03, 0x04 }},// Data comment
>> +              })
>> +            }  /*
>> +               This is multiple line comments
>> +               The seconde line comment
>> +               */
>> +            // This is a comment
>> +        """
>> +        Expect_TestStr4 = """{CODE({
>> +              {0x01, {0x02, 0x03, 0x04 }},
>> +              {0x01, {0x02, 0x03, 0x04 }},
>> +              })
>> +            }"""
>> +        self.assertEquals(Expect_TestStr4,
>RemoveCComments(TestStr4).strip())
>> +
>> +    def Test_ArrayIndex(self):
>> +        TestStr1 = """[1]"""
>> +        self.assertEquals(['[1]'], ArrayIndex.findall(TestStr1))
>> +
>> +        TestStr2 = """[1][2][0x1][0x01][]"""
>> +        self.assertEquals(['[1]','[2]','[0x1]','[0x01]','[]'],
>ArrayIndex.findall(TestStr2))
>> +
>> +if __name__ == '__main__':
>> +    unittest.main()
>>

      reply	other threads:[~2018-12-18  1:47 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-14 16:15 [Patch V2] BaseTools: Fix PcdArray issue BobCF
2018-12-17 10:05 ` Philippe Mathieu-Daudé
2018-12-18  1:47   ` Gao, Liming [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4A89E2EF3DFEDB4C8BFDE51014F606A14E38DC6F@SHSMSX104.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox