[U-Boot] [PATCH v2 04/23] SPL: tiny-printf: add "l" modifier

[Siarhei Siamashka <siarhei.siamashka@gmail.com>]

On Mon,  5 Dec 2016 01:52:11 +0000
Andre Przywara <andre.przywara@arm.com> wrote:

> tiny-printf does not know about the "l" modifier so far, which breaks
> the crash dump on AArch64, because it uses %lx to print the registers.
> Add an easy way of handling longs correctly. Also there are printfs
> using the '-' modifier, which we choose to ignore for simplicity.

If the '-' modifier is so useless in practice, then why don't we just
remove it from the format string of the caller rather than making
the printf implementation deviate from the expected behaviour?

From "man 3 printf":

" - The converted value is to be left adjusted on the field
    boundary.  (The default is right justification.)  The converted
    value is padded on the right with blanks, rather than on the left
    with blanks or zeros.  A - overrides a 0 if both are given."

Either way, I think that this change needs to be done as a separate
patch. Smuggling it as a part of this "l" modifier patch looks rather
fishy ;-)

> 
> Using a relatively decent compiler (GCC 5.3.0) this does _not_ increase
> the code size of tiny-printf.o for 32-bit builds (where long and int
> are actually the same), actually it looses three (ARM Thumb2) instructions
> from the actual SPL (numbers for orangepi_plus_defconfig):
>   text    data     bss     dec     hex filename
>    758       0       0     758     2f6 spl/lib/tiny-printf.o	before
>  18839     488     232   19559    4c67 spl/u-boot-spl		before
>    758       0       0     758     2f6 spl/lib/tiny-printf.o	after
>  18833     488     232   19553    4c61 spl/u-boot-spl		after

Very cool :-)

> 
> This adds some substantial amount of code to a 64-bit build, though:
> (taken after a later commit, which enables the ARM64 SPL build for sunxi)
>   text    data     bss     dec     hex filename
>   1542       0       0    1542     606 spl/lib/tiny-printf.o	before
>  25830     392     360   26582    67d6 spl/u-boot-spl		before
>   1758       0       0    1758     6de spl/lib/tiny-printf.o	after
>  26040     392     360   26792    68a8 spl/u-boot-spl		after

OK, I guess we have to live with this. One more win for Thumb2 vs.
AArch64 though.

And thanks for adding these stats to the commit message.

> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  lib/tiny-printf.c | 50 +++++++++++++++++++++++++++++++++++++++-----------
>  1 file changed, 39 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
> index 30ac759..dfa8432 100644
> --- a/lib/tiny-printf.c
> +++ b/lib/tiny-printf.c
> @@ -38,8 +38,8 @@ static void out_dgt(struct printf_info *info, char dgt)
>  	info->zs = 1;
>  }
>  
> -static void div_out(struct printf_info *info, unsigned int *num,
> -		    unsigned int div)
> +static void div_out(struct printf_info *info, unsigned long *num,
> +		    unsigned long div)
>  {
>  	unsigned char dgt = 0;
>  
> @@ -56,9 +56,9 @@ int _vprintf(struct printf_info *info, const char *fmt, va_list va)
>  {
>  	char ch;
>  	char *p;
> -	unsigned int num;
> +	unsigned long num;
>  	char buf[12];
> -	unsigned int div;
> +	unsigned long div;
>  
>  	while ((ch = *(fmt++))) {
>  		if (ch != '%') {
> @@ -66,8 +66,12 @@ int _vprintf(struct printf_info *info, const char *fmt, va_list va)
>  		} else {
>  			bool lz = false;
>  			int width = 0;
> +			bool islong = false;
>  
>  			ch = *(fmt++);
> +			if (ch == '-')
> +				ch = *(fmt++);
> +
>  			if (ch == '0') {
>  				ch = *(fmt++);
>  				lz = 1;
> @@ -80,6 +84,11 @@ int _vprintf(struct printf_info *info, const char *fmt, va_list va)
>  					ch = *fmt++;
>  				}
>  			}
> +			if (ch == 'l') {
> +				ch = *(fmt++);
> +				islong = true;
> +			}
> +
>  			info->bf = buf;
>  			p = info->bf;
>  			info->zs = 0;
> @@ -89,24 +98,43 @@ int _vprintf(struct printf_info *info, const char *fmt, va_list va)
>  				goto abort;
>  			case 'u':
>  			case 'd':
> -				num = va_arg(va, unsigned int);
> -				if (ch == 'd' && (int)num < 0) {
> -					num = -(int)num;
> -					out(info, '-');
> +				div = 1000000000;
> +				if (islong) {
> +					num = va_arg(va, unsigned long);
> +					if (sizeof(long) > 4)
> +						div *= div * 10;
> +				} else {
> +					num = va_arg(va, unsigned int);
> +				}
> +
> +				if (ch == 'd') {
> +					if (islong && (long)num < 0) {
> +						num = -(long)num;
> +						out(info, '-');
> +					} else if (!islong && (int)num < 0) {
> +						num = -(int)num;
> +						out(info, '-');
> +					}
>  				}
>  				if (!num) {
>  					out_dgt(info, 0);
>  				} else {
> -					for (div = 1000000000; div; div /= 10)
> +					for (; div; div /= 10)
>  						div_out(info, &num, div);
>  				}
>  				break;
>  			case 'x':
> -				num = va_arg(va, unsigned int);
> +				if (islong) {
> +					num = va_arg(va, unsigned long);
> +					div = 1UL << (sizeof(long) * 8 - 4);
> +				} else {
> +					num = va_arg(va, unsigned int);
> +					div = 0x10000000;
> +				}
>  				if (!num) {
>  					out_dgt(info, 0);
>  				} else {
> -					for (div = 0x10000000; div; div /= 0x10)
> +					for (; div; div /= 0x10)
>  						div_out(info, &num, div);
>  				}
>  				break;



-- 
Best regards,
Siarhei Siamashka