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

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

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