Re: [PATCH] MdeModulePkg/DxeCore: Add ASSERT to ensure no subtract underflow

["Yao, Jiewen" <jiewen.yao@intel.com>]

Reviewed-by: jiewen.yao@intel.com

> -----Original Message-----
> From: Wu, Hao A
> Sent: Monday, April 10, 2017 2:16 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A <hao.a.wu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH] MdeModulePkg/DxeCore: Add ASSERT to ensure no subtract
> underflow
> 
> For function SplitRecord() in file PropertiesTable.c, there is a
> potential subtract underflow case for line:
> 
>   return TotalNewRecordCount - 1;
> 
> However, such case will not happen since the logic in function
> SplitTable() ensure that when calling SplitRecord(), the variable
> 'TotalNewRecordCount' will not be zero when performing the subtraction.
> It will be handled in the previous if statement:
> 
>   if (MaxSplitRecordCount == 0) {
>     CopyMem (NewRecord, OldRecord, DescriptorSize);
>     return 0;
>   }
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Hao Wu <hao.a.wu@intel.com>
> ---
>  MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> index e7c4a95712..6cf5edcbe5 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> @@ -576,6 +576,11 @@ SplitRecord (
>      TempRecord.NumberOfPages = EfiSizeToPages (PhysicalEnd -
> PhysicalStart);
>    } while ((ImageRecord != NULL) && (PhysicalStart < PhysicalEnd));
> 
> +  //
> +  // The logic in function SplitTable() ensures that TotalNewRecordCount will
> not be zero if the
> +  // code reaches here.
> +  //
> +  ASSERT (TotalNewRecordCount != 0);
>    return TotalNewRecordCount - 1;
>  }
> 
> --
> 2.12.0.windows.1