On 05/02/16 17:09, Laszlo Ersek wrote:
> On 04/29/16 09:01, Ruiyu Ni wrote:
>> EnableQuietBoot and DisableQuietBoot are copied from
>> IntelFrameworkModulePkg/Library/GenericBdsLib/BdsConsole.c.
>> Because these two functions are not in UefiBootManagerLib.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ruiyu Ni <ruiyu...@intel.com>
>> Cc: Jordan Justen <jordan.l.jus...@intel.com>
>> Cc: Laszlo Ersek <ler...@redhat.com>
>> ---
>>  .../Library/PlatformBootManagerLib/BdsPlatform.h   |  32 +
>>  .../PlatformBootManagerLib.inf                     |   6 +
>>  OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c | 671 
>> +++++++++++++++++++++
>>  3 files changed, 709 insertions(+)
>>  create mode 100644 OvmfPkg/Library/PlatformBootManagerLib/QuietBoot.c
> 
> Reviewed-by: Laszlo Ersek <ler...@redhat.com>

When building this series with the GCC48 toolchain for the Ia32
architecture (and DEBUG target), the compiler complains that the Height
and Width variables of the EnableQuietBoot() function may be used
uninitialized.

* These warnings look unjustified to me. Namely, near the beginning of
the function, there is a while(1) loop. In that loop,
ConvertBmpToGopBlt() is called unconditionally. If the call fails, the
rest of the loop body is not reached (where the Height and Width
variables are used -- the compiler warns about their use in the switch
statement). If the call succeeds, then the variables are set.

Can you please insert a separate patch after this one in the series,
setting Height and Width to zero in EnableQuietBoot()? A good location
is just under "Instance = 0;".

The commit message should state that the assignments suppress an invalid
gcc-4.8 warning (on Ia32). If you wish, you can include my paragraph
marked with * above.

For this patch (i.e., v4 20/23), my R-b stands, of course.

Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to