public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Gao, Liming" <liming.gao@intel.com>
To: "Feng, Bob C" <bob.c.feng@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Subject: Re: [Patch] BaseTools: Correct PcdArray value assigment statement
Date: Fri, 4 Jan 2019 01:18:44 +0000	[thread overview]
Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14E3A7F1E@SHSMSX152.ccr.corp.intel.com> (raw)
In-Reply-To: <20181229084308.43512-1-bob.c.feng@intel.com>

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

>-----Original Message-----
>From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>BobCF
>Sent: Saturday, December 29, 2018 4:43 PM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming <liming.gao@intel.com>
>Subject: [edk2] [Patch] BaseTools: Correct PcdArray value assigment
>statement
>
>https://bugzilla.tianocore.org/show_bug.cgi?id=1410
>BaseTools should not generate C structure array initial value
>if the value is not specified with CODE style.
>
>This patch is going to remove the incorrect initial value statement
>and correct the Pcd Array value assignment statement.
>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Bob Feng <bob.c.feng@intel.com>
>Cc: Liming Gao <liming.gao@intel.com>
>---
> .../Python/Workspace/BuildClassObject.py      |   1 -
> .../Source/Python/Workspace/DscBuildData.py   | 117 +++++++++++-------
> 2 files changed, 73 insertions(+), 45 deletions(-)
>
>diff --git a/BaseTools/Source/Python/Workspace/BuildClassObject.py
>b/BaseTools/Source/Python/Workspace/BuildClassObject.py
>index 52b3369561..73920c5153 100644
>--- a/BaseTools/Source/Python/Workspace/BuildClassObject.py
>+++ b/BaseTools/Source/Python/Workspace/BuildClassObject.py
>@@ -99,11 +99,10 @@ class PcdClassObject(object):
>             for demesionattr in self.DefaultValues:
>                 deme = ArrayIndex.findall(demesionattr)
>                 for i in range(len(deme)-1):
>                     if int(deme[i].lstrip("[").rstrip("]").strip()) > int(self._Capacity[i]):
>                         print "error"
>-        self._Capacity = [str(int(d) + 1) for d in self._Capacity]
>         return self._Capacity
>     @property
>     def DatumType(self):
>         return self._DatumType
>
>diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
>b/BaseTools/Source/Python/Workspace/DscBuildData.py
>index 7f6e966b5f..7e82e8e934 100644
>--- a/BaseTools/Source/Python/Workspace/DscBuildData.py
>+++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
>@@ -1738,21 +1738,26 @@ class DscBuildData(PlatformBuildClassObject):
>
>     def GenerateSizeFunction(self, Pcd):
>         CApp = "// Default Value in Dec \n"
>         CApp = CApp + "void Cal_%s_%s_Size(UINT32 *Size){\n" %
>(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)
>         if Pcd.IsArray():
>-            if (len(Pcd.Capacity) == 1 and Pcd.Capacity[0] != '0') or
>(len(Pcd.Capacity) >1 and reduce(lambda x,y:int(x)*int(y), Pcd.Capacity)) > 0:
>-                CApp += "  *Size = (sizeof (%s) * (%s) > *Size) ? sizeof (%s) * (%s):
>*Size; \n" % (Pcd.BaseDatumType,
>"*".join(Pcd.Capacity),Pcd.BaseDatumType, "*".join(Pcd.Capacity))
>-            if "{CODE(" in Pcd.DefaultValueFromDec:
>-                CApp += "  *Size = (sizeof (%s_%s_INIT_Value) > *Size ? sizeof
>(%s_%s_INIT_Value) : *Size);\n" %
>(Pcd.TokenSpaceGuidCName,Pcd.TokenCName,Pcd.TokenSpaceGuidCName,
>Pcd.TokenCName)
>-            for skuname in Pcd.SkuInfoList:
>-                skuobj = Pcd.SkuInfoList[skuname]
>-                if skuobj.VariableName:
>-                    for defaultstore in skuobj.DefaultStoreDict:
>-                        CApp += "  *Size = (sizeof (%s_%s_%s_%s_Value) > *Size ? sizeof
>(%s_%s_%s_%s_Value) : *Size);\n" %
>(Pcd.TokenSpaceGuidCName,Pcd.TokenCName,skuname,defaultstore,Pcd.T
>okenSpaceGuidCName,Pcd.TokenCName,skuname,defaultstore)
>-                else:
>-                    CApp += "  *Size = (sizeof (%s_%s_%s_%s_Value) > *Size ? sizeof
>(%s_%s_%s_%s_Value) : *Size);\n" %
>(Pcd.TokenSpaceGuidCName,Pcd.TokenCName,skuname,TAB_DEFAULT_STO
>RES_DEFAULT,Pcd.TokenSpaceGuidCName,Pcd.TokenCName,skuname,TAB_
>DEFAULT_STORES_DEFAULT)
>+            if Pcd.Type in PCD_DYNAMIC_TYPE_SET |
>PCD_DYNAMIC_EX_TYPE_SET:
>+                for skuname in Pcd.SkuInfoList:
>+                    skuobj = Pcd.SkuInfoList[skuname]
>+                    if skuobj.VariableName:
>+                        for defaultstore in skuobj.DefaultStoreDict:
>+                            pcddef =
>self.GetPcdDscRawDefaultValue(Pcd,skuname,defaultstore)
>+                            if pcddef and "{CODE(" in pcddef:
>+                                CApp += "  *Size = (sizeof (%s_%s_%s_%s_Value) > *Size ?
>sizeof (%s_%s_%s_%s_Value) : *Size);\n" %
>(Pcd.TokenSpaceGuidCName,Pcd.TokenCName,skuname,defaultstore,Pcd.T
>okenSpaceGuidCName,Pcd.TokenCName,skuname,defaultstore)
>+                    else:
>+                        pcddef =
>self.GetPcdDscRawDefaultValue(Pcd,skuname,TAB_DEFAULT_STORES_DEFA
>ULT)
>+                        if pcddef and "{CODE(" in pcddef:
>+                            CApp += "  *Size = (sizeof (%s_%s_%s_%s_Value) > *Size ?
>sizeof (%s_%s_%s_%s_Value) : *Size);\n" %
>(Pcd.TokenSpaceGuidCName,Pcd.TokenCName,skuname,TAB_DEFAULT_STO
>RES_DEFAULT,Pcd.TokenSpaceGuidCName,Pcd.TokenCName,skuname,TAB_
>DEFAULT_STORES_DEFAULT)
>+            else:
>+                pcddef =
>self.GetPcdDscRawDefaultValue(Pcd,TAB_DEFAULT,TAB_DEFAULT_STORES_
>DEFAULT)
>+                if pcddef and "{CODE(" in pcddef:
>+                    CApp += "  *Size = (sizeof (%s_%s_%s_%s_Value) > *Size ? sizeof
>(%s_%s_%s_%s_Value) : *Size);\n" %
>(Pcd.TokenSpaceGuidCName,Pcd.TokenCName,TAB_DEFAULT,TAB_DEFAULT
>_STORES_DEFAULT,Pcd.TokenSpaceGuidCName,Pcd.TokenCName,TAB_DEFA
>ULT,TAB_DEFAULT_STORES_DEFAULT)
>         for index in Pcd.DefaultValues:
>             FieldList = Pcd.DefaultValues[index]
>             if not FieldList:
>                 continue
>             for FieldName in FieldList:
>@@ -1860,20 +1865,40 @@ class DscBuildData(PlatformBuildClassObject):
>             CApp = CApp + "  *Size = (%d > *Size ? %d : *Size); // The Pcd maxsize
>is %d \n" % (Pcd.GetPcdMaxSize(), Pcd.GetPcdMaxSize(),
>Pcd.GetPcdMaxSize())
>         CApp = CApp + "}\n"
>         return CApp
>
>     @staticmethod
>-    def GenerateSizeStatments(Pcd):
>+    def GenerateSizeStatments(Pcd,skuname,defaultstorename):
>         if Pcd.IsArray():
>             r_datatype = [Pcd.BaseDatumType]
>+            lastoneisEmpty = False
>             for dem in Pcd.Capacity:
>-                if dem == '0':
>+                if lastoneisEmpty:
>+                    EdkLogger.error('Build', FORMAT_INVALID, "Invalid value format
>for %s.  " %
>+                                        (".".join((Pcd.TokenSpaceGuidCName,
>Pcd.TokenCName))))
>+                if dem == '0' or dem == "-1":
>                     r_datatype.append("[1]")
>+                    lastoneisEmpty = True
>                 else:
>                     r_datatype.append("[" + dem + "]")
>-            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)  )
>+
>+            if Pcd.Type in [MODEL_PCD_DYNAMIC_EX_HII,
>MODEL_PCD_DYNAMIC_HII]:
>+                PcdDefValue =
>Pcd.SkuInfoList.get(skuname).DefaultStoreDict.get(defaultstorename)
>+            elif Pcd.Type in
>[MODEL_PCD_DYNAMIC_EX_DEFAULT,MODEL_PCD_DYNAMIC_VPD,MODEL
>_PCD_DYNAMIC_DEFAULT,MODEL_PCD_DYNAMIC_EX_VPD]:
>+                PcdDefValue = Pcd.SkuInfoList.get(skuname).DefaultValue
>+            else:
>+                PcdDefValue = Pcd.DefaultValue
>+            if lastoneisEmpty:
>+                if "{CODE(" not in PcdDefValue:
>+                    sizebasevalue_plus = "(%s / sizeof(%s) + 1)" %
>((DscBuildData.GetStructurePcdMaxSize(Pcd), "".join(r_datatype)))
>+                    sizebasevalue = "(%s / sizeof(%s))" %
>((DscBuildData.GetStructurePcdMaxSize(Pcd), "".join(r_datatype)))
>+                    sizeof = "sizeof(%s)" % Pcd.BaseDatumType
>+                    CApp = '  Size = %s %% %s ? %s : %s ;\n' %
>( (DscBuildData.GetStructurePcdMaxSize(Pcd), sizeof, sizebasevalue_plus,
>sizebasevalue))
>+                else:
>+                    CApp = "  Size = 0;\n"
>+            else:
>+                CApp = '  Size = sizeof(%s);\n' % ("".join(r_datatype) )
>         else:
>             CApp = '  Size = sizeof(%s);\n' % (Pcd.DatumType)
>         CApp = CApp + '  Cal_%s_%s_Size(&Size);\n' %
>(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)
>         return CApp
>
>@@ -1983,26 +2008,30 @@ class DscBuildData(PlatformBuildClassObject):
>     @staticmethod
>     def GenerateDefaultValueAssignStatement(Pcd):
>         CApp = '  Assign_%s_%s_Default_Value(Pcd);\n' %
>(Pcd.TokenSpaceGuidCName, Pcd.TokenCName)
>         return CApp
>
>+    def GetPcdDscRawDefaultValue(self,Pcd, SkuName,DefaultStoreName):
>+        if Pcd.Type in PCD_DYNAMIC_TYPE_SET or Pcd.Type in
>PCD_DYNAMIC_EX_TYPE_SET:
>+            if (SkuName, DefaultStoreName) == (TAB_DEFAULT,
>TAB_DEFAULT_STORES_DEFAULT):
>+                pcddefaultvalue = Pcd.DefaultFromDSC.get(TAB_DEFAULT,
>{}).get(TAB_DEFAULT_STORES_DEFAULT) if Pcd.DefaultFromDSC else None
>+            else:
>+                pcddefaultvalue = Pcd.DscRawValue.get(SkuName,
>{}).get(DefaultStoreName)
>+        else:
>+            pcddefaultvalue = Pcd.DscRawValue.get(SkuName,
>{}).get(TAB_DEFAULT_STORES_DEFAULT)
>+
>+        return pcddefaultvalue
>     def GenerateInitValueFunction(self, Pcd, SkuName, DefaultStoreName):
>         CApp = "// Value in Dsc for Sku: %s, DefaultStore %s\n" % (SkuName,
>DefaultStoreName)
>         CApp = CApp + "void Assign_%s_%s_%s_%s_Value(%s *Pcd){\n" %
>(Pcd.TokenSpaceGuidCName, Pcd.TokenCName, SkuName,
>DefaultStoreName, Pcd.BaseDatumType)
>         CApp = CApp + '  UINT32  FieldSize;\n'
>         CApp = CApp + '  CHAR8   *Value;\n'
>
>         CApp = CApp + "// SkuName: %s,  DefaultStoreName: %s \n" %
>(TAB_DEFAULT, TAB_DEFAULT_STORES_DEFAULT)
>         inherit_OverrideValues = Pcd.SkuOverrideValues[SkuName]
>-        if Pcd.Type in PCD_DYNAMIC_TYPE_SET or Pcd.Type in
>PCD_DYNAMIC_EX_TYPE_SET:
>-            if (SkuName, DefaultStoreName) == (TAB_DEFAULT,
>TAB_DEFAULT_STORES_DEFAULT):
>-                pcddefaultvalue = Pcd.DefaultFromDSC.get(TAB_DEFAULT,
>{}).get(TAB_DEFAULT_STORES_DEFAULT) if Pcd.DefaultFromDSC else None
>-            else:
>-                pcddefaultvalue = Pcd.DscRawValue.get(SkuName,
>{}).get(DefaultStoreName)
>-        else:
>-            pcddefaultvalue = Pcd.DscRawValue.get(SkuName,
>{}).get(TAB_DEFAULT_STORES_DEFAULT)
>
>+        pcddefaultvalue = self.GetPcdDscRawDefaultValue(Pcd, SkuName,
>DefaultStoreName)
>         if pcddefaultvalue:
>             FieldList = pcddefaultvalue
>             IsArray = IsFieldValueAnArray(FieldList)
>             if IsArray:
>                 if "{CODE(" not in FieldList:
>@@ -2021,11 +2050,11 @@ class DscBuildData(PlatformBuildClassObject):
>                         CApp = CApp + '  Pcd = %s; // From DSC Default Value %s\n' %
>(Value, Pcd.DefaultFromDSC.get(TAB_DEFAULT,
>{}).get(TAB_DEFAULT_STORES_DEFAULT, Pcd.DefaultValue) if
>Pcd.DefaultFromDSC else Pcd.DefaultValue)
>                 elif IsArray:
>                 #
>                 # Use memcpy() to copy value into field
>                 #
>-                    if Pcd.IsArray():
>+                    if Pcd.IsArray() and "{CODE(" in pcddefaultvalue:
>                         CApp = CApp + '  memcpy (Pcd, %s_%s_%s_%s_Value,
>sizeof(%s_%s_%s_%s_Value));\n' % (Pcd.TokenSpaceGuidCName,
>Pcd.TokenCName,SkuName, DefaultStoreName,Pcd.TokenSpaceGuidCName,
>Pcd.TokenCName,SkuName, DefaultStoreName)
>                     else:
>                         CApp = CApp + '  Value     = %s; // From DSC Default Value %s\n' %
>(DscBuildData.IntToCString(Value, ValueSize),
>Pcd.DefaultFromDSC.get(TAB_DEFAULT,
>{}).get(TAB_DEFAULT_STORES_DEFAULT, Pcd.DefaultValue) if
>Pcd.DefaultFromDSC else Pcd.DefaultValue)
>                         CApp = CApp + '  memcpy (Pcd, Value, %d);\n' % (ValueSize)
>             else:
>@@ -2036,11 +2065,11 @@ class DscBuildData(PlatformBuildClassObject):
>                         CApp = CApp + '  Pcd = %s; // From DSC Default Value %s\n' %
>(Value, Pcd.DscRawValue.get(SkuName, {}).get(DefaultStoreName))
>                 elif IsArray:
>                 #
>                 # Use memcpy() to copy value into field
>                 #
>-                    if Pcd.IsArray():
>+                    if Pcd.IsArray() and "{CODE(" in pcddefaultvalue:
>                         CApp = CApp + '  memcpy (Pcd, %s_%s_%s_%s_Value,
>sizeof(%s_%s_%s_%s_Value));\n' % (Pcd.TokenSpaceGuidCName,
>Pcd.TokenCName,SkuName, DefaultStoreName,Pcd.TokenSpaceGuidCName,
>Pcd.TokenCName,SkuName, DefaultStoreName)
>                     else:
>                         CApp = CApp + '  Value     = %s; // From DSC Default Value %s\n' %
>(DscBuildData.IntToCString(Value, ValueSize),
>Pcd.DscRawValue.get(SkuName, {}).get(DefaultStoreName))
>                         CApp = CApp + '  memcpy (Pcd, Value, %d);\n' % (ValueSize)
>
>@@ -2266,11 +2295,11 @@ class DscBuildData(PlatformBuildClassObject):
>             # array member is detected, and the size is based on the highest index
>used with
>             # the flexible array member.  The flexible array member must be the
>last field
>             # in a structure.  The size formula for this case is:
>             # OFFSET_OF(FlexbleArrayField) + sizeof(FlexibleArray[0]) *
>(HighestIndex + 1)
>             #
>-            CApp = CApp + DscBuildData.GenerateSizeStatments(Pcd)
>+            CApp = CApp +
>DscBuildData.GenerateSizeStatments(Pcd,SkuName,DefaultStoreName)
>
>             #
>             # Allocate and zero buffer for the PCD
>             # Must handle cases where current value is smaller, larger, or same size
>             # Always keep that larger one as the current size
>@@ -2326,39 +2355,39 @@ class DscBuildData(PlatformBuildClassObject):
>             Demesion += "[]"
>
>         Value = Pcd.DefaultValueFromDec
>         if "{CODE(" in Pcd.DefaultValueFromDec:
>             realvalue = Pcd.DefaultValueFromDec.strip()[6:-2] #
>"{CODE(").rstrip(")}"
>-        else:
>-            realvalue = Pcd.DefaultValueFromDec.strip()
>-        CApp += "static %s %s_%s_INIT_Value%s = %s;\n" %
>(Pcd.BaseDatumType,Pcd.TokenSpaceGuidCName,Pcd.TokenCName,Demesi
>on,realvalue)
>+            CApp += "static %s %s_%s_INIT_Value%s = %s;\n" %
>(Pcd.BaseDatumType,Pcd.TokenSpaceGuidCName,Pcd.TokenCName,Demesi
>on,realvalue)
>
>         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]
>+                        pcddscrawdefaultvalue = self.GetPcdDscRawDefaultValue(Pcd,
>skuname, defaultstore)
>+                        if pcddscrawdefaultvalue:
>+                            Value = skuinfo[defaultstore]
>+                            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:
>+                    pcddscrawdefaultvalue = self.GetPcdDscRawDefaultValue(Pcd,
>skuname, TAB_DEFAULT_STORES_DEFAULT)
>+                    if pcddscrawdefaultvalue:
>+                        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,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)
>+                            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)
>+            pcddscrawdefaultvalue = self.GetPcdDscRawDefaultValue(Pcd,
>TAB_DEFAULT, TAB_DEFAULT_STORES_DEFAULT)
>+            if pcddscrawdefaultvalue:
>+                if "{CODE(" in Pcd.DefaultValue:
>+                    realvalue = Pcd.DefaultValue.strip()[6:-2] # "{CODE(").rstrip(")}"
>+                    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:
>             if OverrideValues[key]:
>--
>2.19.1.windows.1
>
>_______________________________________________
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel


      reply	other threads:[~2019-01-04  1:18 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-29  8:43 [Patch] BaseTools: Correct PcdArray value assigment statement BobCF
2019-01-04  1:18 ` 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=4A89E2EF3DFEDB4C8BFDE51014F606A14E3A7F1E@SHSMSX152.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