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

[Wolfgang Denk]

Dear Heinrich Schuchardt,

In message <d643b22b-2511-9b40-772b-5f7f4486c289 at gmx.de> you wrote:
>
> >>>> +	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?

It should be somewhere - but you cannot handle an error condition
that you don't report to the caller.

Instead of returning from the function with a clear error message
and an error return code, you ignore it.

The minimum action to take here would be that device_path_string()
returns NULL when the error occurs, if there was a chance for the
caller to handle that.

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

But if you don't even return an error code you have no chance to
handle it elsewhere.

> > 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.

This is incorrect. printf() is declared as

	int    printf(const char *fmt, ...);

as usual.

> So here we could only panic().
> 
> Is this really what you have in mind?

Well, which other options do you see when you run out of memory?
I can't see how you could recover from this, so if you don't abort
here with a clear error message you will either do the same
elsewhere (with a misleading error, as the actual problem happened
eventually long before), or you will just crash, or run into
undefined behaviour.

Ignoring error condistions is always a Bad thing (TM).

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
How long does it take a  DEC  field  service  engineer  to  change  a
lightbulb?       It depends on how many bad ones he brought with him.