[PATCH] efidebug: avoid 'dfu_alt_info not defined' message

[Heinrich Schuchardt]

On 1/17/22 01:45, AKASHI Takahiro wrote:
> On Sat, Jan 15, 2022 at 02:18:21AM +0100, Heinrich Schuchardt wrote:
>> If variable dfu_alt_info is not defined duplicate messages are displayed.
>>
>>      => efidebug boot dump
>>      Scanning disk mmc2.blk...
>>      Scanning disk mmc1.blk...
>>      Scanning disk mmc0.blk...
>>      Found 3 disks
>>      No EFI system partition
>>      "dfu_alt_info" env variable not defined!
>>      Probably dfu_alt_info not defined
>>      "dfu_alt_info" env variable not defined!
>>      Probably dfu_alt_info not defined
>>
>> Remove the 'Probably dfu_alt_info not defined' message.
>> Instead write a warning if the variable contains no entities.
>>
>> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
>> ---
>>   lib/efi_loader/efi_firmware.c | 7 +++++--
>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
>> index a1b88dbfc2..519a47267c 100644
>> --- a/lib/efi_loader/efi_firmware.c
>> +++ b/lib/efi_loader/efi_firmware.c
>> @@ -128,8 +128,11 @@ static efi_status_t efi_get_dfu_info(
>>   	size_t names_len, total_size;
>>   	int dfu_num, i;
>>   	u16 *name, *next;
>> +	int ret;
>>   
>> -	dfu_init_env_entities(NULL, NULL);
>> +	ret = dfu_init_env_entities(NULL, NULL);
>> +	if (ret)
>> +		return EFI_SUCCESS;
> 
> Do you want to return EFI_SUCCESS here?
> It is mandatory that "dfu_alt_info" be defined so that the current
> FMP drivers should work.
> So if the variable is not defined, I think,
> efi_firmware_[fit|raw]_get_image_info() should fail.
> Even if it returns EFI_SUCCESS, descriptor_count should be set to zero.

This patch did not change the return value. It only removed superfluous 
messages that I saw everytime I used the sandbox.

For dfu_num == 0 we are returning EFI_SUCCESS. If the number is 0 due to 
the content of dfu_alt_info or due to its absence, should not make a 
difference in the return code. Writing a message if dfu_alt_info exists 
but contains no entity seems reasonable.

If you think EFI_SUCCESS is not correct in both cases, please, send a 
followup patch for review.

Best regards

Heinrich

> 
> -Takahiro Akashi
> 
>>   	names_len = 0;
>>   	dfu_num = 0;
>> @@ -138,7 +141,7 @@ static efi_status_t efi_get_dfu_info(
>>   		dfu_num++;
>>   	}
>>   	if (!dfu_num) {
>> -		log_warning("Probably dfu_alt_info not defined\n");
>> +		log_warning("No entities in dfu_alt_info\n");
>>   		*image_info_size = 0;
>>   		dfu_free_entities();
>>   
>> -- 
>> 2.33.1
>>