Hi Hao, I have below comments:
Case 1: In order to keep consistence, can you also change other cases which still initialized with '\0' mOptions.VfrFileName[0] = '\0'; mOptions.RecordListFile = NULL; mOptions.CreateRecordListFile = FALSE; mOptions.CreateIfrPkgFile = FALSE; mOptions.PkgOutputFileName = NULL; mOptions.COutputFileName = NULL; mOptions.OutputDirectory[0] = '\0'; mOptions.PreprocessorOutputFileName = NULL; mOptions.VfrBaseFileName[0] = '\0'; Case 2: for below case, you can use the actual size for the strncpy instead of MAX_PATH - 1. Also the last line code is not needed. if (strlen (Argv[Index]) > MAX_PATH - 1) { DebugError (NULL, 0, 1003, "Invalid option value", "Output directory name %s is too long", Argv[Index]); goto Fail; } strncpy (mOptions.OutputDirectory, Argv[Index], MAX_PATH - 1); mOptions.OutputDirectory[MAX_PATH - 1] = 0; Thanks, Eric > -----Original Message----- > From: Wu, Hao A > Sent: Wednesday, October 12, 2016 8:21 PM > To: edk2-devel@lists.01.org > Cc: Wu, Hao A; Gao, Liming; Zhu, Yonghong; Dong, Eric; Bi, Dandan > Subject: [PATCH 52/52] BaseTools/VfrCompile/Pccts: Make assignment operator > not returning void > > The assignment operators for class ANTLRTokenPtr return void in current > code. > > This commit makes them return the reference to the object just like > primitive types do. > > Cc: Liming Gao <liming....@intel.com> > Cc: Yonghong Zhu <yonghong....@intel.com> > Cc: Eric Dong <eric.d...@intel.com> > Cc: Dandan Bi <dandan...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Hao Wu <hao.a...@intel.com> > --- > BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h | 4 ++-- > BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h | 6 ++++-- > 2 files changed, 6 insertions(+), 4 deletions(-) > > diff --git a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h > b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h > index 75b4c86..df89d8f 100644 > --- a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h > +++ b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtr.h > @@ -58,8 +58,8 @@ public: > // 7-Apr-97 133MR1 > // Fix suggested by Andreas Magnusson > // (andreas.magnus...@mailbox.swipnet.se) > - void operator = (const ANTLRTokenPtr & lhs); // MR1 > - void operator = (ANTLRAbstractToken *addr); > + ANTLRTokenPtr& operator = (const ANTLRTokenPtr & lhs); // MR1 > + ANTLRTokenPtr& operator = (ANTLRAbstractToken *addr); > int operator != (const ANTLRTokenPtr &q) const // MR1 > // MR11 unsigned -> int > { return this->ptr_ != q.ptr_; } > int operator == (const ANTLRTokenPtr &q) const // MR1 > // MR11 unsigned -> int > diff --git a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h > b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h > index 9c07cf5..a1efc3b 100644 > --- a/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h > +++ b/BaseTools/Source/C/VfrCompile/Pccts/h/ATokPtrImpl.h > @@ -71,18 +71,20 @@ ANTLRTokenPtr::~ANTLRTokenPtr() > // 8-Apr-97 MR1 Make operator -> a const member function > // as weall as some other member functions > // > -void ANTLRTokenPtr::operator = (const ANTLRTokenPtr & lhs) // MR1 > +ANTLRTokenPtr& ANTLRTokenPtr::operator = (const ANTLRTokenPtr & lhs) // > MR1 > { > lhs.ref(); // protect against "xp = xp"; ie same underlying object > deref(); > ptr_ = lhs.ptr_; > + return *this; > } > > -void ANTLRTokenPtr::operator = (ANTLRAbstractToken *addr) > +ANTLRTokenPtr& ANTLRTokenPtr::operator = (ANTLRAbstractToken *addr) > { > if (addr != NULL) { > addr->ref(); > } > deref(); > ptr_ = addr; > + return *this; > } > -- > 1.9.5.msysgit.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel