On 21/05/2021 16.15, Heinrich Schuchardt wrote: > On 21.05.21 14:53, Rasmus Villemoes wrote: >> On 20/05/2021 19.51, Simon Glass wrote: >>> Hi Rasmus, >>> >>> On Thu, 20 May 2021 at 04:05, Rasmus Villemoes >>> <rasmus.villem...@prevas.dk> wrote: >>>> >>>> Most callers (or callers of callers, etc.) of vsnprintf() are not >>>> prepared for it to return a negative value. >>>> >>>> The only case where that can currently happen is %pD, and it's IMO >>>> more user-friendly to produce some output that clearly shows that some >>>> "impossible" thing happened instead of having the message completely >>>> ignored - or mishandled as for example log.c would currently do. >>>> >>>> Signed-off-by: Rasmus Villemoes <rasmus.villem...@prevas.dk> >>>> --- >>>> lib/vsprintf.c | 10 +--------- >>>> 1 file changed, 1 insertion(+), 9 deletions(-) >>> >>> I think that is debatable. If we want the calling code to be fixed, >>> then it needs to get an error code back. Otherwise the error will be >>> apparent to the user but (perhaps) not ever debugged. >> >> But it is not the calling code that is at fault for the vsnprintf() >> implementation (1) being able to fail and (2) actually encountering an >> ENOMEM situation. There's _nothing_ the calling code can do about that. > > include/vsnprintf.h states: > > "This function follows C99 vsnprintf, but has some extensions:".
Happy to update that comment (which is copied from linux BTW, and in the kernel there's a very simple rule: "This is printk. We want it to work." - that extends to the workhorse vsnprintf() which is not allowed to take locks or do allocations) with an amendment "... among which is that it never returns a negative value." > It is obvious that the calling code needs to be fixed if it cannot > handle negative return values. > > So NAK to the patch. So you'd rather fix the ~200 places that use the return value assuming it's non-negative, plus the unknown number of places that assume the output buffer is a valid nul-terminated string after vsnprintf() returns? And, again taking that %pD example, do you really rather want _nothing_ printed (which is the only thing log.c could sensibly do if vsnprintf() returned something negative) in the rare case of allocation failure? I must admit I completely fail to see how this can possibly be better than improving the guarantees provided by U-Boot's vsnprintf() implementation. Rasmus
