Hi Udit, What does "If mSmmIoLibGcdMemSpace is NULL then if condition is not reachable." mean?
ASSERT_EFI_ERROR macro only effects at DEBUG mode, and the if condition is for error handling. Thanks, Star -----Original Message----- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Udit Kumar Sent: Tuesday, June 6, 2017 7:11 PM To: Zeng, Star <star.z...@intel.com>; edk2-devel@lists.01.org Cc: Yao, Jiewen <jiewen....@intel.com> Subject: Re: [edk2] [PATCH] MdePkg SmmIoLib: Use NULL pointer check instead of useless Status check Hi Star > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > Star Zeng > Sent: Monday, June 05, 2017 12:11 PM > To: edk2-devel@lists.01.org > Cc: Jiewen Yao <jiewen....@intel.com>; Star Zeng <star.z...@intel.com> > Subject: [edk2] [PATCH] MdePkg SmmIoLib: Use NULL pointer check > instead of useless Status check > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=587 > > The Status check in "if (!EFI_ERROR (Status))" condition is useless, > it should be NULL pointer check. And this patch also fixes a typo > "continous" to "continuous". > > Cc: Jiewen Yao <jiewen....@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Star Zeng <star.z...@intel.com> > --- > MdePkg/Library/SmmIoLib/SmmIoLib.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/MdePkg/Library/SmmIoLib/SmmIoLib.c > b/MdePkg/Library/SmmIoLib/SmmIoLib.c > index 181abb8e25df..f1cb0dace500 100644 > --- a/MdePkg/Library/SmmIoLib/SmmIoLib.c > +++ b/MdePkg/Library/SmmIoLib/SmmIoLib.c > @@ -156,7 +156,7 @@ SmmIsMmioValid ( > } > > /** > - Merge continous entries whose type is EfiGcdMemoryTypeMemoryMappedIo. > + Merge continuous entries whose type is > EfiGcdMemoryTypeMemoryMappedIo. > > @param[in, out] GcdMemoryMap A pointer to the buffer in which > firmware places > the current GCD memory map. > @@ -217,7 +217,8 @@ MergeGcdMmioEntry ( > @param[in] Interface Points to the interface instance. > @param[in] Handle The handle on which the interface was installed. > > - @retval EFI_SUCCESS Notification runs successfully. > + @retval EFI_SUCCESS Notification runs successfully. > + @retval EFI_OUT_OF_RESOURCES No enough resources to save GCD MMIO > map. > **/ > EFI_STATUS > EFIAPI > @@ -237,10 +238,10 @@ SmmIoLibInternalEndOfDxeNotify ( > MergeGcdMmioEntry (MemSpaceMap, &NumberOfDescriptors); > > mSmmIoLibGcdMemSpace = AllocateCopyPool (NumberOfDescriptors * > sizeof (EFI_GCD_MEMORY_SPACE_DESCRIPTOR), MemSpaceMap); > - ASSERT_EFI_ERROR (Status); > - if (EFI_ERROR (Status)) { > + ASSERT (mSmmIoLibGcdMemSpace != NULL); > + if (mSmmIoLibGcdMemSpace == NULL) { If mSmmIoLibGcdMemSpace is NULL then if condition is not reachable. If not NULL then if condition will be false always, I think if condition is not needed. > gBS->FreePool (MemSpaceMap); > - return Status; > + return EFI_OUT_OF_RESOURCES; > } > > mSmmIoLibGcdMemNumberOfDesc = NumberOfDescriptors; > -- > 2.7.0.windows.1 Regards Udit > _______________________________________________ > 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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
