[PATCH] tiny-printf: Support %i

[Tom Rini]

On Tue, Apr 14, 2020 at 02:24:18PM +0200, Marek Vasut wrote:
> On 4/14/20 5:03 AM, Tom Rini wrote:
> > On Tue, Apr 14, 2020 at 03:17:16AM +0200, Marek Vasut wrote:
> >> On 4/14/20 1:27 AM, Tom Rini wrote:
> >>> On Fri, Apr 10, 2020 at 08:54:49PM +0200, Marek Vasut wrote:
> >>>
> >>>> The most basic printf("%i", value) formating string was missing,
> >>>> add it for the sake of convenience.
> >>>>
> >>>> Signed-off-by: Marek Vasut <marex at denx.de>
> >>>> Cc: Simon Glass <sjg at chromium.org>
> >>>> Cc: Stefan Roese <sr at denx.de>
> >>>> ---
> >>>>  lib/tiny-printf.c | 3 ++-
> >>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>
> >>>> diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
> >>>> index 1138c7012a..8fc7e48d99 100644
> >>>> --- a/lib/tiny-printf.c
> >>>> +++ b/lib/tiny-printf.c
> >>>> @@ -242,6 +242,7 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va)
> >>>>  				goto abort;
> >>>>  			case 'u':
> >>>>  			case 'd':
> >>>> +			case 'i':
> >>>>  				div = 1000000000;
> >>>>  				if (islong) {
> >>>>  					num = va_arg(va, unsigned long);
> >>>> @@ -251,7 +252,7 @@ static int _vprintf(struct printf_info *info, const char *fmt, va_list va)
> >>>>  					num = va_arg(va, unsigned int);
> >>>>  				}
> >>>>  
> >>>> -				if (ch == 'd') {
> >>>> +				if (ch != 'u') {
> >>>>  					if (islong && (long)num < 0) {
> >>>>  						num = -(long)num;
> >>>>  						out(info, '-');
> >>>
> >>> How much does the size change and where do we see this as a problem?
> >>
> >> Any code which uses %i in SPL just misbehaves, e.g.
> >> printf("%s[%i] value=%x", __func__, __LINE__, val);
> >> prints function name and then incorrect value, because %i is ignored.
> >> This is also documented in the commit message.
> >>
> >> U-Boot grows in size massively due to all the DM/DT bloat which is being
> >> forced upon everyone, but there the uncontrolled growth is apparently OK
> >> even if it brings no obvious improvement, rather the opposite. And yet
> >> here, size increase suddenly matters? Sorry, that's not right.
> >>
> >> The code grows by 6 bytes.
> > 
> > Yes, it matters for _tiny-printf_ as that's where we have little to no
> > room for growth.
> 
> How many systems that use tiny-printf in SPL are also forced to use DM
> in SPL ?

I don't know how many times I've said no one is forced to switch to DM
in SPL.

> > So it's just debug prints you were doing that ran in
> > to a problem?  Thanks!
> 
> git grep %i indicates ~400 sites where %i is used, so no, not just debug
> prints. All of those are broken. And no, I'm not inclined to patch all
> the code to use %d instead of %i just because handling %i is a problem.

Not all 400 of them but the ones that are expected to be used in SPL and
with TINY_PRINTF need to, yes.  Go look at the git log of tiny-printf.c,
we've changed things around to avoid growth when at all possible.
Because yes, I don't want to grow a few hundred boards by 6 bytes when
we have a reasonable alternative.  There's 300 hits, of which a dozen
are non-debug and likely to ever be in SPL code.  And no, this isn't the
first time I've raised such an issue, it's just the first time you've
been hit by this, sorry.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200414/6fbbc19a/attachment.sig>