| Message ID | 20220506205605.359830-10-nikos.nikoleris@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | EFI and ACPI support for arm64 | expand |
On Fri, May 06, 2022 at 09:55:51PM +0100, Nikos Nikoleris wrote: > This follows the typical format of: > > printf("%.Ns", *str); > > Where N might be a decimal digit string or '*'. This feature is used > by a future change. > > See also: man 3 printf > > Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com> > --- > lib/printf.c | 84 ++++++++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 71 insertions(+), 13 deletions(-) > > diff --git a/lib/printf.c b/lib/printf.c > index 1269723..724befa 100644 > --- a/lib/printf.c > +++ b/lib/printf.c > @@ -19,6 +19,7 @@ typedef struct strprops { > char pad; > int npad; > bool alternate; > + int precision; > } strprops_t; > > static void addchar(pstream_t *p, char c) > @@ -43,7 +44,7 @@ static void print_str(pstream_t *p, const char *s, strprops_t props) > } > } > > - while (*s) > + while (*s && props.precision--) > addchar(p, *s++); > > if (npad < 0) { > @@ -147,9 +148,61 @@ static int fmtnum(const char **fmt) > return num; > } > > +static inline int isdigit(int c) > +{ > + return '0' <= c && c <= '9'; > +} We desperately need to add ctype to our library. We've already got isblank, isalpha, and isalnum local to argv.c and I see later you add isspace. I'll post a patch now that introduces ctype.[ch] with the ones used by argv. Then, when you respin this series you can add your ctype functions there. > + > +/* > + * Adapted from drivers/firmware/efi/libstub/vsprintf.c > + */ > +static int skip_atoi(const char **s) > +{ > + int i = 0; > + > + do { > + i = i*10 + *((*s)++) - '0'; > + } while (isdigit(**s)); > + > + return i; > +} > + > +/* > + * Adapted from drivers/firmware/efi/libstub/vsprintf.c > + */ > +static int get_int(const char **fmt, va_list *ap) > +{ > + if (isdigit(**fmt)) { > + return skip_atoi(fmt); > + } > + if (**fmt == '*') { > + ++(*fmt); > + /* it's the next argument */ > + return va_arg(*ap, int); > + } > + return 0; > +} > + > int vsnprintf(char *buf, int size, const char *fmt, va_list va) > { > pstream_t s; > + va_list args; > + > + /* > + * We want to pass our input va_list to helper functions by reference, > + * but there's an annoying edge case. If va_list was originally passed > + * to us by value, we could just pass &ap down to the helpers. This is > + * the case on, for example, X86_32. > + * However, on X86_64 (and possibly others), va_list is actually a > + * size-1 array containing a structure. Our function parameter ap has > + * decayed from T[1] to T*, and &ap has type T** rather than T(*)[1], > + * which is what will be expected by a function taking a va_list * > + * parameter. > + * One standard way to solve this mess is by creating a copy in a local > + * variable of type va_list and then passing a pointer to that local > + * copy instead, which is what we do here. > + */ > + va_copy(args, va); > > s.buffer = buf; > s.remain = size - 1; > @@ -160,6 +213,7 @@ int vsnprintf(char *buf, int size, const char *fmt, va_list va) > strprops_t props; > memset(&props, 0, sizeof(props)); > props.pad = ' '; > + props.precision = -1; > > if (f != '%') { > addchar(&s, f); > @@ -172,11 +226,14 @@ int vsnprintf(char *buf, int size, const char *fmt, va_list va) > addchar(&s, '%'); > break; > case 'c': > - addchar(&s, va_arg(va, int)); > + addchar(&s, va_arg(args, int)); > break; > case '\0': > --fmt; > break; > + case '.': > + props.precision = get_int(&fmt, &args); > + goto morefmt; > case '#': > props.alternate = true; > goto morefmt; > @@ -204,54 +261,55 @@ int vsnprintf(char *buf, int size, const char *fmt, va_list va) > case 'd': > switch (nlong) { > case 0: > - print_int(&s, va_arg(va, int), 10, props); > + print_int(&s, va_arg(args, int), 10, props); > break; > case 1: > - print_int(&s, va_arg(va, long), 10, props); > + print_int(&s, va_arg(args, long), 10, props); > break; > default: > - print_int(&s, va_arg(va, long long), 10, props); > + print_int(&s, va_arg(args, long long), 10, props); > break; > } > break; > case 'u': > switch (nlong) { > case 0: > - print_unsigned(&s, va_arg(va, unsigned), 10, props); > + print_unsigned(&s, va_arg(args, unsigned), 10, props); > break; > case 1: > - print_unsigned(&s, va_arg(va, unsigned long), 10, props); > + print_unsigned(&s, va_arg(args, unsigned long), 10, props); > break; > default: > - print_unsigned(&s, va_arg(va, unsigned long long), 10, props); > + print_unsigned(&s, va_arg(args, unsigned long long), 10, props); > break; > } > break; > case 'x': > switch (nlong) { > case 0: > - print_unsigned(&s, va_arg(va, unsigned), 16, props); > + print_unsigned(&s, va_arg(args, unsigned), 16, props); > break; > case 1: > - print_unsigned(&s, va_arg(va, unsigned long), 16, props); > + print_unsigned(&s, va_arg(args, unsigned long), 16, props); > break; > default: > - print_unsigned(&s, va_arg(va, unsigned long long), 16, props); > + print_unsigned(&s, va_arg(args, unsigned long long), 16, props); > break; > } > break; > case 'p': > props.alternate = true; > - print_unsigned(&s, (unsigned long)va_arg(va, void *), 16, props); > + print_unsigned(&s, (unsigned long)va_arg(args, void *), 16, props); > break; > case 's': > - print_str(&s, va_arg(va, const char *), props); > + print_str(&s, va_arg(args, const char *), props); > break; > default: > addchar(&s, f); > break; > } > } > + va_end(args); > *s.buffer = 0; > return s.added; > } > -- > 2.25.1 > I think I should also post a patches that finally reformat these older files. The tab+4spaces stuff must go! When we get ctype we'll want to move out isdigit, but otherwise Reviewed-by: Andrew Jones <drjones@redhat.com> Thanks, drew
On Thu, May 19, 2022 at 04:52:33PM +0200, Andrew Jones wrote: > On Fri, May 06, 2022 at 09:55:51PM +0100, Nikos Nikoleris wrote: > > This follows the typical format of: > > > > printf("%.Ns", *str); > > > > Where N might be a decimal digit string or '*'. This feature is used > > by a future change. > > > > See also: man 3 printf > > > > Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com> > > --- > > lib/printf.c | 84 ++++++++++++++++++++++++++++++++++++++++++++-------- > > 1 file changed, 71 insertions(+), 13 deletions(-) > > > > diff --git a/lib/printf.c b/lib/printf.c > > index 1269723..724befa 100644 > > --- a/lib/printf.c > > +++ b/lib/printf.c > > @@ -19,6 +19,7 @@ typedef struct strprops { > > char pad; > > int npad; > > bool alternate; > > + int precision; > > } strprops_t; > > > > static void addchar(pstream_t *p, char c) > > @@ -43,7 +44,7 @@ static void print_str(pstream_t *p, const char *s, strprops_t props) > > } > > } > > > > - while (*s) > > + while (*s && props.precision--) > > addchar(p, *s++); > > > > if (npad < 0) { > > @@ -147,9 +148,61 @@ static int fmtnum(const char **fmt) > > return num; > > } > > > > +static inline int isdigit(int c) > > +{ > > + return '0' <= c && c <= '9'; > > +} > > We desperately need to add ctype to our library. We've already got > isblank, isalpha, and isalnum local to argv.c and I see later you > add isspace. I'll post a patch now that introduces ctype.[ch] with > the ones used by argv. Then, when you respin this series you can > add your ctype functions there. > > > + > > +/* > > + * Adapted from drivers/firmware/efi/libstub/vsprintf.c > > + */ > > +static int skip_atoi(const char **s) > > +{ > > + int i = 0; > > + > > + do { > > + i = i*10 + *((*s)++) - '0'; > > + } while (isdigit(**s)); > > + > > + return i; > > +} > > + > > +/* > > + * Adapted from drivers/firmware/efi/libstub/vsprintf.c > > + */ > > +static int get_int(const char **fmt, va_list *ap) > > +{ > > + if (isdigit(**fmt)) { > > + return skip_atoi(fmt); > > + } > > + if (**fmt == '*') { > > + ++(*fmt); > > + /* it's the next argument */ > > + return va_arg(*ap, int); > > + } > > + return 0; > > +} > > + > > int vsnprintf(char *buf, int size, const char *fmt, va_list va) > > { > > pstream_t s; > > + va_list args; > > + > > + /* > > + * We want to pass our input va_list to helper functions by reference, > > + * but there's an annoying edge case. If va_list was originally passed > > + * to us by value, we could just pass &ap down to the helpers. This is > > + * the case on, for example, X86_32. > > + * However, on X86_64 (and possibly others), va_list is actually a > > + * size-1 array containing a structure. Our function parameter ap has > > + * decayed from T[1] to T*, and &ap has type T** rather than T(*)[1], > > + * which is what will be expected by a function taking a va_list * > > + * parameter. > > + * One standard way to solve this mess is by creating a copy in a local > > + * variable of type va_list and then passing a pointer to that local > > + * copy instead, which is what we do here. > > + */ > > + va_copy(args, va); > > > > s.buffer = buf; > > s.remain = size - 1; > > @@ -160,6 +213,7 @@ int vsnprintf(char *buf, int size, const char *fmt, va_list va) > > strprops_t props; > > memset(&props, 0, sizeof(props)); > > props.pad = ' '; > > + props.precision = -1; > > > > if (f != '%') { > > addchar(&s, f); > > @@ -172,11 +226,14 @@ int vsnprintf(char *buf, int size, const char *fmt, va_list va) > > addchar(&s, '%'); > > break; > > case 'c': > > - addchar(&s, va_arg(va, int)); > > + addchar(&s, va_arg(args, int)); > > break; > > case '\0': > > --fmt; > > break; > > + case '.': > > + props.precision = get_int(&fmt, &args); > > + goto morefmt; > > case '#': > > props.alternate = true; > > goto morefmt; > > @@ -204,54 +261,55 @@ int vsnprintf(char *buf, int size, const char *fmt, va_list va) > > case 'd': > > switch (nlong) { > > case 0: > > - print_int(&s, va_arg(va, int), 10, props); > > + print_int(&s, va_arg(args, int), 10, props); > > break; > > case 1: > > - print_int(&s, va_arg(va, long), 10, props); > > + print_int(&s, va_arg(args, long), 10, props); > > break; > > default: > > - print_int(&s, va_arg(va, long long), 10, props); > > + print_int(&s, va_arg(args, long long), 10, props); > > break; > > } > > break; > > case 'u': > > switch (nlong) { > > case 0: > > - print_unsigned(&s, va_arg(va, unsigned), 10, props); > > + print_unsigned(&s, va_arg(args, unsigned), 10, props); > > break; > > case 1: > > - print_unsigned(&s, va_arg(va, unsigned long), 10, props); > > + print_unsigned(&s, va_arg(args, unsigned long), 10, props); > > break; > > default: > > - print_unsigned(&s, va_arg(va, unsigned long long), 10, props); > > + print_unsigned(&s, va_arg(args, unsigned long long), 10, props); > > break; > > } > > break; > > case 'x': > > switch (nlong) { > > case 0: > > - print_unsigned(&s, va_arg(va, unsigned), 16, props); > > + print_unsigned(&s, va_arg(args, unsigned), 16, props); > > break; > > case 1: > > - print_unsigned(&s, va_arg(va, unsigned long), 16, props); > > + print_unsigned(&s, va_arg(args, unsigned long), 16, props); > > break; > > default: > > - print_unsigned(&s, va_arg(va, unsigned long long), 16, props); > > + print_unsigned(&s, va_arg(args, unsigned long long), 16, props); > > break; > > } > > break; > > case 'p': > > props.alternate = true; > > - print_unsigned(&s, (unsigned long)va_arg(va, void *), 16, props); > > + print_unsigned(&s, (unsigned long)va_arg(args, void *), 16, props); > > break; > > case 's': > > - print_str(&s, va_arg(va, const char *), props); > > + print_str(&s, va_arg(args, const char *), props); > > break; > > default: > > addchar(&s, f); > > break; > > } > > } > > + va_end(args); > > *s.buffer = 0; > > return s.added; > > } > > -- > > 2.25.1 > > > > I think I should also post a patches that finally reformat these older > files. The tab+4spaces stuff must go! > > When we get ctype we'll want to move out isdigit, but otherwise > > Reviewed-by: Andrew Jones <drjones@redhat.com> Actually, I'd prefer we make the feature work correctly for %d/%x/%# from the beginning. It probably wouldn't take much to ensure precision acts as a minimum (with zero padding when necessary) for numbers. Thanks, drew
diff --git a/lib/printf.c b/lib/printf.c index 1269723..724befa 100644 --- a/lib/printf.c +++ b/lib/printf.c @@ -19,6 +19,7 @@ typedef struct strprops { char pad; int npad; bool alternate; + int precision; } strprops_t; static void addchar(pstream_t *p, char c) @@ -43,7 +44,7 @@ static void print_str(pstream_t *p, const char *s, strprops_t props) } } - while (*s) + while (*s && props.precision--) addchar(p, *s++); if (npad < 0) { @@ -147,9 +148,61 @@ static int fmtnum(const char **fmt) return num; } +static inline int isdigit(int c) +{ + return '0' <= c && c <= '9'; +} + +/* + * Adapted from drivers/firmware/efi/libstub/vsprintf.c + */ +static int skip_atoi(const char **s) +{ + int i = 0; + + do { + i = i*10 + *((*s)++) - '0'; + } while (isdigit(**s)); + + return i; +} + +/* + * Adapted from drivers/firmware/efi/libstub/vsprintf.c + */ +static int get_int(const char **fmt, va_list *ap) +{ + if (isdigit(**fmt)) { + return skip_atoi(fmt); + } + if (**fmt == '*') { + ++(*fmt); + /* it's the next argument */ + return va_arg(*ap, int); + } + return 0; +} + int vsnprintf(char *buf, int size, const char *fmt, va_list va) { pstream_t s; + va_list args; + + /* + * We want to pass our input va_list to helper functions by reference, + * but there's an annoying edge case. If va_list was originally passed + * to us by value, we could just pass &ap down to the helpers. This is + * the case on, for example, X86_32. + * However, on X86_64 (and possibly others), va_list is actually a + * size-1 array containing a structure. Our function parameter ap has + * decayed from T[1] to T*, and &ap has type T** rather than T(*)[1], + * which is what will be expected by a function taking a va_list * + * parameter. + * One standard way to solve this mess is by creating a copy in a local + * variable of type va_list and then passing a pointer to that local + * copy instead, which is what we do here. + */ + va_copy(args, va); s.buffer = buf; s.remain = size - 1; @@ -160,6 +213,7 @@ int vsnprintf(char *buf, int size, const char *fmt, va_list va) strprops_t props; memset(&props, 0, sizeof(props)); props.pad = ' '; + props.precision = -1; if (f != '%') { addchar(&s, f); @@ -172,11 +226,14 @@ int vsnprintf(char *buf, int size, const char *fmt, va_list va) addchar(&s, '%'); break; case 'c': - addchar(&s, va_arg(va, int)); + addchar(&s, va_arg(args, int)); break; case '\0': --fmt; break; + case '.': + props.precision = get_int(&fmt, &args); + goto morefmt; case '#': props.alternate = true; goto morefmt; @@ -204,54 +261,55 @@ int vsnprintf(char *buf, int size, const char *fmt, va_list va) case 'd': switch (nlong) { case 0: - print_int(&s, va_arg(va, int), 10, props); + print_int(&s, va_arg(args, int), 10, props); break; case 1: - print_int(&s, va_arg(va, long), 10, props); + print_int(&s, va_arg(args, long), 10, props); break; default: - print_int(&s, va_arg(va, long long), 10, props); + print_int(&s, va_arg(args, long long), 10, props); break; } break; case 'u': switch (nlong) { case 0: - print_unsigned(&s, va_arg(va, unsigned), 10, props); + print_unsigned(&s, va_arg(args, unsigned), 10, props); break; case 1: - print_unsigned(&s, va_arg(va, unsigned long), 10, props); + print_unsigned(&s, va_arg(args, unsigned long), 10, props); break; default: - print_unsigned(&s, va_arg(va, unsigned long long), 10, props); + print_unsigned(&s, va_arg(args, unsigned long long), 10, props); break; } break; case 'x': switch (nlong) { case 0: - print_unsigned(&s, va_arg(va, unsigned), 16, props); + print_unsigned(&s, va_arg(args, unsigned), 16, props); break; case 1: - print_unsigned(&s, va_arg(va, unsigned long), 16, props); + print_unsigned(&s, va_arg(args, unsigned long), 16, props); break; default: - print_unsigned(&s, va_arg(va, unsigned long long), 16, props); + print_unsigned(&s, va_arg(args, unsigned long long), 16, props); break; } break; case 'p': props.alternate = true; - print_unsigned(&s, (unsigned long)va_arg(va, void *), 16, props); + print_unsigned(&s, (unsigned long)va_arg(args, void *), 16, props); break; case 's': - print_str(&s, va_arg(va, const char *), props); + print_str(&s, va_arg(args, const char *), props); break; default: addchar(&s, f); break; } } + va_end(args); *s.buffer = 0; return s.added; }
This follows the typical format of: printf("%.Ns", *str); Where N might be a decimal digit string or '*'. This feature is used by a future change. See also: man 3 printf Signed-off-by: Nikos Nikoleris <nikos.nikoleris@arm.com> --- lib/printf.c | 84 ++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 71 insertions(+), 13 deletions(-)