[U-Boot] [PATCH 1/1] vsprintf.c: add EFI device path printing

[Heinrich Schuchardt]

On 11/20/2017 04:44 PM, Wolfgang Denk wrote:
> Dear Heinrich,
> 
> In message <452bd166-5cc7-ff1d-925c-dc8b8e1bdbe5 at gmx.de> you wrote:
>>
>>>> +#ifdef CONFIG_EFI_LOADER
>>>> +static char *device_path_string(char *buf, char *end, void *dp, int field_width,
>>>> +				int precision, int flags)
>>>> +{
>>>> +	u16 *str = efi_dp_str((struct efi_device_path *)dp);
>>>> +
>>>> +	buf = string16(buf, end, str, field_width, precision, flags);
>>>> +	efi_free_pool(str);
>>>
>>> efi_dp_str() can return NULL. Should this not be handled?
>>
>> Thanks for reviewing.
>>
>> This situation is caught in string16:
>> u16 *str = s ? s : L"<NULL>";
> 
> This is crash-preventing cosmetics, but no real error handling.  I
> mean, should we not wave a big red flag and shout at the user: "Hey,
> your system is going berserk here!" ?

Why do you think that the error handling should be in THIS function?

I can understand that we might improve error handling in the EFI coding 
but I do not believe we should do it here.

> 
>> It can only occur if we are out of memory. All other error handling
>> should be added to efi_convert_device_path_to_text().
> 
> Yes, and running out of memory at such a place is likely to be
> unrecoverable. So we should terminate all further processing baed on
> this result, and not continue with bogus data.

printf() does not have any return value. So here we could only panic().

Is this really what you have in mind?

Best regards

Heinrich