Re: [PATCH] BaseTools: covert "unicode string" to "byte array" if value type diff.

["Carsey, Jaben" <jaben.carsey@intel.com>]

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Wednesday, October 17, 2018 11:37 AM
> To: Zhao, ZhiqiangX <zhiqiangx.zhao@intel.com>
> Cc: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [PATCH] BaseTools: covert "unicode string" to "byte
> array" if value type diff.
> 
> Hi,
> 
> On 10/17/18 13:08, Zhaozh1x wrote:
> > For the same one VOID* pcd, if the default value type of one sku is
> > "unicode string", the other skus are "OtherVOID*"(ascii string or
> > byte array),Then covert "unicode string" to "byte array".
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: ZhiqiangX Zhao <zhiqiangx.zhao@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Yonghong Zhu <yonghong.zhu@intel.com>
> > Reviewed-by: Bob Feng <bob.c.feng@intel.com>
> > ---
> >  BaseTools/Source/Python/Workspace/DscBuildData.py | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py
> b/BaseTools/Source/Python/Workspace/DscBuildData.py
> > index 7854e71db6..9b783de84b 100644
> > --- a/BaseTools/Source/Python/Workspace/DscBuildData.py
> > +++ b/BaseTools/Source/Python/Workspace/DscBuildData.py
> > @@ -2877,6 +2877,15 @@ class DscBuildData(PlatformBuildClassObject):
> >              elif TAB_DEFAULT in pcd.SkuInfoList and TAB_COMMON in
> pcd.SkuInfoList:
> >                  del pcd.SkuInfoList[TAB_COMMON]
> >
> > +        #For the same one VOID* pcd, if the default value type of one sku is
> "unicode string",
> > +        #the other skus are "OtherVOID*"(ascii string or byte array),Then
> covert "unicode string" to "byte array".
> > +        for pcd in Pcds.values():
> > +            PcdValueTypeList = []
> > +            for sku in pcd.SkuInfoList.values():
> > +                PcdValueTypeList.append("UnicodeString" if
> sku.DefaultValue.startswith("L") else "OtherVOID*")
> > +            if "UnicodeString" in PcdValueTypeList and "OtherVOID*" in
> PcdValueTypeList:

If you use a set instead of a list here, it will save memory any then instead of testing for both items being there, you could just check the length being > 1 which would also be faster than 2 "in" tests.

> > +                for sku in pcd.SkuInfoList.values():
> > +                    sku.DefaultValue = StringToArray(sku.DefaultValue) if
> sku.DefaultValue.startswith("L") else sku.DefaultValue
> >
> >          map(self.FilterSkuSettings, Pcds.values())
> >          return Pcds
> >
> 
> no comments on the code change, just a superficial one on the subject
> line: please replace "covert" with "convert".
> 
> There are two more instances of the typo in the patch: in the commit
> message, and in the new code too. (Three occurrences in total.)
> 
> Normally I shouldn't obsess about such small typos, in case they don't
> obscure the intended meaning. However, "covert" did confuse me, because
> it is an existent word, and it has a specific meaning in computing:
> 
> https://en.wikipedia.org/wiki/Covert_channel
> 
> Thanks!
> Laszlo
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel