On 11/20/2017 04:44 PM, Wolfgang Denk wrote:
Dear Heinrich,

In message <452bd166-5cc7-ff1d-925c-dc8b8e1bd...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Reply via email to