[U-Boot] [PATCH v2 01/18] efi_loader: efi_bootmgr: do not make hidden assignments

[Heinrich Schuchardt]

On 11/17/2017 03:04 PM, Rob Clark wrote:
> On Sun, Nov 12, 2017 at 9:02 AM, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>> Assignments should not be made in the middle of nowhere.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>> v2
>>          Call efi_dp_str in debug mode only.
>> ---
>>   lib/efi_loader/efi_bootmgr.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
>> index 857d88a879..a55f210f0b 100644
>> --- a/lib/efi_loader/efi_bootmgr.c
>> +++ b/lib/efi_loader/efi_bootmgr.c
>> @@ -120,11 +120,13 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path,
>>
>>          if (lo.attributes & LOAD_OPTION_ACTIVE) {
>>                  efi_status_t ret;
>> -               u16 *str = NULL;
>> +#ifdef _DEBUG
>> +               u16 *str = efi_dp_str(lo.file_path);
>>
>>                  debug("%s: trying to load \"%ls\" from: %ls\n", __func__,
>> -                     lo.label, (str = efi_dp_str(lo.file_path)));
>> +                     lo.label, str);
>>                  efi_free_pool(str);
>> +#endif /* _DEBUG */
> 
> I was trying to avoid the #ifdef DEBUG in the first place.. ;-)
> 
> btw, don't have the code in front of me atm, but I thought _DEBUG was
> unconditionally defined to either 0 or 1.. so maybe you mean '#if
> _DEBUG' or '#ifdef DEBUG' (no underscore)?

Thanks for catching this

I will change this to
#if _DEBUG

I often found the necessity to change the value of _DEBUG multiple times 
within a single C file to get only the relevant debug messages. This is 
why I prefer checking _DEBUG.

> 
> btw2, possibly a better/cleaner option is add printf support for
> format modifier to print device-paths?

This is the only place where efi_dp_str is called.

Regards

Heinrich

> 
> BR,
> -R
> 
>>
>>                  ret = efi_load_image_from_path(lo.file_path, &image);
>>
>> --
>> 2.15.0
>>
>