Hi Shenglei, Looking at this patch more closely. There appear to be bugs... please see below. Please fix this along with your poor use of semi-colons and white-space.
Thanks, Nate On 9/12/2019 11:54 AM, Nate DeSimone wrote: > Your whitespace doesn't quite match the edk2 coding style guidelines, but we > can fix that during commit. > > Reviewed-by: Nate DeSimone <nathaniel.l.desim...@intel.com> > > -----Original Message----- > From: Zhang, Shenglei > Sent: Wednesday, September 11, 2019 7:55 PM > To: devel@edk2.groups.io > Cc: Kubacki, Michael A <michael.a.kuba...@intel.com>; Chiu, Chasel > <chasel.c...@intel.com>; Desimone, Nathaniel L > <nathaniel.l.desim...@intel.com>; Gao, Liming <liming....@intel.com> > Subject: [PATCH] MinPlatformPkg/TestPointCheckLib: Add check for pointers > > In DxeCheckBootVariable.c, add check for BootOrder and Variable that return > EFI_NOT_FOUND when they are NULL. > In DxeCheckGcd.c, add check for GcdIoMap to ensure it not NULL when > allocating memory to what it points to. > > Cc: Michael Kubacki <michael.a.kuba...@intel.com> > Cc: Chasel Chiu <chasel.c...@intel.com> > Cc: Nate DeSimone <nathaniel.l.desim...@intel.com> > Cc: Liming Gao <liming....@intel.com> > Signed-off-by: Shenglei Zhang <shenglei.zh...@intel.com> > --- > > v2: Update copyright > > .../Test/Library/TestPointCheckLib/DxeCheckBootVariable.c | 8 +++++++- > .../Test/Library/TestPointCheckLib/DxeCheckGcd.c | 6 ++++-- > 2 files changed, 11 insertions(+), 3 deletions(-) > > diff --git > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c > > b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c > index 85bd5b3d..98130683 100644 > --- > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckBootVariable.c > +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh > +++ eckBootVariable.c > @@ -1,6 +1,6 @@ > /** @file > > -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -130,6 +130,9 @@ TestPointCheckLoadOptionVariable ( > for (ListIndex = 0; ListIndex < > sizeof(mLoadOptionVariableList)/sizeof(mLoadOptionVariableList[0]); > ListIndex++) { > UnicodeSPrint (BootOrderName, sizeof(BootOrderName), L"%sOrder", > mLoadOptionVariableList[ListIndex]); > Status = GetVariable2 (BootOrderName, &gEfiGlobalVariableGuid, (VOID > **)&BootOrder, &OrderSize); > + if(BootOrder == NULL) { > + return EFI_NOT_FOUND;; > + } > if (EFI_ERROR(Status)) { > continue; > } > @@ -222,6 +225,9 @@ TestPointCheckKeyOptionVariable ( > for (Index = 0; ; Index++) { > UnicodeSPrint (KeyOptionName, sizeof(KeyOptionName), L"%s%04x", > mKeyOptionVariableList[ListIndex], Index); > Status = GetVariable2 (KeyOptionName, &gEfiGlobalVariableGuid, > &Variable, &Size); > + if(Variable == NULL) { > + return EFI_NOT_FOUND;; > + } > if (!EFI_ERROR(Status)) { > DumpKeyOption (KeyOptionName, Variable, Size); > } else { > diff --git > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c > b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c > index 82709d44..c90b37f2 100644 > --- > a/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCheckGcd.c > +++ b/Platform/Intel/MinPlatformPkg/Test/Library/TestPointCheckLib/DxeCh > +++ eckGcd.c > @@ -1,6 +1,6 @@ > /** @file > > -Copyright (c) 2017, Intel Corporation. All rights reserved.<BR> > +Copyright (c) 2017-2019, Intel Corporation. All rights reserved.<BR> > SPDX-License-Identifier: BSD-2-Clause-Patent > > **/ > @@ -241,7 +241,9 @@ TestPointDumpGcd ( > } > } > if (GcdMemoryMap != NULL) { > - *GcdIoMap = AllocateCopyPool (NumberOfDescriptors * > sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap); > + if (GcdIoMap != NULL){ > + *GcdIoMap = AllocateCopyPool (NumberOfDescriptors * > sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR), IoMap); > + } GcdIoMap will always be NULL. Please see line 199 of this file. I believe your patch is introducing a new bug. > *GcdIoMapNumberOfDescriptors = NumberOfDescriptors; > } > } > -- > 2.18.0.windows.1 > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#47233): https://edk2.groups.io/g/devel/message/47233 Mute This Topic: https://groups.io/mt/33110621/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-