On 21/11/2016 16:56, Andre Przywara wrote:
Hi Alex,
On 21/11/16 15:42, Alexander Graf wrote:
On 20/11/2016 15:56, Andre Przywara 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.
Signed-off-by: Andre Przywara <andre.przyw...@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++);
+
What does this do? I don't see '-' mentioned in the patch description.
Argh, apparently the comment in the commit message got lost during a
patch reshuffle. Sorry, will re-add it.
We need it because some SPL printf uses '-', just ignoring it here seems
fine for SPL purposes though.
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) {
Check here if sizeof(long) > 4, so that the whole branch gets optimized
away on 32bit.
Good idea.
+ 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;
Num is a long now and before. So if you have a 32bit signed input, it
will sign extend incorrectly here. You need an additional check
if (islong)
num = -(long)num;
else
num = -(int)num;
Let's hope the compiler on 32bit is smart enough to know that it can
combine those two cases :).
out(info, '-');
}
if (!num) {
out_dgt(info, 0);
} else {
- for (div = 1000000000; div; div /= 10)
+ for (; div; div /= 10)
Any particular reason for that change?
This algorithm so far only cared for 32-bit values, so it set the start
divider to 1E9. This is not sufficient for 64-bit longs in AA64.
So I compute div above, depending on the actual size of long.
Ah, I missed the div *= up there. Sure, then it makes sense. Btw, have
you checked that the compiler is smart enough to do constant propagation
here? Multiplications can be very expensive.
Alex
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
