Re: [PATCH] MdePkg SmmIoLib: Use NULL pointer check instead of useless Status check

[Udit Kumar <udit.kumar@nxp.com>]

Hi Star

I restricted myself into debug mode only , while looking at patch .

Regards
Udit

> -----Original Message-----
> From: Zeng, Star [mailto:star.zeng@intel.com]
> Sent: Wednesday, June 07, 2017 6:41 AM
> To: Udit Kumar <udit.kumar@nxp.com>; edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH] MdePkg SmmIoLib: Use NULL pointer check instead
> of useless Status check
> 
> 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-bounces@lists.01.org] On Behalf Of Udit
> Kumar
> Sent: Tuesday, June 6, 2017 7:11 PM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@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-bounces@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.yao@intel.com>; Star Zeng <star.zeng@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.yao@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Star Zeng <star.zeng@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