[U-Boot] [PATCH 05/24] SPL: tiny-printf: add "l" modifier

[Siarhei Siamashka]

On Sun, 20 Nov 2016 14:56:59 +0000
Andre Przywara <andre.przywara at 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.

I can't help but notice that the changes of this kind are in a way
defeating the original purpose of tiny-printf. And it is surely not
the first patch adding features to tiny-printf. I guess, in the end
we may end up with a large and bloated printf implementation again :-)

A possible solution might be just a strict check when parsing format
modifiers and abort with an error message (yeah, this will introduce
some size increase, but hopefully the last one). This way we
acknowledge the fact that tiny-printf is a reduced incomplete
implementation, and that the callers need to take this into account.

As for the "l" modifier. How much does it add to the code size? IMHO
this information should be mentioned in the commit message. Can the
AArch64 crash dump code be modified to avoid using it? Or can we have
the "l" modifier supported on 64-bit platforms only?

> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
> ---
>  lib/tiny-printf.c | 43 +++++++++++++++++++++++++++++++++----------
>  1 file changed, 33 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/tiny-printf.c b/lib/tiny-printf.c
> index 30ac759..b01099d 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,38 @@ 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;
> +				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' && (long)num < 0) {
> +					num = -(long)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