Re: [edk2] [RFC v2 1/2] BaseTool/VfrCompile: Support Union type in VFR

2017-06-05 Thread Bi, Dandan 
Hi Ray,

Thanks for your comments! Yes, we also can avoid using member variable.

Based on my understanding, we can parse value to sub-statement.
So when parsing a Struct/Union type, it  can pass this info to its member 
fields , then we can detect whether current Data Type is Struct or Union.
In this case, we need to update all the definitions of the dataStructField and 
related functions.

If we add member variable/ global variable, we can keep the definition of the 
dataStructField same as before, no need to consider whether  they are members
Of a Struct or Union in the parsing  part. When calculating the TotalSize of 
current  Data Type , we can know the Data Type according to the member 
variable/ global variable.

I am ok to update the codes to avoid adding new member variable and will send 
new patches.

Liming/Eric, do you have any comments for this ?


Thanks,
Dandan

-Original Message-
From: Ni, Ruiyu 
Sent: Monday, June 5, 2017 2:49 PM
To: Bi, Dandan ; edk2-devel@lists.01.org
Cc: Dong, Eric ; Gao, Liming 
Subject: RE: [RFC v2 1/2] BaseTool/VfrCompile: Support Union type in VFR

Dandan,
Is it possible to avoid adding IsUnion member variable?
I remember .G syntax supports passing value to sub-statement.
In this case, can you use: vfrDataStructFields [IsUnion]?

Thanks/Ray

> -Original Message-
> From: Bi, Dandan
> Sent: Monday, June 5, 2017 12:31 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric ; Gao, Liming 
> ; Ni, Ruiyu 
> Subject: [RFC v2 1/2] BaseTool/VfrCompile: Support Union type in VFR
> 
> V2: Update VfrCompiler to use member variable instead of global 
> varable to indicate whether current date type is Union.
> 
> Cc: Eric Dong 
> Cc: Liming Gao 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi 
> ---
>  BaseTools/Source/C/VfrCompile/VfrSyntax.g   | 19 ++-
>  BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp | 16 ++--
>  BaseTools/Source/C/VfrCompile/VfrUtilityLib.h   |  3 ++-
>  3 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> index 406dbc5..9e1212a 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> +++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> @@ -155,10 +155,11 @@ VfrParserStart (
>  #token Label("label")   "label"
>  #token Timeout("timeout")   "timeout"
>  #token Inventory("inventory")   "inventory"
>  #token NonNvDataMap("_NON_NV_DATA_MAP") "_NON_NV_DATA_MAP"
>  #token Struct("struct") "struct"
> +#token Union("union")   "union"
>  #token Boolean("BOOLEAN")   "BOOLEAN"
>  #token Uint64("UINT64") "UINT64"
>  #token Uint32("UINT32") "UINT32"
>  #token Uint16("UINT16") "UINT16"
>  #token Char16("CHAR16") "CHAR16"
> @@ -270,10 +271,11 @@ vfrProgram > [UINT8 Return] :
>   mConstantOnlyInExpression = FALSE;
>>>
>(
>vfrPragmaPackDefinition
>  | vfrDataStructDefinition
> +| vfrDataUnionDefinition
>)*
>vfrFormSetDefinition
><< $Return = mParserStatus; >>
>;
> 
> @@ -318,12 +320,27 @@ vfrPragmaPackDefinition :
>  | pragmaPackNumber
>}
>"\)"
>;
> 
> +  vfrDataUnionDefinition :
> +  { TypeDef } Union<<
> gCVfrVarDataTypeDB.DeclareDataTypeBegin (TRUE); >>
> +  { NonNvDataMap }
> +  {
> +N1:StringIdentifier <<
> _PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
> +  }
> +  OpenBrace
> +vfrDataStructFields
> +  CloseBrace
> +  {
> +N2:StringIdentifier <<
> _PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N2->getText()), N2); >>
> +  }
> +  ";"   << 
> gCVfrVarDataTypeDB.DeclareDataTypeEnd
> ();>>
> +  ;
> +
>  vfrDataStructDefinition :
> -  { TypeDef } Struct<<
> gCVfrVarDataTypeDB.DeclareDataTypeBegin (); >>
> +  { TypeDef } Struct<<
> gCVfrVarDataTypeDB.DeclareDataTypeBegin (FALSE); >>
>{ NonNvDataMap }
>{
>  N1:StringIdentifier <<
> _PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
>}
>OpenBrace
> diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> index 2f97975..186b9c9 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> +++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> @@ -776,10 +776,11 @@

Re: [edk2] [RFC v2 1/2] BaseTool/VfrCompile: Support Union type in VFR

2017-06-05 Thread Ni, Ruiyu 
Dandan,
Is it possible to avoid adding IsUnion member variable?
I remember .G syntax supports passing value to sub-statement.
In this case, can you use: vfrDataStructFields [IsUnion]?

Thanks/Ray

