[staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Check ImageIndex first before Image/ImageSize

[staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Check ImageIndex first before Image/ImageSize

[Star Zeng]

It does not make sense to check the Image/ImageSize if ImageIndex
has been invalid.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 FmpDevicePkg/FmpDxe/FmpDxe.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index eb452737543e..94fdf5608529 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -497,6 +497,15 @@ GetTheImage (
 
   Status = EFI_SUCCESS;
 
+  //
+  // Check to make sure index is 1 (only 1 image for this device)
+  //
+  if (ImageIndex != 1) {
+    DEBUG ((DEBUG_ERROR, "FmpDxe: GetImage() - Image Index Invalid.\n"));
+    Status = EFI_INVALID_PARAMETER;
+    goto cleanup;
+  }
+
   if ((ImageSize == NULL)) {
     DEBUG ((DEBUG_ERROR, "FmpDxe: GetImage() - ImageSize Pointer Parameter is NULL.\n"));
     Status = EFI_INVALID_PARAMETER;
@@ -523,16 +532,6 @@ GetTheImage (
     goto cleanup;
   }
 
-  //
-  // Check to make sure index is 1 (only 1 image for this device)
-  //
-  if (ImageIndex != 1) {
-    DEBUG ((DEBUG_ERROR, "FmpDxe: GetImage() - Image Index Invalid.\n"));
-    Status = EFI_INVALID_PARAMETER;
-    goto cleanup;
-  }
-
-
   Status = FmpDeviceGetImage (Image, ImageSize);
 cleanup:
 
-- 
2.7.0.windows.1

Re: [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Check ImageIndex first before Image/ImageSize

[Kinney, Michael D]

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

Mike

> -----Original Message-----
> From: Zeng, Star
> Sent: Friday, July 27, 2018 6:26 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Subject: [staging/FmpDevicePkg-master][PATCH]
> FmpDevicePkg FmpDxe: Check ImageIndex first before
> Image/ImageSize
> 
> It does not make sense to check the Image/ImageSize if
> ImageIndex
> has been invalid.
> 
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  FmpDevicePkg/FmpDxe/FmpDxe.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c
> b/FmpDevicePkg/FmpDxe/FmpDxe.c
> index eb452737543e..94fdf5608529 100644
> --- a/FmpDevicePkg/FmpDxe/FmpDxe.c
> +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
> @@ -497,6 +497,15 @@ GetTheImage (
> 
>    Status = EFI_SUCCESS;
> 
> +  //
> +  // Check to make sure index is 1 (only 1 image for
> this device)
> +  //
> +  if (ImageIndex != 1) {
> +    DEBUG ((DEBUG_ERROR, "FmpDxe: GetImage() - Image
> Index Invalid.\n"));
> +    Status = EFI_INVALID_PARAMETER;
> +    goto cleanup;
> +  }
> +
>    if ((ImageSize == NULL)) {
>      DEBUG ((DEBUG_ERROR, "FmpDxe: GetImage() -
> ImageSize Pointer Parameter is NULL.\n"));
>      Status = EFI_INVALID_PARAMETER;
> @@ -523,16 +532,6 @@ GetTheImage (
>      goto cleanup;
>    }
> 
> -  //
> -  // Check to make sure index is 1 (only 1 image for
> this device)
> -  //
> -  if (ImageIndex != 1) {
> -    DEBUG ((DEBUG_ERROR, "FmpDxe: GetImage() - Image
> Index Invalid.\n"));
> -    Status = EFI_INVALID_PARAMETER;
> -    goto cleanup;
> -  }
> -
> -
>    Status = FmpDeviceGetImage (Image, ImageSize);
>  cleanup:
> 
> --
> 2.7.0.windows.1