public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Bi, Dandan" <dandan.bi@intel.com>
To: "Ni, Ruiyu" <ruiyu.ni@intel.com>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Dong, Eric" <eric.dong@intel.com>, "Gao, Liming" <liming.gao@intel.com>
Subject: Re: [RFC v2 1/2] BaseTool/VfrCompile: Support Union type in VFR
Date: Mon, 5 Jun 2017 08:52:12 +0000	[thread overview]
Message-ID: <3C0D5C461C9E904E8F62152F6274C0BB3B928BAE@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <734D49CCEBEEF84792F5B80ED585239D5B987242@SHSMSX104.ccr.corp.intel.com>

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 <dandan.bi@intel.com>; edk2-devel@lists.01.org
Cc: Dong, Eric <eric.dong@intel.com>; Gao, Liming <liming.gao@intel.com>
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 <eric.dong@intel.com>; Gao, Liming 
> <liming.gao@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> 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 <eric.dong@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  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, &pFieldType), 
> 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 (mPackAlign, MAX (pFieldType->mAlign,
> mNewDataType->mAlign));
> -  mNewDataType->mTotalSize = pNewField->mOffset + (pNewField-
> >mFieldType->mTotalSize) * ((ArrayNum == 0) ? 1 : ArrayNum);
> +
> +  if (mNewDataType->mIsUnionType) {
> +    if (MaxDataTypeSize < pNewField->mFieldType->mTotalSize) {
> +      mNewDataType->mTotalSize = pNewField->mFieldType->mTotalSize;
> +    }
> +    pNewField->mOffset = 0;
> +  } else {
> +    mNewDataType->mTotalSize = pNewField->mOffset +
> + (pNewField->mFieldType->mTotalSize) * ((ArrayNum == 0) ? 1 :
> + ArrayNum);  }
> 
>    return VFR_RETURN_SUCCESS;
>  }
> 
>  VOID
> diff --git a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
> b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
> index 59509c3..13b75e4 100644
> --- a/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
> +++ b/BaseTools/Source/C/VfrCompile/VfrUtilityLib.h
> @@ -124,10 +124,11 @@ struct SVfrDataType {
>    UINT8                     mType;
>    UINT32                    mAlign;
>    UINT32                    mTotalSize;
>    SVfrDataField             *mMembers;
>    SVfrDataType              *mNext;
> +  BOOLEAN                   mIsUnionType;
>  };
> 
>  #define VFR_PACK_ASSIGN     0x01
>  #define VFR_PACK_SHOW       0x02
>  #define VFR_PACK_PUSH       0x04
> @@ -201,11 +202,11 @@ private:
> 
>  public:
>    CVfrVarDataTypeDB (VOID);
>    ~CVfrVarDataTypeDB (VOID);
> 
> -  VOID                DeclareDataTypeBegin (VOID);
> +  VOID                DeclareDataTypeBegin (BOOLEAN);
>    EFI_VFR_RETURN_CODE SetNewTypeName (IN CHAR8 *);
>    EFI_VFR_RETURN_CODE DataTypeAddField (IN CHAR8 *, IN CHAR8 *, IN 
> UINT32);
>    VOID                DeclareDataTypeEnd (VOID);
> 
>    EFI_VFR_RETURN_CODE GetDataType (IN CHAR8 *, OUT SVfrDataType **);
> --
> 1.9.5.msysgit.1



  reply	other threads:[~2017-06-05  8:51 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-05  4:30 [RFC v2 0/2] Support Union type in VFR Dandan Bi
2017-06-05  4:30 ` [RFC v2 1/2] BaseTool/VfrCompile: " Dandan Bi
2017-06-05  6:49   ` Ni, Ruiyu
2017-06-05  8:52     ` Bi, Dandan [this message]
2017-06-05  4:30 ` [RFC v2 2/2] MdeModulePkg/DriverSample: Add sample questions to refer union type Dandan Bi

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=3C0D5C461C9E904E8F62152F6274C0BB3B928BAE@shsmsx102.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