Skip to content

Commit

Permalink
Corrected printf of long and long long args. Closes #314
Browse files Browse the repository at this point in the history
  • Loading branch information
@krichardsson
krichardsson committed May 7, 2018
1 parent b2e8893 commit 9173b9b
Showing 1 changed file with 29 additions and 14 deletions.
43 changes: 29 additions & 14 deletions src/utils/src/eprintf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,46 +151,54 @@ static int itoa16(putc_t putcf, uint64_t num, int width, char padChar)
return len;
}

static int handleLongLong(putc_t putcf, char** fmt, va_list ap, int width, char padChar)
static int handleLongLong(putc_t putcf, char** fmt, unsigned long long int val, int width, char padChar)
{
int len = 0;

switch(*((*fmt)++))
{
case 'i':
case 'd':
return itoa10(putcf, va_arg(ap, long long int), 0);
len = itoa10(putcf, (long long int)val, 0);
break;
case 'u':
return itoa10Unsigned(putcf, va_arg(ap, long long unsigned int));
len = itoa10Unsigned(putcf, val);
break;
case 'x':
case 'X':
return itoa16(putcf, va_arg(ap, uint64_t), width, padChar);
len = itoa16(putcf, val, width, padChar);
break;
default:
// Nothing here
break;
}

return 0;
return len;
}

static int handleLong(putc_t putcf, char** fmt, va_list ap, int width, char padChar)
static int handleLong(putc_t putcf, char** fmt, unsigned long int val, int width, char padChar)
{
int len = 0;

switch(*((*fmt)++))
{
case 'i':
case 'd':
return itoa10(putcf, va_arg(ap, long int), 0);
len = itoa10(putcf, (long int)val, 0);
break;
case 'u':
return itoa10Unsigned(putcf, va_arg(ap, unsigned long int));
len = itoa10Unsigned(putcf, val);
break;
case 'x':
case 'X':
return itoa16(putcf, va_arg(ap, unsigned long int), width, padChar);
case 'l':
return handleLongLong(putcf, fmt, ap, width, padChar);
len = itoa16(putcf, val, width, padChar);
break;
default:
// Nothing here
break;
}

return 0;
return len;
}

int evprintf(putc_t putcf, char * fmt, va_list ap)
Expand Down Expand Up @@ -250,7 +258,14 @@ int evprintf(putc_t putcf, char * fmt, va_list ap)
len += itoa16(putcf, va_arg(ap, unsigned int), width, padChar);
break;
case 'l':
len += handleLong(putcf, &fmt, ap, width, padChar);
// Look ahead for ll
if (*fmt == 'l') {
fmt++;
len += handleLongLong(putcf, &fmt, va_arg(ap, unsigned long long int), width, padChar);
} else {
len += handleLong(putcf, &fmt, va_arg(ap, unsigned long int), width, padChar);
}

break;
case 'f':
num = va_arg(ap, double);
Expand All @@ -265,7 +280,7 @@ int evprintf(putc_t putcf, char * fmt, va_list ap)
len += itoa10(putcf, (num - (int)num) * power(10,precision), precision);
break;
case 's':
str = va_arg( ap, char* );
str = va_arg(ap, char* );
while(*str)
{
putcf(*str++);
Expand Down

4 comments on commit 9173b9b

@ledvinap
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing pointer to va_list is another option (https://stackoverflow.com/questions/3369588/pass-va-list-or-pointer-to-va-list). It will encapsulate consuming of arguments

@krichardsson
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ledvinap, I had the same idea, but it turned out to be problematic. According to http://en.cppreference.com/w/cpp/utility/variadic/va_list it is not OK to use a va_list after it has been passed to another function as a pointer.
I'm not an expert on variadic functions but I realized it can be a bit more complex than I used to think. Apparently on some architectures the first arguments are passed to a function in registers, as opposed to the stack, which complicates matters when passing them on to the next function.

The behaviour in the unit tests (running on my Mac) and in the Crazyflie are also very different, some solutions did work on one platform but not the other, some did not even compile on one of the platforms but was OK on the other.

I decided that it was easier to not pass the va_list on to the next function :-)

@ledvinap
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Just for completeness)
from cppreference: It is legal to pass a pointer to a va_list object to another function and then use that object after the function returns.
And footnote from standard (from https://stackoverflow.com/questions/3369588/pass-va-list-or-pointer-to-va-list) 215) It is permitted to create a pointer to a va_list and pass that pointer to another function, in which case the original function may make further use of the original list after the other function returns

Even amd64 implementation uses struct, pointer to this struct can be passed around (both saved registers and stack are in same place in called function)

@krichardsson
Copy link
Contributor Author

@krichardsson krichardsson commented on 9173b9b May 9, 2018 via email

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.