From 23922bbc0821270ad09bcffdf7c16b2fb4984d6e Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 19 Feb 2021 00:04:51 +0100 Subject: [PATCH 01/16] tools/gcc-plugin: fix format precision/width type `%*.*pEXT` applied the extension type to the precision and width (*.*) too. Oops. Signed-off-by: David Lamparter --- tools/gcc-plugins/frr-format.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/gcc-plugins/frr-format.c b/tools/gcc-plugins/frr-format.c index 6d91d2cdcd..efb2c6393c 100644 --- a/tools/gcc-plugins/frr-format.c +++ b/tools/gcc-plugins/frr-format.c @@ -2343,7 +2343,7 @@ check_argument_type (const format_char_info *fci, /* note printf extension type checks are *additional* - %p must always * be pointer compatible, %d always int compatible. */ - if (!kef) + if (first_wanted_type->kind != CF_KIND_FORMAT || !kef) return true; const struct kernel_ext_fmt *kef_now; From bcf9d7d8aa77c66ac2e99f75ae11e276accecb1d Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 19 Feb 2021 00:05:35 +0100 Subject: [PATCH 02/16] tools/gcc-plugin: support [un]signed in pragma Need `unsigned char *` for `%pHX`. Signed-off-by: David Lamparter --- tools/gcc-plugins/frr-format.c | 17 +++++++++++++++-- tools/gcc-plugins/frr-format.h | 2 ++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/tools/gcc-plugins/frr-format.c b/tools/gcc-plugins/frr-format.c index efb2c6393c..e9f397f225 100644 --- a/tools/gcc-plugins/frr-format.c +++ b/tools/gcc-plugins/frr-format.c @@ -4241,6 +4241,11 @@ handle_finish_parse (void *event_data, void *data) continue; } node = TREE_TYPE (node); + + if (etab->t_unsigned) + node = c_common_unsigned_type (node); + else if (etab->t_signed) + node = c_common_signed_type (node); } etab->type = node; @@ -4357,9 +4362,17 @@ handle_pragma_printfrr_ext (cpp_reader *dummy) ttype = pragma_lex (&token, &loc); /* qualifiers */ - if (ttype == CPP_NAME && !strcmp (IDENTIFIER_POINTER (token), "const")) + while (ttype == CPP_NAME) { - etab->t_const = true; + if (!strcmp (IDENTIFIER_POINTER (token), "const")) + etab->t_const = true; + else if (!strcmp (IDENTIFIER_POINTER (token), "signed")) + etab->t_signed = true; + else if (!strcmp (IDENTIFIER_POINTER (token), "unsigned")) + etab->t_unsigned = true; + else + break; + ttype = pragma_lex (&token, &loc); } diff --git a/tools/gcc-plugins/frr-format.h b/tools/gcc-plugins/frr-format.h index 87d2049ed4..599dbc56f9 100644 --- a/tools/gcc-plugins/frr-format.h +++ b/tools/gcc-plugins/frr-format.h @@ -113,6 +113,8 @@ struct kernel_ext_fmt tree_code type_code; int ptrlevel; bool t_const; + bool t_unsigned; + bool t_signed; bool warned; const char *type_str; From 212e04e5a7f682594dab26cd8354bc4fa040b61f Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Thu, 18 Feb 2021 22:52:23 +0100 Subject: [PATCH 03/16] 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 --- bgpd/bgp_table.c | 17 ++++--- lib/nexthop.c | 87 ++++++++++++++++++++++-------------- lib/prefix.c | 94 +++++++++++++++++++-------------------- lib/printf/glue.c | 20 +++++---- lib/printf/printflocal.h | 7 +-- lib/printf/vfprintf.c | 84 +++++++++++++++++++++++++++------- lib/printfrr.h | 21 ++++----- lib/sockunion.c | 69 ++++++++++++++-------------- lib/srcdest_table.c | 16 +++---- tests/lib/test_printfrr.c | 34 ++++++++++++++ 10 files changed, 277 insertions(+), 172 deletions(-) diff --git a/bgpd/bgp_table.c b/bgpd/bgp_table.c index 7e3aa2a48a..50123c608b 100644 --- a/bgpd/bgp_table.c +++ b/bgpd/bgp_table.c @@ -201,18 +201,17 @@ struct bgp_node *bgp_table_subtree_lookup(const struct bgp_table *table, } printfrr_ext_autoreg_p("BD", printfrr_bd) -static ssize_t printfrr_bd(char *buf, size_t bsz, const char *fmt, +static ssize_t printfrr_bd(struct fbuf *buf, const char **fmt, int prec, const void *ptr) { const struct bgp_dest *dest = ptr; - const struct prefix *p; + const struct prefix *p = bgp_dest_get_prefix(dest); + char cbuf[PREFIX_STRLEN]; - if (dest) { - p = bgp_dest_get_prefix(dest); - prefix2str(p, buf, bsz); - } else { - strlcpy(buf, "NULL", bsz); - } + if (!dest) + return bputs(buf, "NULL"); - return 2; + /* need to get the real length even if buffer too small */ + prefix2str(p, cbuf, sizeof(cbuf)); + return bputs(buf, cbuf); } diff --git a/lib/nexthop.c b/lib/nexthop.c index 17ef95c687..64b4f3adb1 100644 --- a/lib/nexthop.c +++ b/lib/nexthop.c @@ -730,80 +730,99 @@ int nexthop_str2backups(const char *str, int *num_backups, * nexthop2str() */ printfrr_ext_autoreg_p("NH", printfrr_nh) -static ssize_t printfrr_nh(char *buf, size_t bsz, const char *fmt, +static ssize_t printfrr_nh(struct fbuf *buf, const char **fmt, int prec, const void *ptr) { const struct nexthop *nexthop = ptr; - struct fbuf fb = { .buf = buf, .pos = buf, .len = bsz - 1 }; bool do_ifi = false; - const char *s, *v_is = "", *v_via = "", *v_viaif = "via "; - ssize_t ret = 3; + const char *v_is = "", *v_via = "", *v_viaif = "via "; + ssize_t ret = 0; - /* NULL-check */ - if (nexthop == NULL) { - if (fmt[2] == 'v' && fmt[3] == 'v') - ret++; - - strlcpy(buf, "NULL", bsz); - - return ret; - } - - switch (fmt[2]) { + switch (**fmt) { case 'v': - if (fmt[3] == 'v') { + (*fmt)++; + if (**fmt == 'v') { v_is = "is "; v_via = "via "; v_viaif = ""; - ret++; + (*fmt)++; } + if (!nexthop) + return bputs(buf, "NULL"); + switch (nexthop->type) { case NEXTHOP_TYPE_IPV4: case NEXTHOP_TYPE_IPV4_IFINDEX: - bprintfrr(&fb, "%s%pI4", v_via, &nexthop->gate.ipv4); + ret += bprintfrr(buf, "%s%pI4", v_via, + &nexthop->gate.ipv4); do_ifi = true; break; case NEXTHOP_TYPE_IPV6: case NEXTHOP_TYPE_IPV6_IFINDEX: - bprintfrr(&fb, "%s%pI6", v_via, &nexthop->gate.ipv6); + ret += bprintfrr(buf, "%s%pI6", v_via, + &nexthop->gate.ipv6); do_ifi = true; break; case NEXTHOP_TYPE_IFINDEX: - bprintfrr(&fb, "%sdirectly connected, %s", v_is, - ifindex2ifname(nexthop->ifindex, - nexthop->vrf_id)); + ret += bprintfrr(buf, "%sdirectly connected, %s", v_is, + ifindex2ifname(nexthop->ifindex, + nexthop->vrf_id)); break; case NEXTHOP_TYPE_BLACKHOLE: + ret += bputs(buf, "unreachable"); + switch (nexthop->bh_type) { case BLACKHOLE_REJECT: - s = " (ICMP unreachable)"; + ret += bputs(buf, " (ICMP unreachable)"); break; case BLACKHOLE_ADMINPROHIB: - s = " (ICMP admin-prohibited)"; + ret += bputs(buf, " (ICMP admin-prohibited)"); break; case BLACKHOLE_NULL: - s = " (blackhole)"; + ret += bputs(buf, " (blackhole)"); break; default: - s = ""; break; } - bprintfrr(&fb, "unreachable%s", s); break; default: break; } if (do_ifi && nexthop->ifindex) - bprintfrr(&fb, ", %s%s", v_viaif, ifindex2ifname( - nexthop->ifindex, - nexthop->vrf_id)); + ret += bprintfrr(buf, ", %s%s", v_viaif, + ifindex2ifname(nexthop->ifindex, + nexthop->vrf_id)); - *fb.pos = '\0'; return ret; case 's': - nexthop2str(nexthop, buf, bsz); - return 3; + (*fmt)++; + + if (!nexthop) + return bputs(buf, "NULL"); + + switch (nexthop->type) { + case NEXTHOP_TYPE_IFINDEX: + ret += bprintfrr(buf, "if %u", nexthop->ifindex); + break; + case NEXTHOP_TYPE_IPV4: + case NEXTHOP_TYPE_IPV4_IFINDEX: + ret += bprintfrr(buf, "%pI4 if %u", &nexthop->gate.ipv4, + nexthop->ifindex); + break; + case NEXTHOP_TYPE_IPV6: + case NEXTHOP_TYPE_IPV6_IFINDEX: + ret += bprintfrr(buf, "%pI6 if %u", &nexthop->gate.ipv6, + nexthop->ifindex); + break; + case NEXTHOP_TYPE_BLACKHOLE: + ret += bputs(buf, "blackhole"); + break; + default: + ret += bputs(buf, "unknown"); + break; + } + return ret; } - return 0; + return -1; } diff --git a/lib/prefix.c b/lib/prefix.c index afc4d3d5c2..169e9ed572 100644 --- a/lib/prefix.c +++ b/lib/prefix.c @@ -1361,92 +1361,92 @@ 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(char *buf, size_t bsz, const char *fmt, +static ssize_t printfrr_ea(struct fbuf *buf, const char **fmt, int prec, const void *ptr) { const struct ethaddr *mac = ptr; + char cbuf[ETHER_ADDR_STRLEN]; - if (mac) - prefix_mac2str(mac, buf, bsz); - else - strlcpy(buf, "NULL", bsz); + if (!mac) + return bputs(buf, "NULL"); - return 2; + /* need real length even if buffer is too short */ + prefix_mac2str(mac, cbuf, sizeof(cbuf)); + return bputs(buf, cbuf); } printfrr_ext_autoreg_p("IA", printfrr_ia) -static ssize_t printfrr_ia(char *buf, size_t bsz, const char *fmt, +static ssize_t printfrr_ia(struct fbuf *buf, const char **fmt, int prec, const void *ptr) { const struct ipaddr *ipa = ptr; + char cbuf[INET6_ADDRSTRLEN]; - if (ipa) - ipaddr2str(ipa, buf, bsz); - else - strlcpy(buf, "NULL", bsz); + if (!ipa) + return bputs(buf, "NULL"); - return 2; + ipaddr2str(ipa, cbuf, sizeof(cbuf)); + return bputs(buf, cbuf); } printfrr_ext_autoreg_p("I4", printfrr_i4) -static ssize_t printfrr_i4(char *buf, size_t bsz, const char *fmt, +static ssize_t printfrr_i4(struct fbuf *buf, const char **fmt, int prec, const void *ptr) { - if (ptr) - inet_ntop(AF_INET, ptr, buf, bsz); - else - strlcpy(buf, "NULL", bsz); + char cbuf[INET_ADDRSTRLEN]; - return 2; + if (!ptr) + return bputs(buf, "NULL"); + + inet_ntop(AF_INET, ptr, cbuf, sizeof(cbuf)); + return bputs(buf, cbuf); } printfrr_ext_autoreg_p("I6", printfrr_i6) -static ssize_t printfrr_i6(char *buf, size_t bsz, const char *fmt, +static ssize_t printfrr_i6(struct fbuf *buf, const char **fmt, int prec, const void *ptr) { - if (ptr) - inet_ntop(AF_INET6, ptr, buf, bsz); - else - strlcpy(buf, "NULL", bsz); + char cbuf[INET6_ADDRSTRLEN]; - return 2; + if (!ptr) + return bputs(buf, "NULL"); + + inet_ntop(AF_INET6, ptr, cbuf, sizeof(cbuf)); + return bputs(buf, cbuf); } printfrr_ext_autoreg_p("FX", printfrr_pfx) -static ssize_t printfrr_pfx(char *buf, size_t bsz, const char *fmt, +static ssize_t printfrr_pfx(struct fbuf *buf, const char **fmt, int prec, const void *ptr) { - if (ptr) - prefix2str(ptr, buf, bsz); - else - strlcpy(buf, "NULL", bsz); + char cbuf[PREFIX_STRLEN]; - return 2; + if (!ptr) + return bputs(buf, "NULL"); + + prefix2str(ptr, cbuf, sizeof(cbuf)); + return bputs(buf, cbuf); } printfrr_ext_autoreg_p("SG4", printfrr_psg) -static ssize_t printfrr_psg(char *buf, size_t bsz, const char *fmt, +static ssize_t printfrr_psg(struct fbuf *buf, const char **fmt, int prec, const void *ptr) { const struct prefix_sg *sg = ptr; - struct fbuf fb = { .buf = buf, .pos = buf, .len = bsz - 1 }; + ssize_t ret = 0; - if (sg) { - if (sg->src.s_addr == INADDR_ANY) - bprintfrr(&fb, "(*,"); - else - bprintfrr(&fb, "(%pI4,", &sg->src); + if (!sg) + return bputs(buf, "NULL"); - if (sg->grp.s_addr == INADDR_ANY) - bprintfrr(&fb, "*)"); - else - bprintfrr(&fb, "%pI4)", &sg->grp); + if (sg->src.s_addr == INADDR_ANY) + ret += bputs(buf, "(*,"); + else + ret += bprintfrr(buf, "(%pI4,", &sg->src); - fb.pos[0] = '\0'; + if (sg->grp.s_addr == INADDR_ANY) + ret += bputs(buf, "*)"); + else + ret += bprintfrr(buf, "%pI4)", &sg->grp); - } else { - strlcpy(buf, "NULL", bsz); - } - - return 3; + return ret; } 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; } diff --git a/lib/printf/printflocal.h b/lib/printf/printflocal.h index 335e09872e..df962fc043 100644 --- a/lib/printf/printflocal.h +++ b/lib/printf/printflocal.h @@ -100,6 +100,7 @@ int _frr_find_arguments(const char *, va_list, union arg **) DSO_LOCAL; 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; +/* returns number of bytes needed for full output, or -1 */ +ssize_t printfrr_extp(struct fbuf *, const char **, int, const void *) + 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 a0634cde4b..3b5bda965a 100644 --- a/lib/printf/vfprintf.c +++ b/lib/printf/vfprintf.c @@ -177,6 +177,7 @@ vbprintfrr(struct fbuf *cb, const char *fmt0, va_list ap) int nextarg; /* 1-based argument index */ va_list orgap; /* original argument pointer */ char *convbuf; /* wide to multibyte conversion result */ + char *extstart = NULL; /* where printfrr_ext* started printing */ static const char xdigs_lower[16] = "0123456789abcdef"; static const char xdigs_upper[16] = "0123456789ABCDEF"; @@ -438,16 +439,14 @@ reswitch: switch (ch) { ulval = SARG(); if (printfrr_ext_char(fmt[0])) { - n2 = printfrr_exti(buf, sizeof(buf), fmt, prec, + if (cb) + extstart = cb->pos; + + size = printfrr_exti(cb, &fmt, prec, (flags & INTMAX_SIZE) ? ujval : (uintmax_t)ulval); - if (n2 > 0) { - fmt += n2; - cp = buf; - size = strlen(cp); - sign = '\0'; - break; - } + if (size >= 0) + goto ext_printed; } if (flags & INTMAX_SIZE) { if ((intmax_t)ujval < 0) { @@ -551,14 +550,13 @@ reswitch: switch (ch) { * -- ANSI X3J11 */ 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; + if (printfrr_ext_char(fmt[0])) { + if (cb) + extstart = cb->pos; + + size = printfrr_extp(cb, &fmt, prec, ptrval); + if (size >= 0) + goto ext_printed; } ujval = (uintmax_t)(uintptr_t)ptrval; base = 16; @@ -686,7 +684,7 @@ number: if ((dprec = prec) >= 0) realsz += 2; prsize = width > realsz ? width : realsz; - if ((unsigned)ret + prsize > INT_MAX) { + if ((unsigned int)ret + prsize > INT_MAX) { ret = EOF; errno = EOVERFLOW; goto error; @@ -720,6 +718,58 @@ number: if ((dprec = prec) >= 0) /* finally, adjust ret */ ret += prsize; + FLUSH(); /* copy out the I/O vectors */ + continue; + +ext_printed: + /* when we arrive here, a printfrr extension has written to cb + * (if non-NULL), but we still need to handle padding. The + * original cb->pos is in extstart; the return value from the + * ext is in size. + * + * Keep analogous to code above please. + */ + + realsz = size; + prsize = width > realsz ? width : realsz; + if ((unsigned int)ret + prsize > INT_MAX) { + ret = EOF; + errno = EOVERFLOW; + goto error; + } + + /* right-adjusting blank padding - need to move the chars + * that the extension has already written. Should be very + * rare. + */ + if (cb && width > size && (flags & (LADJUST|ZEROPAD)) == 0) { + size_t nwritten = cb->pos - extstart; + size_t navail = cb->buf + cb->len - extstart; + size_t npad = width - realsz; + size_t nmove; + + if (navail < npad) + navail = 0; + else + navail -= npad; + nmove = MIN(nwritten, navail); + + memmove(extstart + npad, extstart, nmove); + + cb->pos = extstart; + PAD(npad, blanks); + cb->pos += nmove; + } + + io.avail = cb ? cb->len - (cb->pos - cb->buf) : 0; + + /* left-adjusting padding (always blank) */ + if (flags & LADJUST) + PAD(width - realsz, blanks); + + /* finally, adjust ret */ + ret += prsize; + FLUSH(); /* copy out the I/O vectors */ } done: diff --git a/lib/printfrr.h b/lib/printfrr.h index 418e839d97..549334ba5b 100644 --- a/lib/printfrr.h +++ b/lib/printfrr.h @@ -112,20 +112,21 @@ struct printfrr_ext { /* 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. + * return value: number of bytes that would be printed if the buffer + * was large enough. be careful about not under-reporting this; + * otherwise asnprintf() & co. will get broken. Returning -1 means + * something went wrong & default %p/%d handling should be executed. * - * bsz is a compile-time constant in printf; it's gonna be relatively - * small. This isn't designed to print Shakespeare from a pointer. + * 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. * * 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, + ssize_t (*print_ptr)(struct fbuf *buf, const char **fmt, int prec, const void *); - ssize_t (*print_int)(char *buf, size_t bsz, const char *fmt, int prec, + ssize_t (*print_int)(struct fbuf *buf, const char **fmt, int prec, uintmax_t); }; @@ -136,7 +137,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(char *, size_t, const char *, int, \ + static ssize_t print_fn(struct fbuf *, const char **, int, \ const void *); \ static const struct printfrr_ext _printext_##print_fn = { \ .match = matchs, \ @@ -149,7 +150,7 @@ void printfrr_ext_reg(const struct printfrr_ext *); /* end */ #define printfrr_ext_autoreg_i(matchs, print_fn) \ - static ssize_t print_fn(char *, size_t, const char *, int, uintmax_t); \ + static ssize_t print_fn(struct fbuf *, const char **, int, 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 d65235b41c..25de8f7dd0 100644 --- a/lib/sockunion.c +++ b/lib/sockunion.c @@ -664,54 +664,51 @@ void sockunion_init(union sockunion *su) } printfrr_ext_autoreg_p("SU", printfrr_psu) -static ssize_t printfrr_psu(char *buf, size_t bsz, const char *fmt, +static ssize_t printfrr_psu(struct fbuf *buf, const char **fmt, int prec, const void *ptr) { const union sockunion *su = ptr; - struct fbuf fb = { .buf = buf, .pos = buf, .len = bsz - 1 }; bool include_port = false; bool endflags = false; - ssize_t consumed = 2; + ssize_t ret = 0; + char cbuf[INET6_ADDRSTRLEN]; - if (su) { - while (!endflags) { - switch (fmt[consumed++]) { - case 'p': - include_port = true; - break; - default: - consumed--; - endflags = true; - break; - } - }; + if (!su) + return bputs(buf, "NULL"); - switch (sockunion_family(su)) { - case AF_UNSPEC: - bprintfrr(&fb, "(unspec)"); - break; - case AF_INET: - inet_ntop(AF_INET, &su->sin.sin_addr, buf, bsz); - fb.pos += strlen(fb.buf); - if (include_port) - bprintfrr(&fb, ":%d", su->sin.sin_port); - break; - case AF_INET6: - inet_ntop(AF_INET6, &su->sin6.sin6_addr, buf, bsz); - fb.pos += strlen(fb.buf); - if (include_port) - bprintfrr(&fb, ":%d", su->sin6.sin6_port); + while (!endflags) { + switch (**fmt) { + case 'p': + (*fmt)++; + include_port = true; break; default: - bprintfrr(&fb, "(af %d)", sockunion_family(su)); + endflags = true; + break; } - - fb.pos[0] = '\0'; - } else { - strlcpy(buf, "NULL", bsz); } - return consumed; + switch (sockunion_family(su)) { + case AF_UNSPEC: + ret += bputs(buf, "(unspec)"); + break; + case AF_INET: + inet_ntop(AF_INET, &su->sin.sin_addr, cbuf, sizeof(cbuf)); + ret += bputs(buf, cbuf); + if (include_port) + ret += bprintfrr(buf, ":%d", su->sin.sin_port); + break; + case AF_INET6: + inet_ntop(AF_INET6, &su->sin6.sin6_addr, cbuf, sizeof(cbuf)); + ret += bputs(buf, cbuf); + if (include_port) + ret += bprintfrr(buf, ":%d", su->sin6.sin6_port); + break; + default: + ret += bprintfrr(buf, "(af %d)", sockunion_family(su)); + } + + return ret; } int sockunion_is_null(const union sockunion *su) 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) diff --git a/tests/lib/test_printfrr.c b/tests/lib/test_printfrr.c index 24de3fa88d..9ffea8b978 100644 --- a/tests/lib/test_printfrr.c +++ b/tests/lib/test_printfrr.c @@ -64,8 +64,37 @@ static void printchk(const char *ref, const char *fmt, ...) { va_list ap; char bufrr[256]; + bool truncfail = false; + size_t i; + size_t expectlen; + memset(bufrr, 0xcc, sizeof(bufrr)); + va_start(ap, fmt); + expectlen = vsnprintfrr(NULL, 0, fmt, ap); + va_end(ap); + + va_start(ap, fmt); + vsnprintfrr(bufrr, 7, fmt, ap); + va_end(ap); + + if (strnlen(bufrr, 7) == 7) + truncfail = true; + if (strnlen(bufrr, 7) < 7 && strncmp(ref, bufrr, 6) != 0) + truncfail = true; + for (i = 7; i < sizeof(bufrr); i++) + if (bufrr[i] != (char)0xcc) { + truncfail = true; + break; + } + + if (truncfail) { + printf("truncation test FAILED:\n" + "fmt: \"%s\"\nref: \"%s\"\nfrr[:7]: \"%s\"\n%s\n\n", + fmt, ref, bufrr, strcmp(ref, bufrr) ? "ERROR" : "ok"); + errors++; + } + va_start(ap, fmt); vsnprintfrr(bufrr, sizeof(bufrr), fmt, ap); va_end(ap); @@ -74,6 +103,10 @@ static void printchk(const char *ref, const char *fmt, ...) fmt, ref, bufrr, strcmp(ref, bufrr) ? "ERROR" : "ok"); if (strcmp(ref, bufrr)) errors++; + if (strlen(bufrr) != expectlen) { + printf("return value <> length mismatch\n"); + errors++; + } } int main(int argc, char **argv) @@ -112,6 +145,7 @@ int main(int argc, char **argv) inet_aton("192.168.1.2", &ip); printchk("192.168.1.2", "%pI4", &ip); printchk(" 192.168.1.2", "%20pI4", &ip); + printchk("192.168.1.2 ", "%-20pI4", &ip); printcmp("%p", &ip); From eba599a39756b3d9424467f1e1f4f4475dffad17 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 26 Mar 2021 19:14:24 +0100 Subject: [PATCH 04/16] 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(-) 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, From 487eefcfbec8a24d639e6b8c805540d5c1fe38e8 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 2 Mar 2021 20:16:18 +0100 Subject: [PATCH 05/16] lib: record output positions in printfrr This replaces `%n` with a safe, out-of-band option that simply records the start and end offset of the output produced for each `%...` specifier. The old `%n` code is removed. Signed-off-by: David Lamparter --- lib/printf/vfprintf.c | 67 +++++++++++++++++++++------------------ lib/printfrr.h | 7 ++++ tests/lib/test_printfrr.c | 22 +++++++++++-- 3 files changed, 64 insertions(+), 32 deletions(-) diff --git a/lib/printf/vfprintf.c b/lib/printf/vfprintf.c index 3b5bda965a..96675e3620 100644 --- a/lib/printf/vfprintf.c +++ b/lib/printf/vfprintf.c @@ -147,7 +147,7 @@ __wcsconv(wchar_t *wcsarg, int prec) * Non-MT-safe version */ ssize_t -vbprintfrr(struct fbuf *cb, const char *fmt0, va_list ap) +vbprintfrr(struct fbuf *cb_in, const char *fmt0, va_list ap) { const char *fmt; /* format string */ int ch; /* character from fmt */ @@ -178,6 +178,8 @@ vbprintfrr(struct fbuf *cb, const char *fmt0, va_list ap) va_list orgap; /* original argument pointer */ char *convbuf; /* wide to multibyte conversion result */ char *extstart = NULL; /* where printfrr_ext* started printing */ + struct fbuf cb_copy, *cb; + struct fmt_outpos *opos; static const char xdigs_lower[16] = "0123456789abcdef"; static const char xdigs_upper[16] = "0123456789ABCDEF"; @@ -269,6 +271,16 @@ vbprintfrr(struct fbuf *cb, const char *fmt0, va_list ap) argtable = NULL; nextarg = 1; va_copy(orgap, ap); + + if (cb_in) { + /* prevent printfrr exts from polluting cb->outpos */ + cb_copy = *cb_in; + cb_copy.outpos = NULL; + cb_copy.outpos_n = cb_copy.outpos_i = 0; + cb = &cb_copy; + } else + cb = NULL; + io_init(&io, cb); ret = 0; @@ -298,6 +310,11 @@ vbprintfrr(struct fbuf *cb, const char *fmt0, va_list ap) sign = '\0'; ox[1] = '\0'; + if (cb_in && cb_in->outpos_i < cb_in->outpos_n) + opos = &cb_in->outpos[cb_in->outpos_i]; + else + opos = NULL; + rflag: ch = *fmt++; reswitch: switch (ch) { case ' ': @@ -502,35 +519,6 @@ reswitch: switch (ch) { size = (prec >= 0) ? strnlen(cp, prec) : strlen(cp); sign = '\0'; break; -#ifdef DANGEROUS_PERCENT_N - /* FRR does not use %n in printf formats. This is just left - * here in case someone tries to use %n and starts debugging - * why the f* it doesn't work - */ - case 'n': - /* - * Assignment-like behavior is specified if the - * value overflows or is otherwise unrepresentable. - * C99 says to use `signed char' for %hhn conversions. - */ - if (flags & LLONGINT) - *GETARG(long long *) = ret; - else if (flags & SIZET) - *GETARG(ssize_t *) = (ssize_t)ret; - else if (flags & PTRDIFFT) - *GETARG(ptrdiff_t *) = ret; - else if (flags & INTMAXT) - *GETARG(intmax_t *) = ret; - else if (flags & LONGINT) - *GETARG(long *) = ret; - else if (flags & SHORTINT) - *GETARG(short *) = ret; - else if (flags & CHARINT) - *GETARG(signed char *) = ret; - else - *GETARG(int *) = ret; - continue; /* no output */ -#endif case 'O': flags |= LONGINT; /*FALLTHROUGH*/ @@ -660,6 +648,7 @@ number: if ((dprec = prec) >= 0) cp = buf; size = 1; sign = '\0'; + opos = NULL; break; } @@ -694,6 +683,9 @@ number: if ((dprec = prec) >= 0) if ((flags & (LADJUST|ZEROPAD)) == 0) PAD(width - realsz, blanks); + if (opos) + opos->off_start = cb->pos - cb->buf; + /* prefix */ if (sign) PRINT(&sign, 1); @@ -711,6 +703,12 @@ number: if ((dprec = prec) >= 0) /* leading zeroes from decimal precision */ PAD(dprec - size, zeroes); PRINT(cp, size); + + if (opos) { + opos->off_end = cb->pos - cb->buf; + cb_in->outpos_i++; + } + /* left-adjusting padding (always blank) */ if (flags & LADJUST) PAD(width - realsz, blanks); @@ -759,10 +757,17 @@ ext_printed: cb->pos = extstart; PAD(npad, blanks); cb->pos += nmove; + extstart += npad; } io.avail = cb ? cb->len - (cb->pos - cb->buf) : 0; + if (opos && extstart <= cb->pos) { + opos->off_start = extstart - cb->buf; + opos->off_end = cb->pos - cb->buf; + cb_in->outpos_i++; + } + /* left-adjusting padding (always blank) */ if (flags & LADJUST) PAD(width - realsz, blanks); @@ -780,6 +785,8 @@ error: free(convbuf); if ((argtable != NULL) && (argtable != statargtable)) free (argtable); + if (cb_in) + cb_in->pos = cb->pos; return (ret); /* NOTREACHED */ } diff --git a/lib/printfrr.h b/lib/printfrr.h index 549334ba5b..49243248d6 100644 --- a/lib/printfrr.h +++ b/lib/printfrr.h @@ -28,10 +28,17 @@ extern "C" { #endif +struct fmt_outpos { + unsigned int off_start, off_end; +}; + struct fbuf { char *buf; char *pos; size_t len; + + struct fmt_outpos *outpos; + size_t outpos_n, outpos_i; }; #define at(a, b) PRINTFRR(a, b) diff --git a/tests/lib/test_printfrr.c b/tests/lib/test_printfrr.c index 9ffea8b978..6d64615faf 100644 --- a/tests/lib/test_printfrr.c +++ b/tests/lib/test_printfrr.c @@ -95,11 +95,21 @@ static void printchk(const char *ref, const char *fmt, ...) errors++; } + struct fmt_outpos outpos[16]; + struct fbuf fb = { + .buf = bufrr, + .pos = bufrr, + .len = sizeof(bufrr) - 1, + .outpos = outpos, + .outpos_n = array_size(outpos), + }; + va_start(ap, fmt); - vsnprintfrr(bufrr, sizeof(bufrr), fmt, ap); + vbprintfrr(&fb, fmt, ap); + fb.pos[0] = '\0'; va_end(ap); - printf("fmt: \"%s\"\nref: \"%s\"\nfrr: \"%s\"\n%s\n\n", + printf("fmt: \"%s\"\nref: \"%s\"\nfrr: \"%s\"\n%s\n", fmt, ref, bufrr, strcmp(ref, bufrr) ? "ERROR" : "ok"); if (strcmp(ref, bufrr)) errors++; @@ -107,6 +117,14 @@ static void printchk(const char *ref, const char *fmt, ...) printf("return value <> length mismatch\n"); errors++; } + + for (size_t i = 0; i < fb.outpos_i; i++) + printf("\t[%zu: %u..%u] = \"%.*s\"\n", i, + outpos[i].off_start, + outpos[i].off_end, + (int)(outpos[i].off_end - outpos[i].off_start), + bufrr + outpos[i].off_start); + printf("\n"); } int main(int argc, char **argv) From 3ea794305960ed9fca230c3e0f8b1b73c2915101 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sat, 20 Mar 2021 09:02:04 +0100 Subject: [PATCH 06/16] 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 | 60 ++++++++++++++++++++++++++++++++++------ lib/sockunion.c | 8 +++--- lib/srcdest_table.c | 4 +-- 9 files changed, 115 insertions(+), 50 deletions(-) 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. - * - * 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)(struct fbuf *buf, const char **fmt, int prec, - const void *); - ssize_t (*print_int)(struct fbuf *buf, const char **fmt, int prec, - uintmax_t); + 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. + * + * 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. + */ + 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; From 2d9a4e2931e4f20c6bc49b09956a1e350664e3f2 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 26 Mar 2021 17:58:54 +0100 Subject: [PATCH 07/16] lib: allow discerning unspec width in printfrr ext With 0 currently the default value for the width specifier, it's not possible to discern that from a %*p where 0 was passed as the length parameter. Use -1 to allow for that. Signed-off-by: David Lamparter --- lib/printf/vfprintf.c | 8 +++++++- lib/printfrr.h | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/lib/printf/vfprintf.c b/lib/printf/vfprintf.c index 7fafa89aa7..1bd24743ee 100644 --- a/lib/printf/vfprintf.c +++ b/lib/printf/vfprintf.c @@ -305,7 +305,7 @@ vbprintfrr(struct fbuf *cb_in, const char *fmt0, va_list ap) flags = 0; dprec = 0; - width = 0; + width = -1; prec = -1; sign = '\0'; ox[1] = '\0'; @@ -688,6 +688,9 @@ number: if ((dprec = prec) >= 0) * Compute actual size, so we know how much to pad. * size excludes decimal prec; realsz includes it. */ + if (width < 0) + width = 0; + realsz = dprec > size ? dprec : size; if (sign) realsz++; @@ -750,6 +753,9 @@ ext_printed: * Keep analogous to code above please. */ + if (width < 0) + width = 0; + realsz = size; prsize = width > realsz ? width : realsz; if ((unsigned int)ret + prsize > INT_MAX) { diff --git a/lib/printfrr.h b/lib/printfrr.h index 754cb09598..9dd20f0a8f 100644 --- a/lib/printfrr.h +++ b/lib/printfrr.h @@ -178,6 +178,23 @@ struct printfrr_eargs { bool leftadj; }; +/* for any extension that needs a buffer length */ + +static inline ssize_t printfrr_ext_len(struct printfrr_eargs *ea) +{ + ssize_t rv; + + if (ea->precision >= 0) + rv = ea->precision; + else if (ea->width >= 0) { + rv = ea->width; + ea->width = -1; + } else + rv = -1; + + return rv; +} + /* 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... From cb4928ce77c089ae521301776ed90e994c29800c Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 26 Mar 2021 18:11:21 +0100 Subject: [PATCH 08/16] lib: add `FMT_NSTD()` for non-standard printf exts ... to suppress the warnings when using something that isn't quite ISO C compatible and would otherwise cause compiler warnings from `-Wformat`. Signed-off-by: David Lamparter --- lib/printfrr.h | 25 +++++++++++++++++++++++++ tests/lib/test_printfrr.c | 5 +++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/lib/printfrr.h b/lib/printfrr.h index 9dd20f0a8f..8245a664b3 100644 --- a/lib/printfrr.h +++ b/lib/printfrr.h @@ -251,6 +251,31 @@ static inline ssize_t bputch(struct fbuf *buf, char ch) return 1; } +/* when using non-ISO-C compatible extension specifiers... */ + +#ifdef _FRR_ATTRIBUTE_PRINTFRR +#define FMT_NSTD_BEGIN +#define FMT_NSTD_END +#else /* !_FRR_ATTRIBUTE_PRINTFRR */ +#define FMT_NSTD_BEGIN \ + _Pragma("GCC diagnostic push") \ + _Pragma("GCC diagnostic ignored \"-Wformat\"") \ + /* end */ +#define FMT_NSTD_END \ + _Pragma("GCC diagnostic pop") \ + /* end */ +#endif + +#define FMT_NSTD(expr) \ + ({ \ + typeof(expr) _v; \ + FMT_NSTD_BEGIN \ + _v = expr; \ + FMT_NSTD_END \ + _v; \ + }) \ + /* end */ + #ifdef __cplusplus } #endif diff --git a/tests/lib/test_printfrr.c b/tests/lib/test_printfrr.c index 6d64615faf..5a8d10e9b7 100644 --- a/tests/lib/test_printfrr.c +++ b/tests/lib/test_printfrr.c @@ -59,8 +59,8 @@ static void printcmp(const char *fmt, ...) errors++; } -static void printchk(const char *ref, const char *fmt, ...) PRINTFRR(2, 3); -static void printchk(const char *ref, const char *fmt, ...) +static int printchk(const char *ref, const char *fmt, ...) PRINTFRR(2, 3); +static int printchk(const char *ref, const char *fmt, ...) { va_list ap; char bufrr[256]; @@ -125,6 +125,7 @@ static void printchk(const char *ref, const char *fmt, ...) (int)(outpos[i].off_end - outpos[i].off_start), bufrr + outpos[i].off_start); printf("\n"); + return 0; } int main(int argc, char **argv) From 9c4380daee0e495ea63d161af61d7f7e70c9d9ea Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 2 Mar 2021 20:45:57 +0100 Subject: [PATCH 09/16] 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(+) 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); From bb12115e0be449df92af6294fc8410eb7745be26 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 2 Mar 2021 21:39:49 +0100 Subject: [PATCH 10/16] 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(+) 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 From a4cb97a6c1d8718be40a16c1c7fc0b2738d17947 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 19 Feb 2021 00:08:11 +0100 Subject: [PATCH 11/16] lib: add `%*pHX` + `%*pHS` hexdump in printfrr (I'll get to `zlog_hexdump()` in a separate pass.) Signed-off-by: David Lamparter --- doc/developer/logging.rst | 2 ++ lib/printfrr.h | 24 ++++++++++++- lib/strformat.c | 75 +++++++++++++++++++++++++++++++++++++++ lib/subdir.am | 1 + tests/lib/test_printfrr.c | 10 ++++++ 5 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 lib/strformat.c diff --git a/doc/developer/logging.rst b/doc/developer/logging.rst index 5e8cb79cc2..f19554b532 100644 --- a/doc/developer/logging.rst +++ b/doc/developer/logging.rst @@ -91,6 +91,8 @@ Extensions +-----------+--------------------------+----------------------------------------------+ | ``%pFX`` | ``struct bgp_dest *`` | ``fe80::1234/64`` (available in BGP only) | +-----------+--------------------------+----------------------------------------------+ +| ``%.*pHX``| ``int len, void *ptr`` | ``12 34 56 78`` (hexdump) | ++-----------+--------------------------+----------------------------------------------+ Printf features like field lengths can be used normally with these extensions, e.g. ``%-15pI4`` works correctly. diff --git a/lib/printfrr.h b/lib/printfrr.h index 8ea8fd69a7..7083e8b582 100644 --- a/lib/printfrr.h +++ b/lib/printfrr.h @@ -227,7 +227,11 @@ void printfrr_ext_reg(const struct printfrr_ext *); } \ /* end */ -/* fbuf helper functions */ +/* fbuf helper functions - note all 3 of these return the length that would + * be written regardless of how much space was available in the buffer, as + * needed for implementing printfrr extensions. (They also accept NULL buf + * for that.) + */ static inline ssize_t bputs(struct fbuf *buf, const char *str) { @@ -251,6 +255,17 @@ static inline ssize_t bputch(struct fbuf *buf, char ch) return 1; } +static inline ssize_t bputhex(struct fbuf *buf, uint8_t val) +{ + static const char hexch[] = "0123456789abcdef"; + + if (buf && buf->pos < buf->buf + buf->len) + *buf->pos++ = hexch[(val >> 4) & 0xf]; + if (buf && buf->pos < buf->buf + buf->len) + *buf->pos++ = hexch[val & 0xf]; + return 2; +} + /* %pVA extension, equivalent to Linux kernel %pV */ struct va_format { @@ -261,6 +276,13 @@ struct va_format { #ifdef _FRR_ATTRIBUTE_PRINTFRR #pragma FRR printfrr_ext "%pFB" (struct fbuf *) #pragma FRR printfrr_ext "%pVA" (struct va_format *) + +#pragma FRR printfrr_ext "%pHX" (signed char *) +#pragma FRR printfrr_ext "%pHX" (unsigned char *) +#pragma FRR printfrr_ext "%pHX" (void *) +#pragma FRR printfrr_ext "%pHS" (signed char *) +#pragma FRR printfrr_ext "%pHS" (unsigned char *) +#pragma FRR printfrr_ext "%pHS" (void *) #endif /* when using non-ISO-C compatible extension specifiers... */ diff --git a/lib/strformat.c b/lib/strformat.c new file mode 100644 index 0000000000..8e49a666fa --- /dev/null +++ b/lib/strformat.c @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2019 David Lamparter, for NetDEF, Inc. + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ + +#ifdef HAVE_CONFIG_H +#include "config.h" +#endif + +#include + +#include "printfrr.h" + +printfrr_ext_autoreg_p("HX", printfrr_hexdump) +static ssize_t printfrr_hexdump(struct fbuf *buf, struct printfrr_eargs *ea, + const void *ptr) +{ + ssize_t ret = 0; + ssize_t input_len = printfrr_ext_len(ea); + char sep = ' '; + const uint8_t *pos, *end; + + if (ea->fmt[0] == 'c') { + ea->fmt++; + sep = ':'; + } else if (ea->fmt[0] == 'n') { + ea->fmt++; + sep = '\0'; + } + + if (input_len < 0) + return 0; + + for (pos = ptr, end = pos + input_len; pos < end; pos++) { + if (sep && pos != ptr) + ret += bputch(buf, sep); + ret += bputhex(buf, *pos); + } + + return ret; +} + +/* string analog for hexdumps / the "this." in ("74 68 69 73 0a |this.|") */ + +printfrr_ext_autoreg_p("HS", printfrr_hexdstr) +static ssize_t printfrr_hexdstr(struct fbuf *buf, struct printfrr_eargs *ea, + const void *ptr) +{ + ssize_t ret = 0; + ssize_t input_len = printfrr_ext_len(ea); + const uint8_t *pos, *end; + + if (input_len < 0) + return 0; + + for (pos = ptr, end = pos + input_len; pos < end; pos++) { + if (*pos >= 0x20 && *pos < 0x7f) + ret += bputch(buf, *pos); + else + ret += bputch(buf, '.'); + } + + return ret; +} diff --git a/lib/subdir.am b/lib/subdir.am index bfd367b134..0853d4bb2b 100644 --- a/lib/subdir.am +++ b/lib/subdir.am @@ -90,6 +90,7 @@ lib_libfrr_la_SOURCES = \ lib/spf_backoff.c \ lib/srcdest_table.c \ lib/stream.c \ + lib/strformat.c \ lib/strlcat.c \ lib/strlcpy.c \ lib/systemd.c \ diff --git a/tests/lib/test_printfrr.c b/tests/lib/test_printfrr.c index 8fa8bfcc23..1ef10b19d0 100644 --- a/tests/lib/test_printfrr.c +++ b/tests/lib/test_printfrr.c @@ -216,5 +216,15 @@ int main(int argc, char **argv) sg.src.s_addr = INADDR_ANY; printchk("(*,224.1.2.3)", "%pSG4", &sg); + uint8_t randhex[] = { 0x12, 0x34, 0x00, 0xca, 0xfe, 0x00, 0xaa, 0x55 }; + + printchk("12 34 00 ca fe 00 aa 55", "%.8pHX", randhex); + printchk("12 34 00 ca fe 00 aa 55", "%.*pHX", + (int)sizeof(randhex), randhex); + printchk("12 34 00 ca", "%.4pHX", randhex); + + printchk("12:34:00:ca:fe:00:aa:55", "%.8pHXc", randhex); + printchk("123400cafe00aa55", "%.8pHXn", randhex); + return !!errors; } From 7798203f5cf93b35aed96925f3ffd6fa00a44790 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Mon, 22 Mar 2021 10:12:42 +0100 Subject: [PATCH 12/16] lib: add `%pSQ` and `%pSE` string escape formats These are for string quoting (`%pSQ`) and string escaping (`%pSE`); the sets / escape methods are currently rather "basic" and might be extended in the future. Signed-off-by: David Lamparter --- lib/printfrr.h | 3 + lib/strformat.c | 197 ++++++++++++++++++++++++++++++++++++++ tests/lib/test_printfrr.c | 39 ++++++-- 3 files changed, 233 insertions(+), 6 deletions(-) diff --git a/lib/printfrr.h b/lib/printfrr.h index 7083e8b582..4338ac3a2f 100644 --- a/lib/printfrr.h +++ b/lib/printfrr.h @@ -283,6 +283,9 @@ struct va_format { #pragma FRR printfrr_ext "%pHS" (signed char *) #pragma FRR printfrr_ext "%pHS" (unsigned char *) #pragma FRR printfrr_ext "%pHS" (void *) + +#pragma FRR printfrr_ext "%pSE" (char *) +#pragma FRR printfrr_ext "%pSQ" (char *) #endif /* when using non-ISO-C compatible extension specifiers... */ diff --git a/lib/strformat.c b/lib/strformat.c index 8e49a666fa..431e573a0c 100644 --- a/lib/strformat.c +++ b/lib/strformat.c @@ -18,7 +18,10 @@ #include "config.h" #endif +#include "compiler.h" + #include +#include #include "printfrr.h" @@ -73,3 +76,197 @@ static ssize_t printfrr_hexdstr(struct fbuf *buf, struct printfrr_eargs *ea, return ret; } + +enum escape_flags { + ESC_N_R_T = (1 << 0), /* use \n \r \t instead of \x0a ...*/ + ESC_SPACE = (1 << 1), /* \ */ + ESC_BACKSLASH = (1 << 2), /* \\ */ + ESC_DBLQUOTE = (1 << 3), /* \" */ + ESC_SGLQUOTE = (1 << 4), /* \' */ + ESC_BACKTICK = (1 << 5), /* \` */ + ESC_DOLLAR = (1 << 6), /* \$ */ + ESC_CLBRACKET = (1 << 7), /* \] for RFC5424 syslog */ + ESC_OTHER = (1 << 8), /* remaining non-alpha */ + + ESC_ALL = ESC_N_R_T | ESC_SPACE | ESC_BACKSLASH | ESC_DBLQUOTE + | ESC_SGLQUOTE | ESC_DOLLAR | ESC_OTHER, + ESC_QUOTSTRING = ESC_N_R_T | ESC_BACKSLASH | ESC_DBLQUOTE, + /* if needed: ESC_SHELL = ... */ +}; + +static ssize_t bquote(struct fbuf *buf, const uint8_t *pos, size_t len, + unsigned int flags) +{ + ssize_t ret = 0; + const uint8_t *end = pos + len; + + for (; pos < end; pos++) { + /* here's to hoping this might be a bit faster... */ + if (__builtin_expect(!!isalnum(*pos), 1)) { + ret += bputch(buf, *pos); + continue; + } + + switch (*pos) { + case '%': + case '+': + case ',': + case '-': + case '.': + case '/': + case ':': + case '@': + case '_': + ret += bputch(buf, *pos); + continue; + + case '\r': + if (!(flags & ESC_N_R_T)) + break; + ret += bputch(buf, '\\'); + ret += bputch(buf, 'r'); + continue; + case '\n': + if (!(flags & ESC_N_R_T)) + break; + ret += bputch(buf, '\\'); + ret += bputch(buf, 'n'); + continue; + case '\t': + if (!(flags & ESC_N_R_T)) + break; + ret += bputch(buf, '\\'); + ret += bputch(buf, 't'); + continue; + + case ' ': + if (flags & ESC_SPACE) + ret += bputch(buf, '\\'); + ret += bputch(buf, *pos); + continue; + + case '\\': + if (flags & ESC_BACKSLASH) + ret += bputch(buf, '\\'); + ret += bputch(buf, *pos); + continue; + + case '"': + if (flags & ESC_DBLQUOTE) + ret += bputch(buf, '\\'); + ret += bputch(buf, *pos); + continue; + + case '\'': + if (flags & ESC_SGLQUOTE) + ret += bputch(buf, '\\'); + ret += bputch(buf, *pos); + continue; + + case '`': + if (flags & ESC_BACKTICK) + ret += bputch(buf, '\\'); + ret += bputch(buf, *pos); + continue; + + case '$': + if (flags & ESC_DOLLAR) + ret += bputch(buf, '\\'); + ret += bputch(buf, *pos); + continue; + + case ']': + if (flags & ESC_CLBRACKET) + ret += bputch(buf, '\\'); + ret += bputch(buf, *pos); + continue; + + /* remaining: !#&'()*;<=>?[^{|}~ */ + + default: + if (*pos >= 0x20 && *pos < 0x7f) { + if (flags & ESC_OTHER) + ret += bputch(buf, '\\'); + ret += bputch(buf, *pos); + continue; + } + } + ret += bputch(buf, '\\'); + ret += bputch(buf, 'x'); + ret += bputhex(buf, *pos); + } + + return ret; +} + +printfrr_ext_autoreg_p("SE", printfrr_escape) +static ssize_t printfrr_escape(struct fbuf *buf, struct printfrr_eargs *ea, + const void *vptr) +{ + ssize_t len = printfrr_ext_len(ea); + const uint8_t *ptr = vptr; + bool null_is_empty = false; + + if (ea->fmt[0] == 'n') { + null_is_empty = true; + ea->fmt++; + } + + if (!ptr) { + if (null_is_empty) + return 0; + return bputs(buf, "(null)"); + } + + if (len < 0) + len = strlen((const char *)ptr); + + return bquote(buf, ptr, len, ESC_ALL); +} + +printfrr_ext_autoreg_p("SQ", printfrr_quote) +static ssize_t printfrr_quote(struct fbuf *buf, struct printfrr_eargs *ea, + const void *vptr) +{ + ssize_t len = printfrr_ext_len(ea); + const uint8_t *ptr = vptr; + ssize_t ret = 0; + bool null_is_empty = false; + bool do_quotes = false; + unsigned int flags = ESC_QUOTSTRING; + + while (ea->fmt[0]) { + switch (ea->fmt[0]) { + case 'n': + null_is_empty = true; + ea->fmt++; + continue; + case 'q': + do_quotes = true; + ea->fmt++; + continue; + case 's': + flags |= ESC_CLBRACKET; + flags &= ~ESC_N_R_T; + ea->fmt++; + continue; + } + break; + } + + if (!ptr) { + if (null_is_empty) + return bputs(buf, do_quotes ? "\"\"" : ""); + return bputs(buf, "(null)"); + } + + if (len < 0) + len = strlen((const char *)ptr); + + if (do_quotes) + ret += bputch(buf, '"'); + ret += bquote(buf, ptr, len, flags); + if (do_quotes) + ret += bputch(buf, '"'); + return ret; +} diff --git a/tests/lib/test_printfrr.c b/tests/lib/test_printfrr.c index 1ef10b19d0..21b3a916b8 100644 --- a/tests/lib/test_printfrr.c +++ b/tests/lib/test_printfrr.c @@ -218,13 +218,40 @@ int main(int argc, char **argv) uint8_t randhex[] = { 0x12, 0x34, 0x00, 0xca, 0xfe, 0x00, 0xaa, 0x55 }; - printchk("12 34 00 ca fe 00 aa 55", "%.8pHX", randhex); - printchk("12 34 00 ca fe 00 aa 55", "%.*pHX", - (int)sizeof(randhex), randhex); - printchk("12 34 00 ca", "%.4pHX", randhex); + FMT_NSTD(printchk("12 34 00 ca fe 00 aa 55", "%.8pHX", randhex)); + FMT_NSTD(printchk("12 34 00 ca fe 00 aa 55", "%.*pHX", + (int)sizeof(randhex), randhex)); + FMT_NSTD(printchk("12 34 00 ca", "%.4pHX", randhex)); - printchk("12:34:00:ca:fe:00:aa:55", "%.8pHXc", randhex); - printchk("123400cafe00aa55", "%.8pHXn", randhex); + printchk("12 34 00 ca fe 00 aa 55", "%8pHX", randhex); + printchk("12 34 00 ca fe 00 aa 55", "%*pHX", + (int)sizeof(randhex), randhex); + printchk("12 34 00 ca", "%4pHX", randhex); + + printchk("", "%pHX", randhex); + + printchk("12:34:00:ca:fe:00:aa:55", "%8pHXc", randhex); + printchk("123400cafe00aa55", "%8pHXn", randhex); + + printchk("/test/pa\\ th/\\~spe\\ncial\\x01/file.name", "%pSE", + "/test/pa th/~spe\ncial\x01/file.name"); + printchk("/test/pa\\ th/\\~spe\\n", "%17pSE", + "/test/pa th/~spe\ncial\x01/file.name"); + + char nulltest[] = { 'n', 'u', 0, 'l', 'l' }; + + printchk("nu\\x00ll", "%5pSE", nulltest); + printchk("nu\\x00ll", "%*pSE", 5, nulltest); + + printchk("bl\\\"ah\\x01te[st\\nab]c", "%pSQ", + "bl\"ah\x01te[st\nab]c"); + printchk("\"bl\\\"ah\\x01te[st\\nab]c\"", "%pSQq", + "bl\"ah\x01te[st\nab]c"); + printchk("\"bl\\\"ah\\x01te[st\\x0aab\\]c\"", "%pSQqs", + "bl\"ah\x01te[st\nab]c"); + printchk("\"\"", "%pSQqn", ""); + printchk("\"\"", "%pSQqn", (char *)NULL); + printchk("(null)", "%pSQq", (char *)NULL); return !!errors; } From 94f78404952795538b72b752ef08d865122ecd09 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 26 Mar 2021 14:16:01 +0100 Subject: [PATCH 13/16] lib: fix & improve `%pSU` format This wasn't quite formatting IPv6+port in a useful way (no brackets), and printing the scope ID (interface index) and unix addrs is useful too. Signed-off-by: David Lamparter --- lib/sockunion.c | 31 ++++++++++++++++++++++++++++--- lib/sockunion.h | 7 +++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/lib/sockunion.c b/lib/sockunion.c index 2175ac3360..37bd3b841e 100644 --- a/lib/sockunion.c +++ b/lib/sockunion.c @@ -668,7 +668,7 @@ static ssize_t printfrr_psu(struct fbuf *buf, struct printfrr_eargs *ea, const void *ptr) { const union sockunion *su = ptr; - bool include_port = false; + bool include_port = false, include_scope = false; bool endflags = false; ssize_t ret = 0; char cbuf[INET6_ADDRSTRLEN]; @@ -682,6 +682,10 @@ static ssize_t printfrr_psu(struct fbuf *buf, struct printfrr_eargs *ea, ea->fmt++; include_port = true; break; + case 's': + ea->fmt++; + include_scope = true; + break; default: endflags = true; break; @@ -696,14 +700,35 @@ static ssize_t printfrr_psu(struct fbuf *buf, struct printfrr_eargs *ea, inet_ntop(AF_INET, &su->sin.sin_addr, cbuf, sizeof(cbuf)); ret += bputs(buf, cbuf); if (include_port) - ret += bprintfrr(buf, ":%d", su->sin.sin_port); + ret += bprintfrr(buf, ":%d", ntohs(su->sin.sin_port)); break; case AF_INET6: + if (include_port) + ret += bputch(buf, '['); inet_ntop(AF_INET6, &su->sin6.sin6_addr, cbuf, sizeof(cbuf)); ret += bputs(buf, cbuf); + if (include_scope && su->sin6.sin6_scope_id) + ret += bprintfrr(buf, "%%%u", + (unsigned int)su->sin6.sin6_scope_id); if (include_port) - ret += bprintfrr(buf, ":%d", su->sin6.sin6_port); + ret += bprintfrr(buf, "]:%d", + ntohs(su->sin6.sin6_port)); break; + case AF_UNIX: { + int len; +#ifdef __linux__ + if (su->sun.sun_path[0] == '\0' && su->sun.sun_path[1]) { + len = strnlen(su->sun.sun_path + 1, + sizeof(su->sun.sun_path) - 1); + ret += bprintfrr(buf, "@%*pSE", len, + su->sun.sun_path + 1); + break; + } +#endif + len = strnlen(su->sun.sun_path, sizeof(su->sun.sun_path)); + ret += bprintfrr(buf, "%*pSE", len, su->sun.sun_path); + break; + } default: ret += bprintfrr(buf, "(af %d)", sockunion_family(su)); } diff --git a/lib/sockunion.h b/lib/sockunion.h index 5e80ba1090..46ab7ff20e 100644 --- a/lib/sockunion.h +++ b/lib/sockunion.h @@ -24,6 +24,7 @@ #include "privs.h" #include "if.h" +#include #ifdef __OpenBSD__ #include #endif @@ -36,6 +37,7 @@ union sockunion { struct sockaddr sa; struct sockaddr_in sin; struct sockaddr_in6 sin6; + struct sockaddr_un sun; #ifdef __OpenBSD__ struct sockaddr_mpls smpls; struct sockaddr_rtlabel rtlabel; @@ -106,6 +108,11 @@ extern int sockunion_is_null(const union sockunion *su); #ifdef _FRR_ATTRIBUTE_PRINTFRR #pragma FRR printfrr_ext "%pSU" (union sockunion *) +#pragma FRR printfrr_ext "%pSU" (struct sockaddr *) +#pragma FRR printfrr_ext "%pSU" (struct sockaddr_storage *) +#pragma FRR printfrr_ext "%pSU" (struct sockaddr_in *) +#pragma FRR printfrr_ext "%pSU" (struct sockaddr_in6 *) +#pragma FRR printfrr_ext "%pSU" (struct sockaddr_un *) #endif #ifdef __cplusplus From 7b183fd8ea23a03480fb40616ca0e8efaf0c1f76 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 26 Mar 2021 14:20:08 +0100 Subject: [PATCH 14/16] lib: add `%dPF` & `%dSO` formats Just quick helpers to print `AF_*` and `SOCK_*` constants. Signed-off-by: David Lamparter --- lib/sockunion.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++ lib/sockunion.h | 5 +++++ 2 files changed, 51 insertions(+) diff --git a/lib/sockunion.c b/lib/sockunion.c index 37bd3b841e..e6340a1743 100644 --- a/lib/sockunion.c +++ b/lib/sockunion.c @@ -752,3 +752,49 @@ int sockunion_is_null(const union sockunion *su) return 0; } } + +printfrr_ext_autoreg_i("PF", printfrr_pf) +static ssize_t printfrr_pf(struct fbuf *buf, struct printfrr_eargs *ea, + uintmax_t val) +{ + switch (val) { + case AF_INET: + return bputs(buf, "AF_INET"); + case AF_INET6: + return bputs(buf, "AF_INET6"); + case AF_UNIX: + return bputs(buf, "AF_UNIX"); +#ifdef AF_PACKET + case AF_PACKET: + return bputs(buf, "AF_PACKET"); +#endif +#ifdef AF_NETLINK + case AF_NETLINK: + return bputs(buf, "AF_NETLINK"); +#endif + } + return bprintfrr(buf, "AF_(%ju)", val); +} + +printfrr_ext_autoreg_i("SO", printfrr_so) +static ssize_t printfrr_so(struct fbuf *buf, struct printfrr_eargs *ea, + uintmax_t val) +{ + switch (val) { + case SOCK_STREAM: + return bputs(buf, "SOCK_STREAM"); + case SOCK_DGRAM: + return bputs(buf, "SOCK_DGRAM"); + case SOCK_SEQPACKET: + return bputs(buf, "SOCK_SEQPACKET"); +#ifdef SOCK_RAW + case SOCK_RAW: + return bputs(buf, "SOCK_RAW"); +#endif +#ifdef SOCK_PACKET + case SOCK_PACKET: + return bputs(buf, "SOCK_PACKET"); +#endif + } + return bprintfrr(buf, "SOCK_(%ju)", val); +} diff --git a/lib/sockunion.h b/lib/sockunion.h index 46ab7ff20e..2cc80bb70f 100644 --- a/lib/sockunion.h +++ b/lib/sockunion.h @@ -113,6 +113,11 @@ extern int sockunion_is_null(const union sockunion *su); #pragma FRR printfrr_ext "%pSU" (struct sockaddr_in *) #pragma FRR printfrr_ext "%pSU" (struct sockaddr_in6 *) #pragma FRR printfrr_ext "%pSU" (struct sockaddr_un *) + +/* AF_INET/PF_INET & co., using "PF" to avoid confusion with AFI/SAFI */ +#pragma FRR printfrr_ext "%dPF" (int) +/* SOCK_STREAM & co. */ +#pragma FRR printfrr_ext "%dSO" (int) #endif #ifdef __cplusplus From 2c12c904ea34b96098f2390f97921ba89b40cb19 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Fri, 26 Mar 2021 14:27:51 +0100 Subject: [PATCH 15/16] lib: save errno in vty_out() ... so `%m` works correctly, without us trampling over `errno` before we get to formatting it. Signed-off-by: David Lamparter --- lib/vty.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/vty.c b/lib/vty.c index d44cc904c5..96cfef1c0a 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -159,6 +159,8 @@ int vty_out(struct vty *vty, const char *format, ...) char buf[1024]; char *p = NULL; char *filtered; + /* format string may contain %m, keep errno intact for printfrr */ + int saved_errno = errno; if (vty->frame_pos) { vty->frame_pos = 0; @@ -166,6 +168,7 @@ int vty_out(struct vty *vty, const char *format, ...) } va_start(args, format); + errno = saved_errno; p = vasnprintfrr(MTYPE_VTY_OUT_BUF, buf, sizeof(buf), format, args); va_end(args); From 19b1a1c6a975f772b22dda9c5c42c6288e2ce459 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Sun, 21 Mar 2021 17:35:43 +0100 Subject: [PATCH 16/16] doc/developer: improve printfrr extension docs The table is getting rather clunky, let's just break this out and make it pretty there. Signed-off-by: David Lamparter --- doc/developer/_static/overrides.css | 16 ++ doc/developer/conf.py | 28 +++ doc/developer/logging.rst | 309 +++++++++++++++++++++++----- 3 files changed, 299 insertions(+), 54 deletions(-) diff --git a/doc/developer/_static/overrides.css b/doc/developer/_static/overrides.css index 1d702bb6e9..302b8d6bd7 100644 --- a/doc/developer/_static/overrides.css +++ b/doc/developer/_static/overrides.css @@ -214,6 +214,22 @@ pre { .highlight .na { color: var(--primary-2); } .highlight .nv { color: var(--complement-0); } +.rst-content code.frrfmtout { + background-color: var(--secondary-1-9); + border-color: var(--secondary-1-1); + font-size:100%; +} +.rst-content code.frrfmtout::before { + content: "⇒ \""; +} +.rst-content code.frrfmtout::after { + content: "\""; +} +.rst-content code.frrfmtout span { + color: var(--secondary-1-4); + font-size:100%; +} + strong { font-weight:500; } diff --git a/doc/developer/conf.py b/doc/developer/conf.py index f4bb65ec79..20265f4aad 100644 --- a/doc/developer/conf.py +++ b/doc/developer/conf.py @@ -17,6 +17,8 @@ import os import re import pygments from sphinx.highlighting import lexers +from sphinx.util import logging +logger = logging.getLogger(__name__) # If extensions (or modules to document with autodoc) are in another directory, # add these directories to sys.path here. If the directory is relative to the @@ -362,11 +364,37 @@ texinfo_documents = [ with open("../extra/frrlexer.py", "rb") as lex: frrlexerpy = lex.read() +frrfmt_re = re.compile(r'^\s*%(?P[^\s]+)\s+\((?P.*)\)\s*$') + +def parse_frrfmt(env, text, node): + from sphinx import addnodes + + m = frrfmt_re.match(text) + if not m: + logger.warning('could not parse frrfmt:: %r' % (text), location=node) + node += addnodes.desc_name(text, text) + return text + + spec, types = m.group('spec'), m.group('types') + + node += addnodes.desc_sig_operator('%', '%') + node += addnodes.desc_name(spec + ' ', spec + ' ') + plist = addnodes.desc_parameterlist() + for typ in types.split(','): + typ = typ.strip() + plist += addnodes.desc_parameter(typ, typ) + node += plist + return '%' + spec + # custom extensions here def setup(app): # object type for FRR CLI commands, can be extended to document parent CLI # node later on app.add_object_type("clicmd", "clicmd") + + # printfrr extensions + app.add_object_type("frrfmt", "frrfmt", parse_node=parse_frrfmt) + # css overrides for HTML theme app.add_stylesheet("overrides.css") # load Pygments lexer for FRR config syntax diff --git a/doc/developer/logging.rst b/doc/developer/logging.rst index f19554b532..b827afd6cc 100644 --- a/doc/developer/logging.rst +++ b/doc/developer/logging.rst @@ -1,5 +1,7 @@ .. _logging: +.. highlight:: c + Logging ======= @@ -52,68 +54,30 @@ are available: if (ret != buf) XFREE(MTYPE_FOO, ret); -Extensions -^^^^^^^^^^ +.. c:function:: ssize_t bprintfrr(struct fbuf *fb, const char *fmt, ...) +.. c:function:: ssize_t vbprintfrr(struct fbuf *fb, const char *fmt, va_list) -``printfrr()`` format strings can be extended with suffixes after `%p` or -`%d`. The following extended format specifiers are available: + These are the "lowest level" functions, which the other variants listed + above use to implement their functionality on top. Mainly useful for + implementing printfrr extensions since those get a ``struct fbuf *`` to + write their output to. -+-----------+--------------------------+----------------------------------------------+ -| Specifier | Argument | Output | -+===========+==========================+==============================================+ -| ``%Lu`` | ``uint64_t`` | ``12345`` | -+-----------+--------------------------+----------------------------------------------+ -| ``%Ld`` | ``int64_t`` | ``-12345`` | -+-----------+--------------------------+----------------------------------------------+ -| ``%pVA`` | ``struct va_format *`` | (recursive printfrr) | -+-----------+--------------------------+----------------------------------------------+ -| ``%pI4`` | ``struct in_addr *`` | ``1.2.3.4`` | -| | | | -| | ``in_addr_t *`` | | -+-----------+--------------------------+----------------------------------------------+ -| ``%pI6`` | ``struct in6_addr *`` | ``fe80::1234`` | -+-----------+--------------------------+----------------------------------------------+ -| ``%pIA`` | ``struct ipaddr *`` | ``1.2.3.4`` | -| | | | -| | | ``fe80::1234`` | -+-----------+--------------------------+----------------------------------------------+ -| ``%pFX`` | ``struct prefix *`` | ``fe80::1234/64`` | -+-----------+--------------------------+----------------------------------------------+ -| ``%pSG4`` | ``struct prefix_sg *`` | ``(*,1.2.3.4)`` | -+-----------+--------------------------+----------------------------------------------+ -| ``%pRN`` | ``struct route_node *`` | ``192.168.1.0/24`` (dst-only node) | -| | | | -| | | ``2001:db8::/32 from fe80::/64`` (SADR node) | -+-----------+--------------------------+----------------------------------------------+ -| ``%pNHv`` | ``struct nexthop *`` | ``1.2.3.4, via eth0`` | -+-----------+--------------------------+----------------------------------------------+ -| ``%pNHs`` | ``struct nexthop *`` | ``1.2.3.4 if 15`` | -+-----------+--------------------------+----------------------------------------------+ -| ``%pFX`` | ``struct bgp_dest *`` | ``fe80::1234/64`` (available in BGP only) | -+-----------+--------------------------+----------------------------------------------+ -| ``%.*pHX``| ``int len, void *ptr`` | ``12 34 56 78`` (hexdump) | -+-----------+--------------------------+----------------------------------------------+ +.. c:macro:: FMT_NSTD(expr) -Printf features like field lengths can be used normally with these extensions, -e.g. ``%-15pI4`` works correctly. + This macro turns off/on format warnings as needed when non-ISO-C + compatible printfrr extensions are used (e.g. ``%.*p`` or ``%Ld``.):: -The extension specifier after ``%p`` or ``%d`` is always an uppercase letter; -by means of established pattern uppercase letters and numbers form the type -identifier which may be followed by lowercase flags. + vty_out(vty, "standard compatible %pI4\n", &addr); + FMT_NSTD(vty_out(vty, "non-standard %-47.*pHX\n", (int)len, buf)); -You can grep the FRR source for ``printfrr_ext_autoreg`` to see all extended -printers and what exactly they do. More printers are likely to be added as -needed/useful, so the list above may become outdated. - -``%Ld`` is not an "extension" for printfrr; it's wired directly into the main -printf logic. + When the frr-format plugin is in use, this macro is a no-op since the + frr-format plugin supports all printfrr extensions. Since the FRR CI + includes a system with the plugin enabled, this means format errors will + not slip by undetected even with FMT_NSTD. .. note:: - The ``zlog_*``/``flog_*`` and ``vty_out`` functions all use printfrr - internally, so these extensions are available there. However, they are - **not** available when calling ``snprintf`` directly. You need to call - ``snprintfrr`` instead. + ``printfrr()`` does not support the ``%n`` format. AS-Safety ^^^^^^^^^ @@ -126,6 +90,243 @@ AS-Safety while AS-Safe) * extensions are only AS-Safe if their printer is AS-Safe +printfrr Extensions +------------------- + +``printfrr()`` format strings can be extended with suffixes after `%p` or `%d`. +Printf features like field lengths can be used normally with these extensions, +e.g. ``%-15pI4`` works correctly, **except if the extension consumes the +width or precision**. Extensions that do so are listed below as ``%*pXX`` +rather than ``%pXX``. + +The extension specifier after ``%p`` or ``%d`` is always an uppercase letter; +by means of established pattern uppercase letters and numbers form the type +identifier which may be followed by lowercase flags. + +You can grep the FRR source for ``printfrr_ext_autoreg`` to see all extended +printers and what exactly they do. More printers are likely to be added as +needed/useful, so the list here may be outdated. + +.. note:: + + The ``zlog_*``/``flog_*`` and ``vty_out`` functions all use printfrr + internally, so these extensions are available there. However, they are + **not** available when calling ``snprintf`` directly. You need to call + ``snprintfrr`` instead. + +Networking data types +^^^^^^^^^^^^^^^^^^^^^ + +.. role:: frrfmtout(code) + +.. frrfmt:: %pI4 (struct in_addr *, in_addr_t *) + + :frrfmtout:`1.2.3.4` + +.. frrfmt:: %pI6 (struct in6_addr *) + + :frrfmtout:`fe80::1234` + +.. frrfmt:: %pEA (struct ethaddr *) + + :frrfmtout:`01:23:45:67:89:ab` + +.. frrfmt:: %pIA (struct ipaddr *) + + :frrfmtout:`1.2.3.4` / :frrfmtout:`fe80::1234` + +.. frrfmt:: %pFX (struct prefix *) + + :frrfmtout:`1.2.3.0/24` / :frrfmtout:`fe80::1234/64` + + This accepts the following types: + + - :c:struct:`prefix` + - :c:struct:`prefix_ipv4` + - :c:struct:`prefix_ipv6` + - :c:struct:`prefix_eth` + - :c:struct:`prefix_evpn` + - :c:struct:`prefix_fs` + + It does **not** accept the following types: + + - :c:struct:`prefix_ls` + - :c:struct:`prefix_rd` + - :c:struct:`prefix_ptr` + - :c:struct:`prefix_sg` (use :frrfmt:`%pSG4`) + - :c:union:`prefixptr` (dereference to get :c:struct:`prefix`) + - :c:union:`prefixconstptr` (dereference to get :c:struct:`prefix`) + +.. frrfmt:: %pSG4 (struct prefix_sg *) + + :frrfmtout:`(*,1.2.3.4)` + + This is *(S,G)* output for use in pimd. (Note prefix_sg is not a prefix + "subclass" like the other prefix_* structs.) + +.. frrfmt:: %pSU (union sockunion *) + + ``%pSU``: :frrfmtout:`1.2.3.4` / :frrfmtout:`fe80::1234` + + ``%pSUs``: :frrfmtout:`1.2.3.4` / :frrfmtout:`fe80::1234%89` + (adds IPv6 scope ID as integer) + + ``%pSUp``: :frrfmtout:`1.2.3.4:567` / :frrfmtout:`[fe80::1234]:567` + (adds port) + + ``%pSUps``: :frrfmtout:`1.2.3.4:567` / :frrfmtout:`[fe80::1234%89]:567` + (adds port and scope ID) + +.. frrfmt:: %pRN (struct route_node *, struct bgp_node *, struct agg_node *) + + :frrfmtout:`192.168.1.0/24` (dst-only node) + + :frrfmtout:`2001:db8::/32 from fe80::/64` (SADR node) + +.. frrfmt:: %pNH (struct nexthop *) + + ``%pNHvv``: :frrfmtout:`via 1.2.3.4, eth0` — verbose zebra format + + ``%pNHv``: :frrfmtout:`1.2.3.4, via eth0` — slightly less verbose zebra format + + ``%pNHs``: :frrfmtout:`1.2.3.4 if 15` — same as :c:func:`nexthop2str()` + +.. frrfmt:: %pBD (struct bgp_dest *) + + :frrfmtout:`fe80::1234/64` + + (only available in bgpd.) + +.. frrfmt:: %dPF (int) + + :frrfmtout:`AF_INET` + + Prints an `AF_*` / `PF_*` constant. ``PF`` is used here to avoid confusion + with `AFI` constants, even though the FRR codebase prefers `AF_INET` over + `PF_INET` & co. + +.. frrfmt:: %dSO (int) + + :frrfmtout:`SOCK_STREAM` + +General utility formats +^^^^^^^^^^^^^^^^^^^^^^^ + +.. frrfmt:: %m (no argument) + + :frrfmtout:`Permission denied` + + Prints ``strerror(errno)``. Does **not** consume any input argument, don't + pass ``errno``! + + (This is a GNU extension not specific to FRR. FRR guarantees it is + available on all systems in printfrr, though BSDs support it in printf too.) + +.. frrfmt:: %pSQ (char *) + + ([S]tring [Q]uote.) Like ``%s``, but produce a quoted string. Options: + + ``n`` - treat ``NULL`` as empty string instead. + + ``q`` - include ``""`` quotation marks. Note: ``NULL`` is printed as + ``(null)``, not ``"(null)"`` unless ``n`` is used too. This is + intentional. + + ``s`` - use escaping suitable for RFC5424 syslog. This means ``]`` is + escaped too. + + If a length is specified (``%*pSQ`` or ``%.*pSQ``), null bytes in the input + string do not end the string and are just printed as ``\x00``. + +.. frrfmt:: %pSE (char *) + + ([S]tring [E]scape.) Like ``%s``, but escape special characters. + Options: + + ``n`` - treat ``NULL`` as empty string instead. + + Unlike :frrfmt:`%pSQ`, this escapes many more characters that are fine for + a quoted string but not on their own. + + If a length is specified (``%*pSE`` or ``%.*pSE``), null bytes in the input + string do not end the string and are just printed as ``\x00``. + +.. frrfmt:: %pVA (struct va_format *) + + Recursively invoke printfrr, with arguments passed in through: + + .. c:struct:: va_format + + .. c:member:: const char *fmt + + Format string to use for the recursive printfrr call. + + .. c:member:: va_list *va + + Formatting arguments. Note this is passed as a pointer, not - as in + most other places - a direct struct reference. Internally uses + ``va_copy()`` so repeated calls can be made (e.g. for determining + output length.) + +.. frrfmt:: %pFB (struct fbuf *) + + Insert text from a ``struct fbuf *``, i.e. the output of a call to + :c:func:`bprintfrr()`. + +.. frrfmt:: %*pHX (void *, char *, unsigned char *) + + ``%pHX``: :frrfmtout:`12 34 56 78` + + ``%pHXc``: :frrfmtout:`12:34:56:78` (separate with [c]olon) + + ``%pHXn``: :frrfmtout:`12345678` (separate with [n]othing) + + Insert hexdump. This specifier requires a precision or width to be + specified. A precision (``%.*pHX``) takes precedence, but generates a + compiler warning since precisions are undefined for ``%p`` in ISO C. If + no precision is given, the width is used instead (and normal handling of + the width is suppressed). + + Note that width and precision are ``int`` arguments, not ``size_t``. Use + like:: + + char *buf; + size_t len; + + snprintfrr(out, sizeof(out), "... %*pHX ...", (int)len, buf); + + /* with padding to width - would generate a warning due to %.*p */ + FMT_NSTD(snprintfrr(out, sizeof(out), "... %-47.*pHX ...", (int)len, buf)); + +.. frrfmt:: %*pHS (void *, char *, unsigned char *) + + ``%pHS``: :frrfmtout:`hex.dump` + + This is a complementary format for :frrfmt:`%*pHX` to print the text + representation for a hexdump. Non-printable characters are replaced with + a dot. + +Integer formats +^^^^^^^^^^^^^^^ + +.. note:: + + These formats currently only exist for advanced type checking with the + ``frr-format`` GCC plugin. They should not be used directly since they will + cause compiler warnings when used without the plugin. Use with + :c:macro:`FMT_NSTD` if necessary. + + It is possible ISO C23 may introduce another format for these, possibly + ``%w64d`` discussed in `JTC 1/SC 22/WG 14/N2680 `_. + +.. frrfmt:: %Lu (uint64_t) + + :frrfmtout:`12345` + +.. frrfmt:: %Ld (int64_t) + + :frrfmtout:`-12345` + Log levels ----------