Re: [edk2] [PATCH 1/3] OvmfPkg: QemuBootOrder: collect active UEFI boot options in advance

Subject: Re: [edk2] [PATCH 1/3] OvmfPkg: QemuBootOrder: collect active UEFI boot options in advance

From: Jordan Justen <jljusten@gmail.com>

To: Laszlo Ersek <lersek@redhat.com>

Date: 2013-09-12 00:04:46

On Wed, Sep 11, 2013 at 2:03 AM, Laszlo Ersek  wrote:
> On 09/11/13 10:15, Jordan Justen wrote:
>> On Fri, Sep 6, 2013 at 2:40 PM, Laszlo Ersek  wrote:
>>> In preparation for the next patch, collect active UEFI boot options in
>>> advance into a new array. Rebase the current inner loop (the matching
>>> loop) to this array.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.0
>>>
>>> Signed-off-by: Laszlo Ersek 
>>> ---
>>>  OvmfPkg/Library/PlatformBdsLib/QemuBootOrder.c |  115 ++++++++++++++++++++----
>>>  1 files changed, 97 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/OvmfPkg/Library/PlatformBdsLib/QemuBootOrder.c b/OvmfPkg/Library/PlatformBdsLib/QemuBootOrder.c
>>> index e9580b9..7139cc2 100644
>>> --- a/OvmfPkg/Library/PlatformBdsLib/QemuBootOrder.c
>>> +++ b/OvmfPkg/Library/PlatformBdsLib/QemuBootOrder.c
>>> @@ -238,6 +238,15 @@ typedef struct {
>>>
>>>
>>>  /**
>>> +  Array element tracking an enumerated boot option that has the
>>> +  LOAD_OPTION_ACTIVE attribute.
>>> +**/
>>> +typedef struct {
>>> +  CONST BDS_COMMON_OPTION *BootOption; // reference only, no ownership
>>> +} ACTIVE_OPTION;
>>> +
>>> +
>>> +/**
>>>
>>>    Append BootOptionId to BootOrder, reallocating the latter if needed.
>>>
>>> @@ -283,6 +292,75 @@ BootOrderAppend (
>>>
>>>
>>>  /**
>>> +
>>> +  Create an array of ACTIVE_OPTION elements for a boot option list.
>>> +
>>> +  @param[in]  BootOptionList  A boot option list, created with
>>> +                              BdsLibEnumerateAllBootOption().
>>> +
>>> +  @param[out] ActiveOption    Pointer to the first element in the new array.
>>> +                              The caller is responsible for freeing the array
>>> +                              with FreePool() after use.
>>> +
>>> +  @param[out] Count           Number of elements in the new array.
>>> +
>>> +
>>> +  @retval RETURN_SUCCESS           The ActiveOption array has been created.
>>> +
>>> +  @retval RETURN_NOT_FOUND         No active entry has been found in
>>> +                                   BootOptionList.
>>> +
>>> +  @retval RETURN_OUT_OF_RESOURCES  Memory allocation failed.
>>> +
>>> +**/
>>> +STATIC
>>> +RETURN_STATUS
>>> +CollectActiveOptions (
>>> +  IN   CONST LIST_ENTRY *BootOptionList,
>>> +  OUT  ACTIVE_OPTION    **ActiveOption,
>>> +  OUT  UINTN            *Count
>>> +  )
>>> +{
>>> +  UINTN ScanMode;
>>> +
>>
>> GCC 4.7 for IA32 wants to issue a warning unless I set *ActiveOption
>> to NULL here. Do you see the same?
>
> Unfortunately, I do.
>
> When building for IA32 I see the warning (treated as an error) with both
> gcc-4.4 and gcc-4.7, issued for the caller:
>
> gcc-4.4:
> .../QemuBootOrder.c: In function 'SetBootOrderFromQemu':
> .../QemuBootOrder.c:1152: error: 'ActiveOption' may be used
> uninitialized in this function
>
> gcc-4.7:
> .../QemuBootOrder.c: In function 'SetBootOrderFromQemu':
> .../QemuBootOrder.c:1152:37: error: 'ActiveOption' may be used
> uninitialized in this function [-Werror=maybe-uninitialized]
>
> When targeting X64, neither gcc version emits the warning.
>
> The warning is wrong by the way.
>
> I wonder what else VS might choke on.

I tried VS2012, and it seemed okay with the original series for both IA32 & X64.

> Do you want me to resubmit the series?

I can add this if you are okay with it.

-Jordan

------------------------------------------------------------------------------
How ServiceNow helps IT people transform IT departments:
1. Consolidate legacy IT systems to a single system of record for IT
2. Standardize and globalize service processes across IT
3. Implement zero-touch automation to replace manual, redundant tasks
http://pubads.g.doubleclick.net/gampad/clk?id=51271111&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel