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/srcdest_table.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) (limited to 'lib/srcdest_table.c') diff --git a/lib/srcdest_table.c b/lib/srcdest_table.c index a115507192..ca48d56e4d 100644 --- a/lib/srcdest_table.c +++ b/lib/srcdest_table.c @@ -307,20 +307,20 @@ 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(char *buf, size_t bsz, const char *fmt, +static ssize_t printfrr_rn(struct fbuf *buf, const char **fmt, int prec, const void *ptr) { const struct route_node *rn = ptr; const struct prefix *dst_p, *src_p; + char cbuf[PREFIX_STRLEN * 2 + 6]; - if (rn) { - srcdest_rnode_prefixes(rn, &dst_p, &src_p); - srcdest2str(dst_p, (const struct prefix_ipv6 *)src_p, buf, bsz); - } else { - strlcpy(buf, "NULL", bsz); - } + if (!rn) + return bputs(buf, "NULL"); - return 2; + srcdest_rnode_prefixes(rn, &dst_p, &src_p); + srcdest2str(dst_p, (const struct prefix_ipv6 *)src_p, + cbuf, sizeof(cbuf)); + return bputs(buf, cbuf); } struct route_table *srcdest_srcnode_table(struct route_node *rn) -- cgit v1.2.3 From eba599a39756b3d9424467f1e1f4f4475dffad17 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 26 Mar 2021 19:14:24 +0100 Subject: lib: print `(null)` rather than `NULL` ... for consistency with `%s`, which also prints `(null)`. Signed-off-by: David Lamparter --- bgpd/bgp_table.c | 2 +- lib/nexthop.c | 4 ++-- lib/prefix.c | 12 ++++++------ lib/sockunion.c | 2 +- lib/srcdest_table.c | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) (limited to 'lib/srcdest_table.c') diff --git a/bgpd/bgp_table.c b/bgpd/bgp_table.c index 50123c608b..d6dd03e2ac 100644 --- a/bgpd/bgp_table.c +++ b/bgpd/bgp_table.c @@ -209,7 +209,7 @@ static ssize_t printfrr_bd(struct fbuf *buf, const char **fmt, char cbuf[PREFIX_STRLEN]; if (!dest) - return bputs(buf, "NULL"); + return bputs(buf, "(null)"); /* need to get the real length even if buffer too small */ prefix2str(p, cbuf, sizeof(cbuf)); diff --git a/lib/nexthop.c b/lib/nexthop.c index 64b4f3adb1..443df99445 100644 --- a/lib/nexthop.c +++ b/lib/nexthop.c @@ -749,7 +749,7 @@ static ssize_t printfrr_nh(struct fbuf *buf, const char **fmt, } if (!nexthop) - return bputs(buf, "NULL"); + return bputs(buf, "(null)"); switch (nexthop->type) { case NEXTHOP_TYPE_IPV4: @@ -799,7 +799,7 @@ static ssize_t printfrr_nh(struct fbuf *buf, const char **fmt, (*fmt)++; if (!nexthop) - return bputs(buf, "NULL"); + return bputs(buf, "(null)"); switch (nexthop->type) { case NEXTHOP_TYPE_IFINDEX: diff --git a/lib/prefix.c b/lib/prefix.c index 169e9ed572..fba26a5b63 100644 --- a/lib/prefix.c +++ b/lib/prefix.c @@ -1368,7 +1368,7 @@ static ssize_t printfrr_ea(struct fbuf *buf, const char **fmt, char cbuf[ETHER_ADDR_STRLEN]; if (!mac) - return bputs(buf, "NULL"); + return bputs(buf, "(null)"); /* need real length even if buffer is too short */ prefix_mac2str(mac, cbuf, sizeof(cbuf)); @@ -1383,7 +1383,7 @@ static ssize_t printfrr_ia(struct fbuf *buf, const char **fmt, char cbuf[INET6_ADDRSTRLEN]; if (!ipa) - return bputs(buf, "NULL"); + return bputs(buf, "(null)"); ipaddr2str(ipa, cbuf, sizeof(cbuf)); return bputs(buf, cbuf); @@ -1396,7 +1396,7 @@ static ssize_t printfrr_i4(struct fbuf *buf, const char **fmt, char cbuf[INET_ADDRSTRLEN]; if (!ptr) - return bputs(buf, "NULL"); + return bputs(buf, "(null)"); inet_ntop(AF_INET, ptr, cbuf, sizeof(cbuf)); return bputs(buf, cbuf); @@ -1409,7 +1409,7 @@ static ssize_t printfrr_i6(struct fbuf *buf, const char **fmt, char cbuf[INET6_ADDRSTRLEN]; if (!ptr) - return bputs(buf, "NULL"); + return bputs(buf, "(null)"); inet_ntop(AF_INET6, ptr, cbuf, sizeof(cbuf)); return bputs(buf, cbuf); @@ -1422,7 +1422,7 @@ static ssize_t printfrr_pfx(struct fbuf *buf, const char **fmt, char cbuf[PREFIX_STRLEN]; if (!ptr) - return bputs(buf, "NULL"); + return bputs(buf, "(null)"); prefix2str(ptr, cbuf, sizeof(cbuf)); return bputs(buf, cbuf); @@ -1436,7 +1436,7 @@ static ssize_t printfrr_psg(struct fbuf *buf, const char **fmt, ssize_t ret = 0; if (!sg) - return bputs(buf, "NULL"); + return bputs(buf, "(null)"); if (sg->src.s_addr == INADDR_ANY) ret += bputs(buf, "(*,"); diff --git a/lib/sockunion.c b/lib/sockunion.c index 25de8f7dd0..ecbb0fcf91 100644 --- a/lib/sockunion.c +++ b/lib/sockunion.c @@ -674,7 +674,7 @@ static ssize_t printfrr_psu(struct fbuf *buf, const char **fmt, char cbuf[INET6_ADDRSTRLEN]; if (!su) - return bputs(buf, "NULL"); + return bputs(buf, "(null)"); while (!endflags) { switch (**fmt) { diff --git a/lib/srcdest_table.c b/lib/srcdest_table.c index ca48d56e4d..b2422ba460 100644 --- a/lib/srcdest_table.c +++ b/lib/srcdest_table.c @@ -315,7 +315,7 @@ static ssize_t printfrr_rn(struct fbuf *buf, const char **fmt, char cbuf[PREFIX_STRLEN * 2 + 6]; if (!rn) - return bputs(buf, "NULL"); + return bputs(buf, "(null)"); srcdest_rnode_prefixes(rn, &dst_p, &src_p); srcdest2str(dst_p, (const struct prefix_ipv6 *)src_p, -- 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/srcdest_table.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