Hi Albert,
On Sun, Sep 25, 2011 at 1:40 AM, Albert ARIBAUD albert.u.boot@aribaud.net wrote:
Le 24/09/2011 16:00, Simon Glass a écrit :
So basically the choice is between:
- adding code to the printf() family to try and fix an issue that it is
fundamentally unable to properly fix, and for which a solution exists, or
- grepping and fixing calls to *sprintf() in U-Boot that do not respect
the known contraints of printf(), by resizing the buffer or calling *snprintf() instead.
I am definitely not in favor of the first option concerning U-Boot.
Sounds fine to me. So I think we need the nprintf() variants in there, but the message is not to use them willy nilly. Going back to my patch series, 3/4 is ok, but 4/4 mostly crosses the line. Do I have that right?
It is the exact opposite for me : 3/4 makes all printf functions work like some kind of *nprintf(), while 4/4 is about the network code switching to *nprintf() for safety, so 3/4 would be nak and 4/4 ack as far as I am concerned.
Basically, printf family functions which do not have the 'n' are *know* by all -- experienced enough :) -- programmers to be *unsafe* (but to require less from the caller) and it should remain so: no programmer should ever encounter an implementation of printf that pretends to be even somewhat safe, because it might bite him/her elsewhere, in another project based on another C library where printf is just the beartrap it usually is.
IOW, programmers already have assumptions about *printf(), including how to deal with length limitations and what happens if you don't, and it is best that these assumption remain true whatever project they work with.
I don't quite understand this. You are saying that we shouldn't modify the existing printf(), serial_printf() etc. so live within the buffer they use? If I call printf() and my resulting string is too long for the library then I would much prefer to get truncated output than have to hunt for a wierd crash.
It sounds like you are asking for a new printf(), serial_printf() which provides the facility of limiting the output to n characters, and leave these ones alone. But these functions are not passed a buffer - they use their own and they set the site of it. So I think they should be responsible for enforcing that size.
If you are asking for a new set of functions - nprintf(), serial_nprintf(), etc. then I wonder how you can pass the 'n' but not the actual buffer. In my mind you should not do one without the other.
So please can you clarify?
By the way, printf() ends up calling the same code, but without limit checking in place. The alternative is to duplicate all the format string processing code (a limit-checking version and an unchecked version) which would be worse.
I don't intend to dictate the way things can be implemented, so the degree of code reuse is an open question as far as I am concerned. I am only voicing my opinion that *printf() APIs and their contracts should remain identical across all implementations of *printf(), and thus that providing *nprintf() where they don't exist is commandable, but hardening printf() is not, since you basically cannot do it without somewhat departing from the de facto standard.
OK fair enough. People are used to printf() just working, no matter what the size. I haven't looked at the implementation but I suspect when the buffer fills it outputs it and starts the buffer again .In any case you don't have to worry about it with the GNU C library.
We probably don't need to go that far in U-Boot, but some simple checking would avoid a nasty surprise for the user. It is obvious from the result that something is truncated, and we can WARN if that helps.
Regards, Simon
Regards, Simon
Amicalement,
Albert.