From 212e04e5a7f682594dab26cd8354bc4fa040b61f Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 18 Feb 2021 22:52:23 +0100 Subject: lib: rework printfrr extensions to output directly Allowing printfrr extensions to directly write to the output buffer has a few advantages: - there is no arbitrary length limit imposed (previously 64) - the output doesn't need to be copied another time - the extension can directly use bprintfrr() to put together pieces The downside is that the theoretical length (regardless of available buffer space) must be computed correctly. Extended unit tests to test these paths a bit more thoroughly. Signed-off-by: David Lamparter --- lib/printf/glue.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'lib/printf/glue.c') diff --git a/lib/printf/glue.c b/lib/printf/glue.c index 29ca26ad5d..477fc0d384 100644 --- a/lib/printf/glue.c +++ b/lib/printf/glue.c @@ -210,15 +210,16 @@ void printfrr_ext_reg(const struct printfrr_ext *ext) exts[i] = ext; } -ssize_t printfrr_extp(char *buf, size_t sz, const char *fmt, int prec, +ssize_t printfrr_extp(struct fbuf *buf, const char **fmtp, int prec, const void *ptr) { + const char *fmt = *fmtp; 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; + return -1; if (entries[i].fmt[1] && entries[i].fmt[1] != fmt[1]) continue; ext = exts[i]; @@ -226,20 +227,22 @@ ssize_t printfrr_extp(char *buf, size_t sz, const char *fmt, int prec, continue; if (strncmp(ext->match, fmt, strlen(ext->match))) continue; - return ext->print_ptr(buf, sz, fmt, prec, ptr); + *fmtp += strlen(ext->match); + return ext->print_ptr(buf, fmtp, prec, ptr); } - return 0; + return -1; } -ssize_t printfrr_exti(char *buf, size_t sz, const char *fmt, int prec, +ssize_t printfrr_exti(struct fbuf *buf, const char **fmtp, int prec, uintmax_t num) { + const char *fmt = *fmtp; 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; + return -1; if (entries[i].fmt[1] && entries[i].fmt[1] != fmt[1]) continue; ext = exts[i]; @@ -247,7 +250,8 @@ ssize_t printfrr_exti(char *buf, size_t sz, const char *fmt, int prec, continue; if (strncmp(ext->match, fmt, strlen(ext->match))) continue; - return ext->print_int(buf, sz, fmt, prec, num); + *fmtp += strlen(ext->match); + return ext->print_int(buf, fmtp, prec, num); } - return 0; + return -1; } -- cgit v1.2.3 From 3ea794305960ed9fca230c3e0f8b1b73c2915101 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sat, 20 Mar 2021 09:02:04 +0100 Subject: lib: put printfrr extension args into struct ... for easier extensibility. Add width, # and - flags while at it. Signed-off-by: David Lamparter --- bgpd/bgp_table.c | 4 ++-- lib/nexthop.c | 14 ++++++------ lib/prefix.c | 24 ++++++++++---------- lib/printf/glue.c | 16 ++++++------- lib/printf/printflocal.h | 5 +++-- lib/printf/vfprintf.c | 30 +++++++++++++++++++++---- lib/printfrr.h | 58 +++++++++++++++++++++++++++++++++++++++++------- lib/sockunion.c | 8 +++---- lib/srcdest_table.c | 4 ++-- 9 files changed, 114 insertions(+), 49 deletions(-) (limited to 'lib/printf/glue.c') diff --git a/bgpd/bgp_table.c b/bgpd/bgp_table.c index d6dd03e2ac..833bdec2ed 100644 --- a/bgpd/bgp_table.c +++ b/bgpd/bgp_table.c @@ -201,8 +201,8 @@ struct bgp_node *bgp_table_subtree_lookup(const struct bgp_table *table, } printfrr_ext_autoreg_p("BD", printfrr_bd) -static ssize_t printfrr_bd(struct fbuf *buf, const char **fmt, - int prec, const void *ptr) +static ssize_t printfrr_bd(struct fbuf *buf, struct printfrr_eargs *ea, + const void *ptr) { const struct bgp_dest *dest = ptr; const struct prefix *p = bgp_dest_get_prefix(dest); diff --git a/lib/nexthop.c b/lib/nexthop.c index 443df99445..8439398149 100644 --- a/lib/nexthop.c +++ b/lib/nexthop.c @@ -730,22 +730,22 @@ int nexthop_str2backups(const char *str, int *num_backups, * nexthop2str() */ printfrr_ext_autoreg_p("NH", printfrr_nh) -static ssize_t printfrr_nh(struct fbuf *buf, const char **fmt, - int prec, const void *ptr) +static ssize_t printfrr_nh(struct fbuf *buf, struct printfrr_eargs *ea, + const void *ptr) { const struct nexthop *nexthop = ptr; bool do_ifi = false; const char *v_is = "", *v_via = "", *v_viaif = "via "; ssize_t ret = 0; - switch (**fmt) { + switch (*ea->fmt) { case 'v': - (*fmt)++; - if (**fmt == 'v') { + ea->fmt++; + if (*ea->fmt == 'v') { v_is = "is "; v_via = "via "; v_viaif = ""; - (*fmt)++; + ea->fmt++; } if (!nexthop) @@ -796,7 +796,7 @@ static ssize_t printfrr_nh(struct fbuf *buf, const char **fmt, return ret; case 's': - (*fmt)++; + ea->fmt++; if (!nexthop) return bputs(buf, "(null)"); diff --git a/lib/prefix.c b/lib/prefix.c index fba26a5b63..141d564606 100644 --- a/lib/prefix.c +++ b/lib/prefix.c @@ -1361,8 +1361,8 @@ char *evpn_es_df_alg2str(uint8_t df_alg, char *buf, int buf_len) } printfrr_ext_autoreg_p("EA", printfrr_ea) -static ssize_t printfrr_ea(struct fbuf *buf, const char **fmt, - int prec, const void *ptr) +static ssize_t printfrr_ea(struct fbuf *buf, struct printfrr_eargs *ea, + const void *ptr) { const struct ethaddr *mac = ptr; char cbuf[ETHER_ADDR_STRLEN]; @@ -1376,8 +1376,8 @@ static ssize_t printfrr_ea(struct fbuf *buf, const char **fmt, } printfrr_ext_autoreg_p("IA", printfrr_ia) -static ssize_t printfrr_ia(struct fbuf *buf, const char **fmt, - int prec, const void *ptr) +static ssize_t printfrr_ia(struct fbuf *buf, struct printfrr_eargs *ea, + const void *ptr) { const struct ipaddr *ipa = ptr; char cbuf[INET6_ADDRSTRLEN]; @@ -1390,8 +1390,8 @@ static ssize_t printfrr_ia(struct fbuf *buf, const char **fmt, } printfrr_ext_autoreg_p("I4", printfrr_i4) -static ssize_t printfrr_i4(struct fbuf *buf, const char **fmt, - int prec, const void *ptr) +static ssize_t printfrr_i4(struct fbuf *buf, struct printfrr_eargs *ea, + const void *ptr) { char cbuf[INET_ADDRSTRLEN]; @@ -1403,8 +1403,8 @@ static ssize_t printfrr_i4(struct fbuf *buf, const char **fmt, } printfrr_ext_autoreg_p("I6", printfrr_i6) -static ssize_t printfrr_i6(struct fbuf *buf, const char **fmt, - int prec, const void *ptr) +static ssize_t printfrr_i6(struct fbuf *buf, struct printfrr_eargs *ea, + const void *ptr) { char cbuf[INET6_ADDRSTRLEN]; @@ -1416,8 +1416,8 @@ static ssize_t printfrr_i6(struct fbuf *buf, const char **fmt, } printfrr_ext_autoreg_p("FX", printfrr_pfx) -static ssize_t printfrr_pfx(struct fbuf *buf, const char **fmt, - int prec, const void *ptr) +static ssize_t printfrr_pfx(struct fbuf *buf, struct printfrr_eargs *ea, + const void *ptr) { char cbuf[PREFIX_STRLEN]; @@ -1429,8 +1429,8 @@ static ssize_t printfrr_pfx(struct fbuf *buf, const char **fmt, } printfrr_ext_autoreg_p("SG4", printfrr_psg) -static ssize_t printfrr_psg(struct fbuf *buf, const char **fmt, - int prec, const void *ptr) +static ssize_t printfrr_psg(struct fbuf *buf, struct printfrr_eargs *ea, + const void *ptr) { const struct prefix_sg *sg = ptr; ssize_t ret = 0; diff --git a/lib/printf/glue.c b/lib/printf/glue.c index 477fc0d384..389f503c33 100644 --- a/lib/printf/glue.c +++ b/lib/printf/glue.c @@ -210,10 +210,10 @@ void printfrr_ext_reg(const struct printfrr_ext *ext) exts[i] = ext; } -ssize_t printfrr_extp(struct fbuf *buf, const char **fmtp, int prec, +ssize_t printfrr_extp(struct fbuf *buf, struct printfrr_eargs *ea, const void *ptr) { - const char *fmt = *fmtp; + const char *fmt = ea->fmt; const struct printfrr_ext *ext; size_t i; @@ -227,16 +227,16 @@ ssize_t printfrr_extp(struct fbuf *buf, const char **fmtp, int prec, continue; if (strncmp(ext->match, fmt, strlen(ext->match))) continue; - *fmtp += strlen(ext->match); - return ext->print_ptr(buf, fmtp, prec, ptr); + ea->fmt += strlen(ext->match); + return ext->print_ptr(buf, ea, ptr); } return -1; } -ssize_t printfrr_exti(struct fbuf *buf, const char **fmtp, int prec, +ssize_t printfrr_exti(struct fbuf *buf, struct printfrr_eargs *ea, uintmax_t num) { - const char *fmt = *fmtp; + const char *fmt = ea->fmt; const struct printfrr_ext *ext; size_t i; @@ -250,8 +250,8 @@ ssize_t printfrr_exti(struct fbuf *buf, const char **fmtp, int prec, continue; if (strncmp(ext->match, fmt, strlen(ext->match))) continue; - *fmtp += strlen(ext->match); - return ext->print_int(buf, fmtp, prec, num); + ea->fmt += strlen(ext->match); + return ext->print_int(buf, ea, num); } return -1; } diff --git a/lib/printf/printflocal.h b/lib/printf/printflocal.h index df962fc043..bac80e801c 100644 --- a/lib/printf/printflocal.h +++ b/lib/printf/printflocal.h @@ -101,6 +101,7 @@ int _frr_find_warguments(const wchar_t *, va_list, union arg **) DSO_LOCAL; #endif /* returns number of bytes needed for full output, or -1 */ -ssize_t printfrr_extp(struct fbuf *, const char **, int, const void *) +ssize_t printfrr_extp(struct fbuf *, struct printfrr_eargs *ea, const void *) + DSO_LOCAL; +ssize_t printfrr_exti(struct fbuf *, struct printfrr_eargs *ea, uintmax_t) DSO_LOCAL; -ssize_t printfrr_exti(struct fbuf *, const char **, int, uintmax_t) DSO_LOCAL; diff --git a/lib/printf/vfprintf.c b/lib/printf/vfprintf.c index 96675e3620..7fafa89aa7 100644 --- a/lib/printf/vfprintf.c +++ b/lib/printf/vfprintf.c @@ -456,14 +456,25 @@ reswitch: switch (ch) { ulval = SARG(); if (printfrr_ext_char(fmt[0])) { + struct printfrr_eargs ea = { + .fmt = fmt, + .precision = prec, + .width = width, + .alt_repr = !!(flags & ALT), + .leftadj = !!(flags & LADJUST), + }; + if (cb) extstart = cb->pos; - size = printfrr_exti(cb, &fmt, prec, + size = printfrr_exti(cb, &ea, (flags & INTMAX_SIZE) ? ujval : (uintmax_t)ulval); - if (size >= 0) + if (size >= 0) { + fmt = ea.fmt; + width = ea.width; goto ext_printed; + } } if (flags & INTMAX_SIZE) { if ((intmax_t)ujval < 0) { @@ -539,12 +550,23 @@ reswitch: switch (ch) { */ ptrval = GETARG(void *); if (printfrr_ext_char(fmt[0])) { + struct printfrr_eargs ea = { + .fmt = fmt, + .precision = prec, + .width = width, + .alt_repr = !!(flags & ALT), + .leftadj = !!(flags & LADJUST), + }; + if (cb) extstart = cb->pos; - size = printfrr_extp(cb, &fmt, prec, ptrval); - if (size >= 0) + size = printfrr_extp(cb, &ea, ptrval); + if (size >= 0) { + fmt = ea.fmt; + width = ea.width; goto ext_printed; + } } ujval = (uintmax_t)(uintptr_t)ptrval; base = 16; diff --git a/lib/printfrr.h b/lib/printfrr.h index 49243248d6..754cb09598 100644 --- a/lib/printfrr.h +++ b/lib/printfrr.h @@ -112,6 +112,8 @@ char *asnprintfrr(struct memtype *mt, char *out, size_t sz, */ #define printfrr_ext_char(ch) ((ch) >= 'A' && (ch) <= 'Z') +struct printfrr_eargs; + struct printfrr_ext { /* embedded string to minimize cache line pollution */ char match[8]; @@ -127,14 +129,53 @@ struct printfrr_ext { * to consume extra input flags after %pXY, increment *fmt. It points * at the first character after %pXY at entry. Convention is to make * those flags lowercase letters or numbers. + */ + ssize_t (*print_ptr)(struct fbuf *buf, struct printfrr_eargs *info, + const void *); + ssize_t (*print_int)(struct fbuf *buf, struct printfrr_eargs *info, + uintmax_t); +}; + +/* additional information passed to extended formatters */ + +struct printfrr_eargs { + /* position in the format string. Points to directly after the + * extension specifier. Increment when consuming extra "flag + * characters". + */ + const char *fmt; + + /* %.1234x / %.*x + * not POSIX compatible when used with %p, will cause warnings from + * GCC & clang. Usable with %d. Not used by the printfrr() itself + * for extension specifiers, so essentially available as a "free" + * parameter. -1 if not specified. Value in the format string + * cannot be negative, but negative values can be passed with %.*x + */ + int precision; + + /* %1234x / %*x + * regular width specification. Internally handled by printfrr(), set + * to 0 if consumed by the extension in order to suppress standard + * width/padding behavior. 0 if not specified. * - * prec is the precision specifier (the 999 in "%.999p") -1 means - * none given (value in the format string cannot be negative) + * NB: always positive, even if a negative value is passed in with + * %*x. (The sign is used for the - flag.) + */ + int width; + + /* %#x + * "alternate representation" flag, not POSIX compatible when used + * with %p or %d, will cause warnings from GCC & clang. Not used by + * printfrr() itself for extension specifiers. + */ + bool alt_repr; + + /* %-x + * left-pad flag. Internally handled by printfrr() if width is + * nonzero. Only use if the extension sets width to 0. */ - ssize_t (*print_ptr)(struct fbuf *buf, const char **fmt, int prec, - const void *); - ssize_t (*print_int)(struct fbuf *buf, const char **fmt, int prec, - uintmax_t); + bool leftadj; }; /* no locking - must be called when single threaded (e.g. at startup.) @@ -144,7 +185,7 @@ struct printfrr_ext { void printfrr_ext_reg(const struct printfrr_ext *); #define printfrr_ext_autoreg_p(matchs, print_fn) \ - static ssize_t print_fn(struct fbuf *, const char **, int, \ + static ssize_t print_fn(struct fbuf *, struct printfrr_eargs *, \ const void *); \ static const struct printfrr_ext _printext_##print_fn = { \ .match = matchs, \ @@ -157,7 +198,8 @@ void printfrr_ext_reg(const struct printfrr_ext *); /* end */ #define printfrr_ext_autoreg_i(matchs, print_fn) \ - static ssize_t print_fn(struct fbuf *, const char **, int, uintmax_t); \ + static ssize_t print_fn(struct fbuf *, struct printfrr_eargs *, \ + uintmax_t); \ static const struct printfrr_ext _printext_##print_fn = { \ .match = matchs, \ .print_int = print_fn, \ diff --git a/lib/sockunion.c b/lib/sockunion.c index ecbb0fcf91..2175ac3360 100644 --- a/lib/sockunion.c +++ b/lib/sockunion.c @@ -664,8 +664,8 @@ void sockunion_init(union sockunion *su) } printfrr_ext_autoreg_p("SU", printfrr_psu) -static ssize_t printfrr_psu(struct fbuf *buf, const char **fmt, - int prec, const void *ptr) +static ssize_t printfrr_psu(struct fbuf *buf, struct printfrr_eargs *ea, + const void *ptr) { const union sockunion *su = ptr; bool include_port = false; @@ -677,9 +677,9 @@ static ssize_t printfrr_psu(struct fbuf *buf, const char **fmt, return bputs(buf, "(null)"); while (!endflags) { - switch (**fmt) { + switch (*ea->fmt) { case 'p': - (*fmt)++; + ea->fmt++; include_port = true; break; default: diff --git a/lib/srcdest_table.c b/lib/srcdest_table.c index b2422ba460..d2e0682e95 100644 --- a/lib/srcdest_table.c +++ b/lib/srcdest_table.c @@ -307,8 +307,8 @@ const char *srcdest_rnode2str(const struct route_node *rn, char *str, int size) } printfrr_ext_autoreg_p("RN", printfrr_rn) -static ssize_t printfrr_rn(struct fbuf *buf, const char **fmt, - int prec, const void *ptr) +static ssize_t printfrr_rn(struct fbuf *buf, struct printfrr_eargs *ea, + const void *ptr) { const struct route_node *rn = ptr; const struct prefix *dst_p, *src_p; -- cgit v1.2.3 From 9c4380daee0e495ea63d161af61d7f7e70c9d9ea Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 2 Mar 2021 20:45:57 +0100 Subject: lib: add `%pVA` recursive printfrr Analogous to Linux kernel `%pV` (but our mechanism expects 2 specifier chars and `%pVA` is clearer anyway.) Signed-off-by: David Lamparter --- doc/developer/logging.rst | 2 ++ lib/printf/glue.c | 18 ++++++++++++++++++ lib/printfrr.h | 11 +++++++++++ tests/lib/test_printfrr.c | 17 +++++++++++++++++ 4 files changed, 48 insertions(+) (limited to 'lib/printf/glue.c') diff --git a/doc/developer/logging.rst b/doc/developer/logging.rst index a35e60619c..5e8cb79cc2 100644 --- a/doc/developer/logging.rst +++ b/doc/developer/logging.rst @@ -65,6 +65,8 @@ Extensions +-----------+--------------------------+----------------------------------------------+ | ``%Ld`` | ``int64_t`` | ``-12345`` | +-----------+--------------------------+----------------------------------------------+ +| ``%pVA`` | ``struct va_format *`` | (recursive printfrr) | ++-----------+--------------------------+----------------------------------------------+ | ``%pI4`` | ``struct in_addr *`` | ``1.2.3.4`` | | | | | | | ``in_addr_t *`` | | diff --git a/lib/printf/glue.c b/lib/printf/glue.c index 389f503c33..2c97dd639e 100644 --- a/lib/printf/glue.c +++ b/lib/printf/glue.c @@ -255,3 +255,21 @@ ssize_t printfrr_exti(struct fbuf *buf, struct printfrr_eargs *ea, } return -1; } + +printfrr_ext_autoreg_p("VA", printfrr_va) +static ssize_t printfrr_va(struct fbuf *buf, struct printfrr_eargs *ea, + const void *ptr) +{ + const struct va_format *vaf = ptr; + va_list ap; + + if (!vaf || !vaf->fmt || !vaf->va) + return bputs(buf, "NULL"); + + /* make sure we don't alter the data passed in - especially since + * bprintfrr (and thus this) might be called on the same format twice, + * when allocating a larger buffer in asnprintfrr() + */ + va_copy(ap, *vaf->va); + return vbprintfrr(buf, vaf->fmt, ap); +} diff --git a/lib/printfrr.h b/lib/printfrr.h index 8245a664b3..6ca4d963c4 100644 --- a/lib/printfrr.h +++ b/lib/printfrr.h @@ -251,6 +251,17 @@ static inline ssize_t bputch(struct fbuf *buf, char ch) return 1; } +/* %pVA extension, equivalent to Linux kernel %pV */ + +struct va_format { + const char *fmt; + va_list *va; +}; + +#ifdef _FRR_ATTRIBUTE_PRINTFRR +#pragma FRR printfrr_ext "%pVA" (struct va_format *) +#endif + /* when using non-ISO-C compatible extension specifiers... */ #ifdef _FRR_ATTRIBUTE_PRINTFRR diff --git a/tests/lib/test_printfrr.c b/tests/lib/test_printfrr.c index 5a8d10e9b7..8fa8bfcc23 100644 --- a/tests/lib/test_printfrr.c +++ b/tests/lib/test_printfrr.c @@ -128,6 +128,21 @@ static int printchk(const char *ref, const char *fmt, ...) return 0; } +static void test_va(const char *ref, const char *fmt, ...) PRINTFRR(2, 3); +static void test_va(const char *ref, const char *fmt, ...) +{ + struct va_format vaf; + va_list ap; + + va_start(ap, fmt); + vaf.fmt = fmt; + vaf.va = ≈ + + printchk(ref, "VA [%pVA] %s", &vaf, "--"); + + va_end(ap); +} + int main(int argc, char **argv) { size_t i; @@ -168,6 +183,8 @@ int main(int argc, char **argv) printcmp("%p", &ip); + test_va("VA [192.168.1.2 1234] --", "%pI4 %u", &ip, 1234); + snprintfrr(buf, sizeof(buf), "test%s", "#1"); csnprintfrr(buf, sizeof(buf), "test%s", "#2"); assert(strcmp(buf, "test#1test#2") == 0); -- cgit v1.2.3 From bb12115e0be449df92af6294fc8410eb7745be26 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 2 Mar 2021 21:39:49 +0100 Subject: lib: add `%pFB` extension to print struct fbuf * Useful to insert output from another bprintfrr() call. Signed-off-by: David Lamparter --- lib/printf/glue.c | 22 ++++++++++++++++++++++ lib/printfrr.h | 1 + 2 files changed, 23 insertions(+) (limited to 'lib/printf/glue.c') diff --git a/lib/printf/glue.c b/lib/printf/glue.c index 2c97dd639e..1147901236 100644 --- a/lib/printf/glue.c +++ b/lib/printf/glue.c @@ -256,6 +256,28 @@ ssize_t printfrr_exti(struct fbuf *buf, struct printfrr_eargs *ea, return -1; } +printfrr_ext_autoreg_p("FB", printfrr_fb) +static ssize_t printfrr_fb(struct fbuf *out, struct printfrr_eargs *ea, + const void *ptr) +{ + const struct fbuf *in = ptr; + ptrdiff_t copy_len; + + if (!in) + return bputs(out, "NULL"); + + if (out) { + copy_len = MIN(in->pos - in->buf, + out->buf + out->len - out->pos); + if (copy_len > 0) { + memcpy(out->pos, in->buf, copy_len); + out->pos += copy_len; + } + } + + return in->pos - in->buf; +} + printfrr_ext_autoreg_p("VA", printfrr_va) static ssize_t printfrr_va(struct fbuf *buf, struct printfrr_eargs *ea, const void *ptr) diff --git a/lib/printfrr.h b/lib/printfrr.h index 6ca4d963c4..8ea8fd69a7 100644 --- a/lib/printfrr.h +++ b/lib/printfrr.h @@ -259,6 +259,7 @@ struct va_format { }; #ifdef _FRR_ATTRIBUTE_PRINTFRR +#pragma FRR printfrr_ext "%pFB" (struct fbuf *) #pragma FRR printfrr_ext "%pVA" (struct va_format *) #endif -- cgit v1.2.3