Re: [patch v2] BaseTools/VfrCompile: Avoid using uninitialized pointer

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

Dandan,
Can you describe the build failure met by Gary Lin in final commit message?
The current commit message is not very clear about which issue it may fix.

Thanks/Ray

> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Gary Lin
> Sent: Wednesday, May 9, 2018 4:20 PM
> To: Bi, Dandan <dandan.bi@intel.com>
> Cc: edk2-devel@lists.01.org; Dong, Eric <eric.dong@intel.com>; Gao, Liming
> <liming.gao@intel.com>
> Subject: Re: [edk2] [patch v2] BaseTools/VfrCompile: Avoid using
> uninitialized pointer
> 
> On Wed, May 09, 2018 at 01:02:11PM +0800, Dandan Bi wrote:
> > V2:
> > Add function _INIT_OPHDR_COND () for variable initialization.
> > Make code logic more clean.
> >
> > Previously _CLEAR_SAVED_OPHDR () is used for variable initialization,
> > and we updated it to clean memory.
> > But _CLEAR_SAVED_OPHDR () is still called for variable initialization.
> > This will cause uninitialized pointer will be checked to free and
> > cause unexpected issue.
> >
> > This patch is to add new function for variable initialization and keep
> > _CLEAR_SAVED_OPHDR () to clean memory which is aligned with its
> > function name.
> >
> This patch fixes the build errors I had :)
> 
> Tested-by: Gary Lin <glin@suse.com>
> 
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Gary Lin <glin@suse.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> > ---
> >  BaseTools/Source/C/VfrCompile/VfrSyntax.g | 23
> > ++++++++++++++++++++---
> >  1 file changed, 20 insertions(+), 3 deletions(-)
> >
> > diff --git a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> > b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> > index 4b0a43606ea..84dd2c3ed3f 100644
> > --- a/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> > +++ b/BaseTools/Source/C/VfrCompile/VfrSyntax.g
> > @@ -4084,11 +4084,19 @@ vfrStatementInvalidSaveRestoreDefaults :
> >
> >  //
> >  // Root expression extension function called by other function.
> >  //
> >  vfrStatementExpression [UINT32 RootLevel, UINT32 ExpOpCount = 0] :
> > -  << if ($RootLevel == 0) {mCIfrOpHdrIndex ++; if (mCIfrOpHdrIndex >=
> > MAX_IFR_EXPRESSION_DEPTH) _PCATCH
> (VFR_RETURN_INVALID_PARAMETER, 0,
> > "The depth of expression exceeds the max supported level 8!");
> > _CLEAR_SAVED_OPHDR ();} >>
> > +                                                       <<
> > +                                                          if ($RootLevel == 0) {
> > +                                                            mCIfrOpHdrIndex ++;
> > +                                                            if (mCIfrOpHdrIndex >=
> MAX_IFR_EXPRESSION_DEPTH) {
> > +                                                              _PCATCH
> (VFR_RETURN_INVALID_PARAMETER, 0, "The depth of expression exceeds
> the max supported level 8!");
> > +                                                            }
> > +                                                            _INIT_OPHDR_COND ();
> > +                                                          }
> > +                                                       >>
> >    andTerm[$RootLevel, $ExpOpCount]
> >    (
> >      L:OR andTerm[$RootLevel, $ExpOpCount]              << $ExpOpCount++;
> CIfrOr OObj(L->getLine()); >>
> >    )*
> >                                                         << @@ -4988,10
> > +4996,11 @@ private:
> >    CIfrOpHeader *      mCIfrOpHdr[MAX_IFR_EXPRESSION_DEPTH];
> >    UINT32              mCIfrOpHdrLineNo[MAX_IFR_EXPRESSION_DEPTH];
> >    UINT8               mCIfrOpHdrIndex;
> >    VOID                _SAVE_OPHDR_COND (IN CIfrOpHeader &, IN BOOLEAN,
> UINT32 LineNo = 0);
> >    VOID                _CLEAR_SAVED_OPHDR (VOID);
> > +  VOID                _INIT_OPHDR_COND (VOID);
> >    BOOLEAN             _SET_SAVED_OPHDR_SCOPE (VOID);
> >
> >
> >    EFI_VARSTORE_INFO   mCurrQestVarInfo;
> >    EFI_GUID            *mOverrideClassGuid;
> > @@ -5077,20 +5086,28 @@ EfiVfrParser::_SAVE_OPHDR_COND (
> >      mCIfrOpHdr[mCIfrOpHdrIndex]       = new CIfrOpHeader(OpHdr);
> >      mCIfrOpHdrLineNo[mCIfrOpHdrIndex] = LineNo;
> >    }
> >  }
> >
> > +VOID
> > +EfiVfrParser::_INIT_OPHDR_COND (
> > +  VOID
> > +  )
> > +{
> > +  mCIfrOpHdr[mCIfrOpHdrIndex]       = NULL;
> > +  mCIfrOpHdrLineNo[mCIfrOpHdrIndex] = 0; }
> > +
> >  VOID
> >  EfiVfrParser::_CLEAR_SAVED_OPHDR (
> >    VOID
> >    )
> >  {
> >    if (mCIfrOpHdr[mCIfrOpHdrIndex] != NULL) {
> >      delete mCIfrOpHdr[mCIfrOpHdrIndex];
> > -    mCIfrOpHdr[mCIfrOpHdrIndex]     = NULL;
> > +    mCIfrOpHdr[mCIfrOpHdrIndex] = NULL;
> >    }
> > -  mCIfrOpHdrLineNo[mCIfrOpHdrIndex] = 0;  }
> >
> >  BOOLEAN
> >  EfiVfrParser::_SET_SAVED_OPHDR_SCOPE (
> >    VOID
> > --
> > 2.14.3.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> >
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel