From bf4d3d80219777e2bc8597f6543203abf49825ee Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sun, 12 May 2019 21:11:30 +0200 Subject: [PATCH] lib/printf: add extension support Inspired by the Linux kernel, this allows us to do %pI4 and similar things. Signed-off-by: David Lamparter --- lib/printf/glue.c | 95 ++++++++++++++++++++++++++++++++++++++++ lib/printf/printflocal.h | 4 ++ lib/printf/vfprintf.c | 32 ++++++++++++-- lib/printfrr.h | 74 +++++++++++++++++++++++++++---- 4 files changed, 194 insertions(+), 11 deletions(-) diff --git a/lib/printf/glue.c b/lib/printf/glue.c index 6b6e17daff..1b760dc2d3 100644 --- a/lib/printf/glue.c +++ b/lib/printf/glue.c @@ -20,8 +20,10 @@ #include #include +#include #include "printfrr.h" +#include "printflocal.h" ssize_t bprintfrr(struct fbuf *out, const char *fmt, ...) { @@ -152,3 +154,96 @@ char *asprintfrr(struct memtype *mt, const char *fmt, ...) ret = qstrdup(mt, ret); return ret; } + +/* Q: WTF? + * A: since printf should be reasonably fast (think debugging logs), the idea + * here is to keep things close by each other in a cacheline. That's why + * ext_quick just has the first 2 characters of an extension, and we do a + * nice linear continuous sweep. Only if we find something, we go do more + * expensive things. + * + * Q: doesn't this need a mutex/lock? + * A: theoretically, yes, but that's quite expensive and I rather elide that + * necessity by putting down some usage rules. Just call this at startup + * while singlethreaded and all is fine. Ideally, just use constructors + * (and make sure dlopen() doesn't mess things up...) + */ +#define MAXEXT 64 + +struct ext_quick { + char fmt[2]; +}; + +static uint8_t ext_offsets[26] __attribute__((aligned(32))); +static struct ext_quick entries[MAXEXT] __attribute__((aligned(64))); +static const struct printfrr_ext *exts[MAXEXT] __attribute__((aligned(64))); + +void printfrr_ext_reg(const struct printfrr_ext *ext) +{ + uint8_t o; + ptrdiff_t i; + + if (!printfrr_ext_char(ext->match[0])) + return; + + o = ext->match[0] - 'A'; + for (i = ext_offsets[o]; + i < MAXEXT && entries[i].fmt[0] && + memcmp(entries[i].fmt, ext->match, 2) < 0; + i++) + ; + if (i == MAXEXT) + return; + for (o++; o <= 'Z' - 'A'; o++) + ext_offsets[o]++; + + memmove(entries + i + 1, entries + i, + (MAXEXT - i - 1) * sizeof(entries[0])); + memmove(exts + i + 1, exts + i, + (MAXEXT - i - 1) * sizeof(exts[0])); + + memcpy(entries[i].fmt, ext->match, 2); + exts[i] = ext; +} + +ssize_t printfrr_extp(char *buf, size_t sz, const char *fmt, int prec, + const void *ptr) +{ + const struct printfrr_ext *ext; + size_t i; + + for (i = ext_offsets[fmt[0] - 'A']; i < MAXEXT; i++) { + if (!entries[i].fmt[0] || entries[i].fmt[0] > fmt[0]) + return 0; + if (entries[i].fmt[1] && entries[i].fmt[1] != fmt[1]) + continue; + ext = exts[i]; + if (!ext->print_ptr) + continue; + if (strncmp(ext->match, fmt, strlen(ext->match))) + continue; + return ext->print_ptr(buf, sz, fmt, prec, ptr); + } + return 0; +} + +ssize_t printfrr_exti(char *buf, size_t sz, const char *fmt, int prec, + uintmax_t num) +{ + const struct printfrr_ext *ext; + size_t i; + + for (i = ext_offsets[fmt[0] - 'A']; i < MAXEXT; i++) { + if (!entries[i].fmt[0] || entries[i].fmt[0] > fmt[0]) + return 0; + if (entries[i].fmt[1] && entries[i].fmt[1] != fmt[1]) + continue; + ext = exts[i]; + if (!ext->print_int) + continue; + if (strncmp(ext->match, fmt, strlen(ext->match))) + continue; + return ext->print_int(buf, sz, fmt, prec, num); + } + return 0; +} diff --git a/lib/printf/printflocal.h b/lib/printf/printflocal.h index 653baf2a4e..335e09872e 100644 --- a/lib/printf/printflocal.h +++ b/lib/printf/printflocal.h @@ -99,3 +99,7 @@ int _frr_find_arguments(const char *, va_list, union arg **) DSO_LOCAL; #ifdef WCHAR_SUPPORT int _frr_find_warguments(const wchar_t *, va_list, union arg **) DSO_LOCAL; #endif + +/* returns number of bytes consumed for extended specifier */ +ssize_t printfrr_extp(char *, size_t, const char *, int, const void *) DSO_LOCAL; +ssize_t printfrr_exti(char *, size_t, const char *, int, uintmax_t) DSO_LOCAL; diff --git a/lib/printf/vfprintf.c b/lib/printf/vfprintf.c index 3245da483b..07df6dd8fe 100644 --- a/lib/printf/vfprintf.c +++ b/lib/printf/vfprintf.c @@ -162,6 +162,7 @@ vbprintfrr(struct fbuf *cb, const char *fmt0, va_list ap) u_long ulval = 0; /* integer arguments %[diouxX] */ uintmax_t ujval = 0; /* %j, %ll, %q, %t, %z integers */ + void *ptrval; /* %p */ int base; /* base for [diouxX] conversion */ int dprec; /* a copy of prec if [diouxX], 0 otherwise */ int realsz; /* field size expanded by dprec, sign, etc */ @@ -431,14 +432,29 @@ reswitch: switch (ch) { /*FALLTHROUGH*/ case 'd': case 'i': - if (flags & INTMAX_SIZE) { + if (flags & INTMAX_SIZE) ujval = SJARG(); + else + ulval = SARG(); + + if (printfrr_ext_char(fmt[0])) { + n2 = printfrr_exti(buf, sizeof(buf), fmt, prec, + (flags & INTMAX_SIZE) ? ujval + : (uintmax_t)ulval); + if (n2 > 0) { + fmt += n2; + cp = buf; + size = strlen(cp); + sign = '\0'; + break; + } + } + if (flags & INTMAX_SIZE) { if ((intmax_t)ujval < 0) { ujval = -ujval; sign = '-'; } } else { - ulval = SARG(); if ((long)ulval < 0) { ulval = -ulval; sign = '-'; @@ -528,7 +544,17 @@ reswitch: switch (ch) { * defined manner.'' * -- ANSI X3J11 */ - ujval = (uintmax_t)(uintptr_t)GETARG(void *); + ptrval = GETARG(void *); + if (printfrr_ext_char(fmt[0]) && + (n2 = printfrr_extp(buf, sizeof(buf), + fmt, prec, ptrval)) > 0) { + fmt += n2; + cp = buf; + size = strlen(cp); + sign = '\0'; + break; + } + ujval = (uintmax_t)(uintptr_t)ptrval; base = 16; xdigs = xdigs_lower; flags = flags | INTMAXT; diff --git a/lib/printfrr.h b/lib/printfrr.h index f98527efd9..95dace7021 100644 --- a/lib/printfrr.h +++ b/lib/printfrr.h @@ -19,6 +19,7 @@ #include #include +#include #include "compiler.h" #include "memory.h" @@ -75,17 +76,74 @@ char *asnprintfrr(struct memtype *mt, char *out, size_t sz, #undef at #undef atm -struct printfrr_ext { - const char *match; - const char *opts; +/* extension specs must start with a capital letter (this is a restriction + * for both performance's and human understanding's sake.) + * + * Note that the entire thing mostly works because a letter directly following + * a %p print specifier is extremely unlikely to occur (why would you want to + * print "0x12345678HELLO"?) Normally, you'd expect spacing or punctuation + * after a placeholder. That also means that neither of those works well for + * extension purposes, e.g. "%p{foo}" is reasonable to see actually used. + * + * TODO: would be nice to support a "%pF%dF" specifier that consumes 2 + * arguments, e.g. to pass an integer + a list of known values... can be + * done, but a bit tricky. + */ +#define printfrr_ext_char(ch) ((ch) >= 'A' && (ch) <= 'Z') - union { - ssize_t (*print_ptr)(struct fbuf *out, const char *fmt, void *); - ssize_t (*print_int)(struct fbuf *out, const char *fmt, int); - }; +struct printfrr_ext { + /* embedded string to minimize cache line pollution */ + char match[8]; + + /* both can be given, if not the code continues searching + * (you can do %pX and %dX in 2 different entries) + * + * return value: number of bytes consumed from the format string, so + * you can consume extra flags (e.g. register for "%pX", consume + * "%pXfoo" or "%pXbar" for flags.) Convention is to make those flags + * lowercase letters or numbers. + * + * bsz is a compile-time constant in printf; it's gonna be relatively + * small. This isn't designed to print Shakespeare from a pointer. + * + * prec is the precision specifier (the 999 in "%.999p") -1 means + * none given (value in the format string cannot be negative) + */ + ssize_t (*print_ptr)(char *buf, size_t bsz, const char *fmt, int prec, + const void *); + ssize_t (*print_int)(char *buf, size_t bsz, const char *fmt, int prec, + uintmax_t); }; +/* no locking - must be called when single threaded (e.g. at startup.) + * this restriction hopefully won't be a huge bother considering normal usage + * scenarios... + */ void printfrr_ext_reg(const struct printfrr_ext *); -void printfrr_ext_unreg(const struct printfrr_ext *); + +#define printfrr_ext_autoreg_p(matchs, print_fn) \ + static ssize_t print_fn(char *, size_t, const char *, int, \ + const void *); \ + static struct printfrr_ext _printext_##print_fn = { \ + .match = matchs, \ + .print_ptr = print_fn, \ + }; \ + static void _printreg_##print_fn(void) __attribute__((constructor)); \ + static void _printreg_##print_fn(void) { \ + printfrr_ext_reg(&_printext_##print_fn); \ + } \ + /* end */ + +#define printfrr_ext_autoreg_i(matchs, print_fn) \ + static ssize_t print_fn(char *, size_t, const char *, int, uintmax_t); \ + static struct printfrr_ext _printext_##print_fn = { \ + .match = matchs, \ + .print_int = print_fn, \ + }; \ + static void _printreg_##print_fn(void) __attribute__((constructor)); \ + static void _printreg_##print_fn(void) { \ + printfrr_ext_reg(&_printext_##print_fn); \ + } \ + /* end */ #endif -- 2.39.5