Re: [PATCH 2/2] MdeModulePkg/BmpSupportLib: Check PixelHeight/PixelWidth against 0

["Zeng, Star" <star.zeng@intel.com>]

Ok. Reviewed-by: Star Zeng <star.zeng@intel.com>.


Thanks,
Star
-----Original Message-----
From: Ni, Ruiyu 
Sent: Tuesday, July 3, 2018 10:17 AM
To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>
Subject: Re: [PATCH 2/2] MdeModulePkg/BmpSupportLib: Check PixelHeight/PixelWidth against 0

On 7/3/2018 10:10 AM, Zeng, Star wrote:
> So, you mean caller knows ((PixelHeight == 0) || (PixelWidth == 0)), and caller knows TranslateGopBltToBmp will return RETURN_UNSUPPORTED for that?
Caller knows an error status may return when any of PixelHeight/PixelWidth is 0.
But that's not the reason that I only add debug message for BmpHeader case.

For first API:
TranslateBmpToGopBlt (
   IN     VOID                           *BmpImage,
   IN     UINTN                          BmpImageSize,
   IN OUT EFI_GRAPHICS_OUTPUT_BLT_PIXEL  **GopBlt,
   IN OUT UINTN                          *GopBltSize,
   OUT    UINTN                          *PixelHeight,
   OUT    UINTN                          *PixelWidth

Caller only needs to read a bmp file from a file system, then passes the buffer to the above API. Caller doesn't know the format of BMP.
API checks for that. And using debug message to help caller understand the detailed error reason.

For second API:
TranslateGopBltToBmp (
   IN     EFI_GRAPHICS_OUTPUT_BLT_PIXEL  *GopBlt,
   IN     UINT32                         PixelHeight,
   IN     UINT32                         PixelWidth,
   IN OUT VOID                           **BmpImage,
   IN OUT UINT32                         *BmpImageSize

PixelHeight and PixelWidth are parameters.


> 
> 
> Thanks,
> Star
> -----Original Message-----
> From: Ni, Ruiyu
> Sent: Tuesday, July 3, 2018 10:03 AM
> To: Zeng, Star <star.zeng@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: Re: [PATCH 2/2] MdeModulePkg/BmpSupportLib: Check 
> PixelHeight/PixelWidth against 0
> 
> On 7/2/2018 6:05 PM, Zeng, Star wrote:
>> Please add reference to bugzilla link into the commit log.
>>
>> https://bugzilla.tianocore.org/show_bug.cgi?id=944
> OK
>>
>> Another, is there any reason to only add debug message for " if ((BmpHeader->PixelHeight == 0) || (BmpHeader->PixelWidth == 0)) { ", but not " if ((PixelHeight == 0) || (PixelWidth == 0)) { " ???
> PixelHeight and PixelWidth are passed in from caller directly.
> But when caller passes in BmpImage buffer, it doesn't know the format of the BMP header, so a debug message is shown to tell caller what happens.
> 
> --
> Thanks,
> Ray
> 


--
Thanks,
Ray