Dear Heinrich Schuchardt, In message <d643b22b-2511-9b40-772b-5f7f4486c...@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: w...@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. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot