Hi Albert, On Sun, Sep 25, 2011 at 1:40 AM, Albert ARIBAUD <albert.u.b...@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. > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
