michaelh/zephyr
1
0
Fork 0
Code Releases Activity

prf.c: remove buffer limitation on field width and string copy

Browse source 
The z_prf() function currently allocates a 200-byte buffer on the
stack to copy strings into, and then perform left/right alignment
and padding. Not only this is a pretty large chunk of stack usage,
but this imposes limitations on field width and string length. Also
the string is copied not only once but _thrice_ making this code
less than optimal.

Let's rework the code to get rid of both the field width limit and
string length limit, as well as the two extra memory copy instances.

While at it, let's fixes printf("%08s", "abcd") which used to
produce "0000abcd".

Signed-off-by: Nicolas Pitre <npitre@baylibre.com>
This commit is contained in:
Nicolas Pitre 2019-06-18 14:27:04 -04:00 • committed by Anas Nashif
commit 33312cfd98
2 changed files with 47 additions and 80 deletions
Download patch file Download diff file Expand all files Collapse all files

108
lib/libc/minimal/source/stdout/prf.c
View file

@ -12,6 +12,7 @@
#include <stdarg.h> #include <stdarg.h>
#include <string.h> #include <string.h>
#include <ctype.h> #include <ctype.h>
#include <limits.h>
#include <sys/types.h> #include <sys/types.h>
#include <sys/util.h> #include <sys/util.h>
@ -455,7 +456,6 @@ int z_prf(int (*func)(), void *dest, const char *format, va_list vargs)
char *cptr; char *cptr;
bool falt, fminus, fplus, fspace; bool falt, fminus, fplus, fspace;
int i; int i;
bool need_justifying;
char pad; char pad;
int precision; int precision;
int prefix; int prefix;
@ -473,7 +473,6 @@ int z_prf(int (*func)(), void *dest, const char *format, va_list vargs)
} else { } else {
fminus = fplus = fspace = falt = false; fminus = fplus = fspace = falt = false;
pad = ' '; /* Default pad character */ pad = ' '; /* Default pad character */
precision = -1; /* No precision specified */
while (strchr("-+ #0", (c = *format++)) != NULL) { while (strchr("-+ #0", (c = *format++)) != NULL) {
switch (c) { switch (c) {
@ -517,16 +516,7 @@ int z_prf(int (*func)(), void *dest, const char *format, va_list vargs)
c = *format++; c = *format++;
} }
/* precision = -1;
* If <width> is INT_MIN, then its absolute value can
* not be expressed as a positive number using 32-bit
* two's complement. To cover that case, cast it to
* an unsigned before comparing it against MAXFLD.
*/
if ((unsigned) width > MAXFLD) {
width = MAXFLD;
}
if (c == '.') { if (c == '.') {
c = *format++; c = *format++;
if (c == '*') { if (c == '*') {
@ -565,14 +555,13 @@ int z_prf(int (*func)(), void *dest, const char *format, va_list vargs)
} }
} }
need_justifying = false; cptr = buf;
prefix = 0; prefix = 0;
switch (c) { switch (c) {
case 'c': case 'c':
buf[0] = va_arg(vargs, int); buf[0] = va_arg(vargs, int);
buf[1] = '\0';
need_justifying = true;
c = 1; c = 1;
pad = ' ';
break; break;
case 'd': case 'd':
@ -599,8 +588,7 @@ int z_prf(int (*func)(), void *dest, const char *format, va_list vargs)
if (fplus || fspace || val < 0) { if (fplus || fspace || val < 0) {
prefix = 1; prefix = 1;
} }
need_justifying = true; if (precision >= 0) {
if (precision != -1) {
pad = ' '; pad = ' ';
} }
break; break;
@ -637,7 +625,6 @@ int z_prf(int (*func)(), void *dest, const char *format, va_list vargs)
if (fplus || fspace || (buf[0] == '-')) { if (fplus || fspace || (buf[0] == '-')) {
prefix = 1; prefix = 1;
} }
need_justifying = true;
break; break;
} }
@ -664,35 +651,29 @@ int z_prf(int (*func)(), void *dest, const char *format, va_list vargs)
*va_arg(vargs, int *) = count; *va_arg(vargs, int *) = count;
break; break;
} }
break; continue;
case 'p': case 'p':
val = (uintptr_t) va_arg(vargs, void *); val = (uintptr_t) va_arg(vargs, void *);
c = _to_hex(buf, val, true, 2*sizeof(void *), 'x'); c = _to_hex(buf, val, true, 2*sizeof(void *), 'x');
need_justifying = true; if (precision >= 0) {
if (precision != -1) {
pad = ' '; pad = ' ';
} }
break; break;
case 's': case 's':
{ cptr = va_arg(vargs, char *);
char *cptr_temp = va_arg(vargs, char *);
/* Get the string length */ /* Get the string length */
for (c = 0; c < MAXFLD; c++) { if (precision < 0) {
if (cptr_temp[c] == '\0') { precision = INT_MAX;
}
for (c = 0; c < precision; c++) {
if (cptr[c] == '\0') {
break; break;
} }
} }
if ((precision >= 0) && (precision < c)) { pad = ' ';
c = precision;
}
if (c > 0) {
memcpy(buf, cptr_temp, (size_t) c);
need_justifying = true;
}
break; break;
}
case 'o': case 'o':
case 'u': case 'u':
@ -726,8 +707,7 @@ int z_prf(int (*func)(), void *dest, const char *format, va_list vargs)
prefix = 2; prefix = 2;
} }
} }
need_justifying = true; if (precision >= 0) {
if (precision != -1) {
pad = ' '; pad = ' ';
} }
break; break;
@ -735,48 +715,48 @@ int z_prf(int (*func)(), void *dest, const char *format, va_list vargs)
case '%': case '%':
PUTC('%'); PUTC('%');
count++; count++;
break; continue;
default: default:
PUTC('%'); PUTC('%');
PUTC(c); PUTC(c);
count += 2; count += 2;
break; continue;
case 0: case 0:
return count; return count;
} }
if (c >= MAXFLD + 1) { /* padding for right justification */
return EOF; if (!fminus && c < width) {
if (pad == ' ') {
prefix = 0;
}
width -= prefix;
c -= prefix;
count += prefix;
while (prefix-- > 0) {
PUTC(*cptr++);
}
width -= c;
count += width;
while (width-- > 0) {
PUTC(pad);
}
} }
if (need_justifying) { /* data out */
if (c < width) { width -= c;
if (fminus) { count += c;
/* Left justify? */ while (c-- > 0) {
for (i = c; i < width; i++) { PUTC(*cptr++);
buf[i] = ' '; }
}
} else {
/* Right justify */
(void) memmove((buf + (width - c)), buf, (size_t) (c
+ 1));
if (pad == ' ') {
prefix = 0;
}
c = width - c + prefix; /* padding for left justification */
for (i = prefix; i < c; i++) { if (width > 0) {
buf[i] = pad; count += width;
} while (width-- > 0) {
} PUTC(' ');
c = width;
}
count += c;
for (cptr = buf; c > 0; c--, cptr++) {
PUTC(*cptr);
} }
} }
} }