> -Original Message-
> From: Bi, Dandan
> Sent: Monday, June 5, 2017 12:31 PM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric ; Gao, Liming ; Ni,
> Ruiyu 
> Subject: [RFC v2 1/2] BaseTool/VfrCompile: Support Union type in VFR
> 
> V2: Update VfrCompiler to use member variable instead of global varable to
> indicate whether current date type is Union.
> 
> Cc: Eric Dong 
> Cc: Liming Gao 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi 
> ---
>  BaseTools/Source/C/VfrCompile/VfrSyntax.g   | 19 ++-
>  BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp | 16 ++--
>  BaseTools/Source/C/VfrCompile/VfrUtilityLib.h   |  3 ++-
>  3 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> index 406dbc5..9e1212a 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> +++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> @@ -155,10 +155,11 @@ VfrParserStart (
>  #token Label("label")   "label"
>  #token Timeout("timeout")   "timeout"
>  #token Inventory("inventory")   "inventory"
>  #token NonNvDataMap("_NON_NV_DATA_MAP") "_NON_NV_DATA_MAP"
>  #token Struct("struct") "struct"
> +#token Union("union")   "union"
>  #token Boolean("BOOLEAN")   "BOOLEAN"
>  #token Uint64("UINT64") "UINT64"
>  #token Uint32("UINT32") "UINT32"
>  #token Uint16("UINT16") "UINT16"
>  #token Char16("CHAR16") "CHAR16"
> @@ -270,10 +271,11 @@ vfrProgram > [UINT8 Return] :
>   mConstantOnlyInExpression = FALSE;
>>>
>(
>vfrPragmaPackDefinition
>  | vfrDataStructDefinition
> +| vfrDataUnionDefinition
>)*
>vfrFormSetDefinition
><< $Return = mParserStatus; >>
>;
> 
> @@ -318,12 +320,27 @@ vfrPragmaPackDefinition :
>  | pragmaPackNumber
>}
>"\)"
>;
> 
> +  vfrDataUnionDefinition :
> +  { TypeDef } Union<<
> gCVfrVarDataTypeDB.DeclareDataTypeBegin (TRUE); >>
> +  { NonNvDataMap }
> +  {
> +N1:StringIdentifier <<
> _PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
> +  }
> +  OpenBrace
> +vfrDataStructFields
> +  CloseBrace
> +  {
> +N2:StringIdentifier <<
> _PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N2->getText()), N2); >>
> +  }
> +  ";"   << 
> gCVfrVarDataTypeDB.DeclareDataTypeEnd
> ();>>
> +  ;
> +
>  vfrDataStructDefinition :
> -  { TypeDef } Struct<<
> gCVfrVarDataTypeDB.DeclareDataTypeBegin (); >>
> +  { TypeDef } Struct<<
> gCVfrVarDataTypeDB.DeclareDataTypeBegin (FALSE); >>
>{ NonNvDataMap }
>{
>  N1:StringIdentifier <<
> _PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
>}
>OpenBrace
> diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> index 2f97975..186b9c9 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> +++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
> @@ -776,10 +776,11 @@ CVfrVarDataTypeDB::InternalTypesListInit (
>  if (New != NULL) {
>strcpy (New->mTypeName, gInternalTypesTable[Index].mTypeName);
>New->mType= gInternalTypesTable[Index].mType;
>New->mAlign   = gInternalTypesTable[Index].mAlign;
>New->mTotalSize   = gInternalTypesTable[Index].mSize;
> +  New->mIsUnionType = FALSE;
>if (strcmp (gInternalTypesTable[Index].mTypeName, "EFI_HII_DATE") == 
> 0) {
>  SVfrDataField *pYearField  = new SVfrDataField;
>  SVfrDataField *pMonthField = new SVfrDataField;
>  SVfrDataField *pDayField   = new SVfrDataField;
> 
> @@ -964,11 +965,11 @@ CVfrVarDataTypeDB::Pack (
>return VFR_RETURN_SUCCESS;
>  }
> 
>  VOID
>  CVfrVarDataTypeDB::DeclareDataTypeBegin (
> -  VOID
> +  BOOLEAN  IsUnionType
>)
>  {
>SVfrDataType *pNewType = NULL;
> 
>pNewType   = new SVfrDataType;
> @@ -976,10 +977,11 @@ CVfrVarDataTypeDB::DeclareDataTypeBegin (
>pNewType->mType= EFI_IFR_TYPE_OTHER;
>pNewType->mAlign   = DEFAULT_ALIGN;
>pNewType->mTotalSize   = 0;
>pNewType->mMembers = NULL;
>pNewType->mNext= NULL;
> +  pNewType->mIsUnionType =

[edk2] [RFC v2 1/2] BaseTool/VfrCompile: Support Union type in VFR

2017-06-04 Thread Dandan Bi 
V2: Update VfrCompiler to use member variable instead of global varable
to indicate whether current date type is Union.

Cc: Eric Dong 
Cc: Liming Gao 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Dandan Bi 
---
 BaseTools/Source/C/VfrCompile/VfrSyntax.g   | 19 ++-
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp | 16 ++--
 BaseTools/Source/C/VfrCompile/VfrUtilityLib.h   |  3 ++-
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g 
b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
index 406dbc5..9e1212a 100644
--- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
+++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
@@ -155,10 +155,11 @@ VfrParserStart (
 #token Label("label")   "label"
 #token Timeout("timeout")   "timeout"
 #token Inventory("inventory")   "inventory"
 #token NonNvDataMap("_NON_NV_DATA_MAP") "_NON_NV_DATA_MAP"
 #token Struct("struct") "struct"
+#token Union("union")   "union"
 #token Boolean("BOOLEAN")   "BOOLEAN"
 #token Uint64("UINT64") "UINT64"
 #token Uint32("UINT32") "UINT32"
 #token Uint16("UINT16") "UINT16"
 #token Char16("CHAR16") "CHAR16"
@@ -270,10 +271,11 @@ vfrProgram > [UINT8 Return] :
  mConstantOnlyInExpression = FALSE;
   >>
   (
   vfrPragmaPackDefinition
 | vfrDataStructDefinition
+| vfrDataUnionDefinition
   )*
   vfrFormSetDefinition
   << $Return = mParserStatus; >>
   ;
 
@@ -318,12 +320,27 @@ vfrPragmaPackDefinition :
 | pragmaPackNumber
   }
   "\)"
   ;
 
+  vfrDataUnionDefinition :
+  { TypeDef } Union<< 
gCVfrVarDataTypeDB.DeclareDataTypeBegin (TRUE); >>
+  { NonNvDataMap }
+  {
+N1:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
+  }
+  OpenBrace
+vfrDataStructFields
+  CloseBrace
+  {
+N2:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N2->getText()), N2); >>
+  }
+  ";"   << 
gCVfrVarDataTypeDB.DeclareDataTypeEnd ();>>
+  ;
+
 vfrDataStructDefinition :
-  { TypeDef } Struct<< 
gCVfrVarDataTypeDB.DeclareDataTypeBegin (); >>
+  { TypeDef } Struct<< 
gCVfrVarDataTypeDB.DeclareDataTypeBegin (FALSE); >>
   { NonNvDataMap }
   {
 N1:StringIdentifier << 
_PCATCH(gCVfrVarDataTypeDB.SetNewTypeName (N1->getText()), N1); >>
   }
   OpenBrace
diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp 
b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
index 2f97975..186b9c9 100644
--- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
+++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.cpp
@@ -776,10 +776,11 @@ CVfrVarDataTypeDB::InternalTypesListInit (
 if (New != NULL) {
   strcpy (New->mTypeName, gInternalTypesTable[Index].mTypeName);
   New->mType= gInternalTypesTable[Index].mType;
   New->mAlign   = gInternalTypesTable[Index].mAlign;
   New->mTotalSize   = gInternalTypesTable[Index].mSize;
+  New->mIsUnionType = FALSE;
   if (strcmp (gInternalTypesTable[Index].mTypeName, "EFI_HII_DATE") == 0) {
 SVfrDataField *pYearField  = new SVfrDataField;
 SVfrDataField *pMonthField = new SVfrDataField;
 SVfrDataField *pDayField   = new SVfrDataField;
 
@@ -964,11 +965,11 @@ CVfrVarDataTypeDB::Pack (
   return VFR_RETURN_SUCCESS;
 }
 
 VOID
 CVfrVarDataTypeDB::DeclareDataTypeBegin (
-  VOID
+  BOOLEAN  IsUnionType
   )
 {
   SVfrDataType *pNewType = NULL;
 
   pNewType   = new SVfrDataType;
@@ -976,10 +977,11 @@ CVfrVarDataTypeDB::DeclareDataTypeBegin (
   pNewType->mType= EFI_IFR_TYPE_OTHER;
   pNewType->mAlign   = DEFAULT_ALIGN;
   pNewType->mTotalSize   = 0;
   pNewType->mMembers = NULL;
   pNewType->mNext= NULL;
+  pNewType->mIsUnionType = IsUnionType;
 
   mNewDataType   = pNewType;
 }
 
 EFI_VFR_RETURN_CODE
@@ -1018,12 +1020,14 @@ CVfrVarDataTypeDB::DataTypeAddField (
 {
   SVfrDataField   *pNewField  = NULL;
   SVfrDataType*pFieldType = NULL;
   SVfrDataField   *pTmp;
   UINT32  Align;
+  UINT32  MaxDataTypeSize;
 
   CHECK_ERROR_RETURN (GetDataType (TypeName, ), VFR_RETURN_SUCCESS);
+  MaxDataTypeSize = mNewDataType->mTotalSize;
 
   if (strlen (FieldName) >= MAX_NAME_LEN) {
return VFR_RETURN_INVALID_PARAMETER;
   }
 
@@ -1055,11 +1059,19 @@ CVfrVarDataTypeDB::DataTypeAddField (
 pTmp->mNext= pNewField;
 pNewField->mNext   = NULL;
   }
 
   mNewDataType->mAlign = MIN