19
tests/lib/sprintf/src/main.c
View file

@ -45,8 +45,6 @@
"66666666666666666666666666666666" \ "66666666666666666666666666666666" \
"666666666666666666666666666666666" "666666666666666666666666666666666"
#define PRINTF_MAX_STRING_LENGTH 200
union raw_double_u { union raw_double_u {
double d; double d;
struct { struct {
@ -580,7 +578,6 @@ void test_sprintf_integer(void)
void test_sprintf_string(void) void test_sprintf_string(void)
{ {
int len;
char buffer[400]; char buffer[400];
sprintf(buffer, "%%"); sprintf(buffer, "%%");
@ -596,19 +593,9 @@ void test_sprintf_string(void)
"sprintf(%%s). " "sprintf(%%s). "
"Expected 'short string', got '%s'\n", buffer); "Expected 'short string', got '%s'\n", buffer);
len = sprintf(buffer, "%s", REALLY_LONG_STRING); sprintf(buffer, "%s", REALLY_LONG_STRING);
#if !defined CONFIG_NEWLIB_LIBC && !defined CONFIG_ARCH_POSIX zassert_true((strcmp(buffer, REALLY_LONG_STRING) == 0),
zassert_true((len == PRINTF_MAX_STRING_LENGTH), "sprintf(%%s) of REALLY_LONG_STRING doesn't match!\n");
"Internals changed. "
"Max string length no longer %d got %d\n",
PRINTF_MAX_STRING_LENGTH, len);
#endif
zassert_true((strncmp(buffer, REALLY_LONG_STRING,
PRINTF_MAX_STRING_LENGTH) == 0),
"First %d characters of REALLY_LONG_STRING "
"not printed!\n",
PRINTF_MAX_STRING_LENGTH);
} }
/** /**