* [PATCH iproute2] json_print: fix print_uint hidden type promotion @ 2018-03-29 19:22 Kevin Darbyshire-Bryant 2018-03-29 21:02 ` Stephen Hemminger 2018-04-25 14:30 ` [PATCH iproute2 v2] json_print: Fix hidden 64-bit " Toke Høiland-Jørgensen 0 siblings, 2 replies; 9+ messages in thread From: Kevin Darbyshire-Bryant @ 2018-03-29 19:22 UTC (permalink / raw) To: netdev; +Cc: Kevin Darbyshire-Bryant print_int used 'int' type internally, whereas print_uint used 'uint64_t' These helper functions eventually call vfprintf(fp, fmt, args) which is a variable argument list function and is dependent upon 'fmt' containing correct information about the length of the passed arguments. Unfortunately print_int v print_uint offered no clue to the programmer that internally passed ints to print_uint were being promoted to 64bits, thus the format passed in 'fmt' string vs the actual passed integer could be different lengths. This is even more interesting on big endian architectures where 'vfprintf' would be looking in the middle of an int64 type. print_u/int now stick with native int size. Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> --- include/json_print.h | 2 +- lib/json_print.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/json_print.h b/include/json_print.h index 2ca7830a..45bc653d 100644 --- a/include/json_print.h +++ b/include/json_print.h @@ -56,10 +56,10 @@ void close_json_array(enum output_type type, const char *delim); print_color_##type_name(t, COLOR_NONE, key, fmt, value); \ } _PRINT_FUNC(int, int); +_PRINT_FUNC(uint, unsigned int); _PRINT_FUNC(bool, bool); _PRINT_FUNC(null, const char*); _PRINT_FUNC(string, const char*); -_PRINT_FUNC(uint, uint64_t); _PRINT_FUNC(hu, unsigned short); _PRINT_FUNC(hex, unsigned int); _PRINT_FUNC(0xhex, unsigned int); diff --git a/lib/json_print.c b/lib/json_print.c index 6518ba98..8d54d1d4 100644 --- a/lib/json_print.c +++ b/lib/json_print.c @@ -117,8 +117,8 @@ void close_json_array(enum output_type type, const char *str) } \ } _PRINT_FUNC(int, int); +_PRINT_FUNC(uint, unsigned int); _PRINT_FUNC(hu, unsigned short); -_PRINT_FUNC(uint, uint64_t); _PRINT_FUNC(lluint, unsigned long long int); _PRINT_FUNC(float, double); #undef _PRINT_FUNC -- 2.14.3 (Apple Git-98) ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH iproute2] json_print: fix print_uint hidden type promotion 2018-03-29 19:22 [PATCH iproute2] json_print: fix print_uint hidden type promotion Kevin Darbyshire-Bryant @ 2018-03-29 21:02 ` Stephen Hemminger 2018-03-29 21:29 ` Kevin Darbyshire-Bryant 2018-04-25 14:30 ` [PATCH iproute2 v2] json_print: Fix hidden 64-bit " Toke Høiland-Jørgensen 1 sibling, 1 reply; 9+ messages in thread From: Stephen Hemminger @ 2018-03-29 21:02 UTC (permalink / raw) To: Kevin Darbyshire-Bryant; +Cc: netdev On Thu, 29 Mar 2018 20:22:20 +0100 Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> wrote: > print_int used 'int' type internally, whereas print_uint used 'uint64_t' > > These helper functions eventually call vfprintf(fp, fmt, args) which is > a variable argument list function and is dependent upon 'fmt' containing > correct information about the length of the passed arguments. > > Unfortunately print_int v print_uint offered no clue to the programmer > that internally passed ints to print_uint were being promoted to 64bits, > thus the format passed in 'fmt' string vs the actual passed integer > could be different lengths. This is even more interesting on big endian > architectures where 'vfprintf' would be looking in the middle of an > int64 type. > > print_u/int now stick with native int size. > > Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> > --- > include/json_print.h | 2 +- > lib/json_print.c | 2 +- > 2 files changed, 2 insertions(+), 2 deletions(-) > > diff --git a/include/json_print.h b/include/json_print.h > index 2ca7830a..45bc653d 100644 > --- a/include/json_print.h > +++ b/include/json_print.h > @@ -56,10 +56,10 @@ void close_json_array(enum output_type type, const char *delim); > print_color_##type_name(t, COLOR_NONE, key, fmt, value); \ > } > _PRINT_FUNC(int, int); > +_PRINT_FUNC(uint, unsigned int); > _PRINT_FUNC(bool, bool); > _PRINT_FUNC(null, const char*); > _PRINT_FUNC(string, const char*); > -_PRINT_FUNC(uint, uint64_t); > _PRINT_FUNC(hu, unsigned short); > _PRINT_FUNC(hex, unsigned int); > _PRINT_FUNC(0xhex, unsigned int); > diff --git a/lib/json_print.c b/lib/json_print.c > index 6518ba98..8d54d1d4 100644 > --- a/lib/json_print.c > +++ b/lib/json_print.c > @@ -117,8 +117,8 @@ void close_json_array(enum output_type type, const char *str) > } \ > } > _PRINT_FUNC(int, int); > +_PRINT_FUNC(uint, unsigned int); > _PRINT_FUNC(hu, unsigned short); > -_PRINT_FUNC(uint, uint64_t); > _PRINT_FUNC(lluint, unsigned long long int); > _PRINT_FUNC(float, double); > #undef _PRINT_FUNC I am concerned that this will break output of 64 bit statistics on 32 bit hosts. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH iproute2] json_print: fix print_uint hidden type promotion 2018-03-29 21:02 ` Stephen Hemminger @ 2018-03-29 21:29 ` Kevin Darbyshire-Bryant 0 siblings, 0 replies; 9+ messages in thread From: Kevin Darbyshire-Bryant @ 2018-03-29 21:29 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev@vger.kernel.org > On 29 Mar 2018, at 22:02, Stephen Hemminger <stephen@networkplumber.org> wrote: > > On Thu, 29 Mar 2018 20:22:20 +0100 > Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> wrote: > >> print_int used 'int' type internally, whereas print_uint used 'uint64_t' >> >> These helper functions eventually call vfprintf(fp, fmt, args) which is >> a variable argument list function and is dependent upon 'fmt' containing >> correct information about the length of the passed arguments. >> >> Unfortunately print_int v print_uint offered no clue to the programmer >> that internally passed ints to print_uint were being promoted to 64bits, >> thus the format passed in 'fmt' string vs the actual passed integer >> could be different lengths. This is even more interesting on big endian >> architectures where 'vfprintf' would be looking in the middle of an >> int64 type. >> >> print_u/int now stick with native int size. >> >> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> >> --- >> include/json_print.h | 2 +- >> lib/json_print.c | 2 +- >> 2 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/include/json_print.h b/include/json_print.h >> index 2ca7830a..45bc653d 100644 >> --- a/include/json_print.h >> +++ b/include/json_print.h >> @@ -56,10 +56,10 @@ void close_json_array(enum output_type type, const char *delim); >> print_color_##type_name(t, COLOR_NONE, key, fmt, value); \ >> } >> _PRINT_FUNC(int, int); >> +_PRINT_FUNC(uint, unsigned int); >> _PRINT_FUNC(bool, bool); >> _PRINT_FUNC(null, const char*); >> _PRINT_FUNC(string, const char*); >> -_PRINT_FUNC(uint, uint64_t); >> _PRINT_FUNC(hu, unsigned short); >> _PRINT_FUNC(hex, unsigned int); >> _PRINT_FUNC(0xhex, unsigned int); >> diff --git a/lib/json_print.c b/lib/json_print.c >> index 6518ba98..8d54d1d4 100644 >> --- a/lib/json_print.c >> +++ b/lib/json_print.c >> @@ -117,8 +117,8 @@ void close_json_array(enum output_type type, const char *str) >> } \ >> } >> _PRINT_FUNC(int, int); >> +_PRINT_FUNC(uint, unsigned int); >> _PRINT_FUNC(hu, unsigned short); >> -_PRINT_FUNC(uint, uint64_t); >> _PRINT_FUNC(lluint, unsigned long long int); >> _PRINT_FUNC(float, double); >> #undef _PRINT_FUNC > > > I am concerned that this will break output of 64 bit statistics on 32 bit hosts. I honestly don’t know what to do. Without the patch I see breakage on <33 bit stats with 32 bit big endian hosts ‘cos the printf formatting doesn’t know the type passed internally by the function is 64bits long. e.g. tc qdisc qdisc noqueue 0: dev lo root refcnt 4486716 qdisc fq_codel 0: dev eth1 root refcnt 4486716 limit 4498840p flows 4536204 quantum 4539856 target 5.0ms interval 100.0ms memory_limit 4Mb ecn qdisc noqueue 0: dev br-lan root refcnt 4486716 qdisc noqueue 0: dev eth1.2 root refcnt 4486716 qdisc noqueue 0: dev br-wifi_guest root refcnt 4486716 qdisc noqueue 0: dev eth1.15 root refcnt 4486716 qdisc noqueue 0: dev wlan1 root refcnt 4486716 qdisc noqueue 0: dev wlan0 root refcnt 4486716 qdisc noqueue 0: dev wlan1-1 root refcnt 4486716 qdisc noqueue 0: dev wlan0-1 root refcnt 4486716 I guess _PRINT_FUNC(int, int) could be _PRINT_FUNC(int, int64_t) and then at least we’d be consistent in doing hidden promotions and see breakage for both signed & unsigned types on certain architectures. But I think I’ve hit my (lack of) skill limit and don’t really know how to take this further forward, or wish to break more protocols :-) ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH iproute2 v2] json_print: Fix hidden 64-bit type promotion 2018-03-29 19:22 [PATCH iproute2] json_print: fix print_uint hidden type promotion Kevin Darbyshire-Bryant 2018-03-29 21:02 ` Stephen Hemminger @ 2018-04-25 14:30 ` Toke Høiland-Jørgensen 2018-04-25 14:55 ` Stephen Hemminger 2018-04-25 15:28 ` [PATCH iproute2 v3] " Toke Høiland-Jørgensen 1 sibling, 2 replies; 9+ messages in thread From: Toke Høiland-Jørgensen @ 2018-04-25 14:30 UTC (permalink / raw) To: netdev; +Cc: Toke Høiland-Jørgensen, Kevin Darbyshire-Bryant print_uint() will silently promote its variable type to uint64_t, but there is nothing that ensures that the format string specifier passed along with it fits (and the function name suggest to pass "%u"). Fix this by changing print_uint() to use a native 'unsigned int' type, and introduce a separate print_u64() function for printing 64-bit values. All call sites that were actually printing 64-bit values using print_uint() are converted to use print_u64() instead. Since print_int() was already using native int types, just add a print_s64() to match, but don't convert any call sites. Cc: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk> --- Changes since v1 (which was sent by Kevin Darbyshire-Bryant): - Add print_u64() and convert call sites that were actually passing 64-bit values to print_uint(). include/json_print.h | 4 ++- include/json_writer.h | 12 ++++++--- ip/ipaddress.c | 62 +++++++++++++++++++++---------------------- ip/ipmacsec.c | 8 +++--- ip/ipmroute.c | 6 ++--- lib/json_print.c | 4 ++- lib/json_writer.c | 30 ++++++++++++++++++--- 7 files changed, 78 insertions(+), 48 deletions(-) diff --git a/include/json_print.h b/include/json_print.h index 2ca7830a..4677618e 100644 --- a/include/json_print.h +++ b/include/json_print.h @@ -56,10 +56,12 @@ void close_json_array(enum output_type type, const char *delim); print_color_##type_name(t, COLOR_NONE, key, fmt, value); \ } _PRINT_FUNC(int, int); +_PRINT_FUNC(s64, int64_t); _PRINT_FUNC(bool, bool); _PRINT_FUNC(null, const char*); _PRINT_FUNC(string, const char*); -_PRINT_FUNC(uint, uint64_t); +_PRINT_FUNC(uint, unsigned int); +_PRINT_FUNC(u64, uint64_t); _PRINT_FUNC(hu, unsigned short); _PRINT_FUNC(hex, unsigned int); _PRINT_FUNC(0xhex, unsigned int); diff --git a/include/json_writer.h b/include/json_writer.h index 4b4dec28..9d3f37f8 100644 --- a/include/json_writer.h +++ b/include/json_writer.h @@ -34,10 +34,12 @@ void jsonw_string(json_writer_t *self, const char *value); void jsonw_bool(json_writer_t *self, bool value); void jsonw_float(json_writer_t *self, double number); void jsonw_float_fmt(json_writer_t *self, const char *fmt, double num); -void jsonw_uint(json_writer_t *self, uint64_t number); +void jsonw_uint(json_writer_t *self, unsigned int number); +void jsonw_u64(json_writer_t *self, uint64_t number); void jsonw_xint(json_writer_t *self, uint64_t number); void jsonw_hu(json_writer_t *self, unsigned short number); -void jsonw_int(json_writer_t *self, int64_t number); +void jsonw_int(json_writer_t *self, int number); +void jsonw_s64(json_writer_t *self, int64_t number); void jsonw_null(json_writer_t *self); void jsonw_lluint(json_writer_t *self, unsigned long long int num); @@ -45,10 +47,12 @@ void jsonw_lluint(json_writer_t *self, unsigned long long int num); void jsonw_string_field(json_writer_t *self, const char *prop, const char *val); void jsonw_bool_field(json_writer_t *self, const char *prop, bool value); void jsonw_float_field(json_writer_t *self, const char *prop, double num); -void jsonw_uint_field(json_writer_t *self, const char *prop, uint64_t num); +void jsonw_uint_field(json_writer_t *self, const char *prop, unsigned int num); +void jsonw_u64_field(json_writer_t *self, const char *prop, uint64_t num); void jsonw_xint_field(json_writer_t *self, const char *prop, uint64_t num); void jsonw_hu_field(json_writer_t *self, const char *prop, unsigned short num); -void jsonw_int_field(json_writer_t *self, const char *prop, int64_t num); +void jsonw_int_field(json_writer_t *self, const char *prop, int num); +void jsonw_s64_field(json_writer_t *self, const char *prop, int64_t num); void jsonw_null_field(json_writer_t *self, const char *prop); void jsonw_lluint_field(json_writer_t *self, const char *prop, unsigned long long int num); diff --git a/ip/ipaddress.c b/ip/ipaddress.c index aecc9a1d..75539e05 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -554,21 +554,21 @@ static void print_vf_stats64(FILE *fp, struct rtattr *vfstats) /* RX stats */ open_json_object("rx"); - print_uint(PRINT_JSON, "bytes", NULL, + print_u64(PRINT_JSON, "bytes", NULL, rta_getattr_u64(vf[IFLA_VF_STATS_RX_BYTES])); - print_uint(PRINT_JSON, "packets", NULL, + print_u64(PRINT_JSON, "packets", NULL, rta_getattr_u64(vf[IFLA_VF_STATS_RX_PACKETS])); - print_uint(PRINT_JSON, "multicast", NULL, + print_u64(PRINT_JSON, "multicast", NULL, rta_getattr_u64(vf[IFLA_VF_STATS_MULTICAST])); - print_uint(PRINT_JSON, "broadcast", NULL, + print_u64(PRINT_JSON, "broadcast", NULL, rta_getattr_u64(vf[IFLA_VF_STATS_BROADCAST])); close_json_object(); /* TX stats */ open_json_object("tx"); - print_uint(PRINT_JSON, "tx_bytes", NULL, + print_u64(PRINT_JSON, "tx_bytes", NULL, rta_getattr_u64(vf[IFLA_VF_STATS_TX_BYTES])); - print_uint(PRINT_JSON, "tx_packets", NULL, + print_u64(PRINT_JSON, "tx_packets", NULL, rta_getattr_u64(vf[IFLA_VF_STATS_TX_PACKETS])); close_json_object(); close_json_object(); @@ -608,69 +608,69 @@ static void __print_link_stats(FILE *fp, struct rtattr *tb[]) /* RX stats */ open_json_object("rx"); - print_uint(PRINT_JSON, "bytes", NULL, s->rx_bytes); - print_uint(PRINT_JSON, "packets", NULL, s->rx_packets); - print_uint(PRINT_JSON, "errors", NULL, s->rx_errors); - print_uint(PRINT_JSON, "dropped", NULL, s->rx_dropped); - print_uint(PRINT_JSON, "over_errors", NULL, s->rx_over_errors); - print_uint(PRINT_JSON, "multicast", NULL, s->multicast); + print_u64(PRINT_JSON, "bytes", NULL, s->rx_bytes); + print_u64(PRINT_JSON, "packets", NULL, s->rx_packets); + print_u64(PRINT_JSON, "errors", NULL, s->rx_errors); + print_u64(PRINT_JSON, "dropped", NULL, s->rx_dropped); + print_u64(PRINT_JSON, "over_errors", NULL, s->rx_over_errors); + print_u64(PRINT_JSON, "multicast", NULL, s->multicast); if (s->rx_compressed) - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "compressed", NULL, s->rx_compressed); /* RX error stats */ if (show_stats > 1) { - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "length_errors", NULL, s->rx_length_errors); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "crc_errors", NULL, s->rx_crc_errors); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "frame_errors", NULL, s->rx_frame_errors); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "fifo_errors", NULL, s->rx_fifo_errors); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "missed_errors", NULL, s->rx_missed_errors); if (s->rx_nohandler) - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "nohandler", NULL, s->rx_nohandler); } close_json_object(); /* TX stats */ open_json_object("tx"); - print_uint(PRINT_JSON, "bytes", NULL, s->tx_bytes); - print_uint(PRINT_JSON, "packets", NULL, s->tx_packets); - print_uint(PRINT_JSON, "errors", NULL, s->tx_errors); - print_uint(PRINT_JSON, "dropped", NULL, s->tx_dropped); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "bytes", NULL, s->tx_bytes); + print_u64(PRINT_JSON, "packets", NULL, s->tx_packets); + print_u64(PRINT_JSON, "errors", NULL, s->tx_errors); + print_u64(PRINT_JSON, "dropped", NULL, s->tx_dropped); + print_u64(PRINT_JSON, "carrier_errors", NULL, s->tx_carrier_errors); - print_uint(PRINT_JSON, "collisions", NULL, s->collisions); + print_u64(PRINT_JSON, "collisions", NULL, s->collisions); if (s->tx_compressed) - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "compressed", NULL, s->tx_compressed); /* TX error stats */ if (show_stats > 1) { - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "aborted_errors", NULL, s->tx_aborted_errors); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "fifo_errors", NULL, s->tx_fifo_errors); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "window_errors", NULL, s->tx_window_errors); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "heartbeat_errors", NULL, s->tx_heartbeat_errors); if (carrier_changes) - print_uint(PRINT_JSON, "carrier_changes", NULL, + print_u64(PRINT_JSON, "carrier_changes", NULL, rta_getattr_u32(carrier_changes)); } diff --git a/ip/ipmacsec.c b/ip/ipmacsec.c index 38ec7136..4e4e158e 100644 --- a/ip/ipmacsec.c +++ b/ip/ipmacsec.c @@ -640,7 +640,7 @@ static void print_attrs(struct rtattr *attrs[]) } } -static __u64 getattr_uint(struct rtattr *stat) +static __u64 getattr_u64(struct rtattr *stat) { switch (RTA_PAYLOAD(stat)) { case sizeof(__u64): @@ -681,7 +681,7 @@ static void print_fp_stats(const char *prefix, pad = strlen(names[i]) + 1; if (stats[i]) - printf("%*llu", pad, getattr_uint(stats[i])); + printf("%*llu", pad, getattr_u64(stats[i])); else printf("%*c", pad, '-'); } @@ -697,8 +697,8 @@ static void print_json_stats(const char *names[], unsigned int num, if (!names[i] || !stats[i]) continue; - print_uint(PRINT_JSON, names[i], - NULL, getattr_uint(stats[i])); + print_u64(PRINT_JSON, names[i], + NULL, getattr_u64(stats[i])); } } diff --git a/ip/ipmroute.c b/ip/ipmroute.c index 59c5b771..cdb4d898 100644 --- a/ip/ipmroute.c +++ b/ip/ipmroute.c @@ -182,12 +182,12 @@ int print_mroute(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg) struct rta_mfc_stats *mfcs = RTA_DATA(tb[RTA_MFC_STATS]); print_string(PRINT_FP, NULL, "%s", _SL_); - print_uint(PRINT_ANY, "packets", " %"PRIu64" packets,", + print_u64(PRINT_ANY, "packets", " %"PRIu64" packets,", mfcs->mfcs_packets); - print_uint(PRINT_ANY, "bytes", " %"PRIu64" bytes", mfcs->mfcs_bytes); + print_u64(PRINT_ANY, "bytes", " %"PRIu64" bytes", mfcs->mfcs_bytes); if (mfcs->mfcs_wrong_if) - print_uint(PRINT_ANY, "wrong_if", + print_u64(PRINT_ANY, "wrong_if", ", %"PRIu64" arrived on wrong iif.", mfcs->mfcs_wrong_if); } diff --git a/lib/json_print.c b/lib/json_print.c index bda72933..7a1cfa57 100644 --- a/lib/json_print.c +++ b/lib/json_print.c @@ -116,8 +116,10 @@ void close_json_array(enum output_type type, const char *str) } \ } _PRINT_FUNC(int, int); +_PRINT_FUNC(s64, int64_t); _PRINT_FUNC(hu, unsigned short); -_PRINT_FUNC(uint, uint64_t); +_PRINT_FUNC(uint, unsigned int); +_PRINT_FUNC(u64, uint64_t); _PRINT_FUNC(lluint, unsigned long long int); _PRINT_FUNC(float, double); #undef _PRINT_FUNC diff --git a/lib/json_writer.c b/lib/json_writer.c index 0ad04218..dc2fdd49 100644 --- a/lib/json_writer.c +++ b/lib/json_writer.c @@ -220,7 +220,12 @@ void jsonw_hu(json_writer_t *self, unsigned short num) jsonw_printf(self, "%hu", num); } -void jsonw_uint(json_writer_t *self, uint64_t num) +void jsonw_uint(json_writer_t *self, unsigned int num) +{ + jsonw_printf(self, "%u", num); +} + +void jsonw_u64(json_writer_t *self, uint64_t num) { jsonw_printf(self, "%"PRIu64, num); } @@ -235,7 +240,12 @@ void jsonw_lluint(json_writer_t *self, unsigned long long int num) jsonw_printf(self, "%llu", num); } -void jsonw_int(json_writer_t *self, int64_t num) +void jsonw_int(json_writer_t *self, int num) +{ + jsonw_printf(self, "%d", num); +} + +void jsonw_s64(json_writer_t *self, int64_t num) { jsonw_printf(self, "%"PRId64, num); } @@ -268,12 +278,18 @@ void jsonw_float_field_fmt(json_writer_t *self, jsonw_float_fmt(self, fmt, val); } -void jsonw_uint_field(json_writer_t *self, const char *prop, uint64_t num) +void jsonw_uint_field(json_writer_t *self, const char *prop, unsigned int num) { jsonw_name(self, prop); jsonw_uint(self, num); } +void jsonw_u64_field(json_writer_t *self, const char *prop, uint64_t num) +{ + jsonw_name(self, prop); + jsonw_u64(self, num); +} + void jsonw_xint_field(json_writer_t *self, const char *prop, uint64_t num) { jsonw_name(self, prop); @@ -294,12 +310,18 @@ void jsonw_lluint_field(json_writer_t *self, jsonw_lluint(self, num); } -void jsonw_int_field(json_writer_t *self, const char *prop, int64_t num) +void jsonw_int_field(json_writer_t *self, const char *prop, int num) { jsonw_name(self, prop); jsonw_int(self, num); } +void jsonw_s64_field(json_writer_t *self, const char *prop, int64_t num) +{ + jsonw_name(self, prop); + jsonw_s64(self, num); +} + void jsonw_null_field(json_writer_t *self, const char *prop) { jsonw_name(self, prop); -- 2.17.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH iproute2 v2] json_print: Fix hidden 64-bit type promotion 2018-04-25 14:30 ` [PATCH iproute2 v2] json_print: Fix hidden 64-bit " Toke Høiland-Jørgensen @ 2018-04-25 14:55 ` Stephen Hemminger 2018-04-25 14:57 ` Toke Høiland-Jørgensen 2018-04-25 15:28 ` [PATCH iproute2 v3] " Toke Høiland-Jørgensen 1 sibling, 1 reply; 9+ messages in thread From: Stephen Hemminger @ 2018-04-25 14:55 UTC (permalink / raw) To: Toke Høiland-Jørgensen; +Cc: netdev, Kevin Darbyshire-Bryant On Wed, 25 Apr 2018 16:30:22 +0200 Toke Høiland-Jørgensen <toke@toke.dk> wrote: > print_uint() will silently promote its variable type to uint64_t, but there > is nothing that ensures that the format string specifier passed along with > it fits (and the function name suggest to pass "%u"). > > Fix this by changing print_uint() to use a native 'unsigned int' type, and > introduce a separate print_u64() function for printing 64-bit values. All > call sites that were actually printing 64-bit values using print_uint() are > converted to use print_u64() instead. > > Since print_int() was already using native int types, just add a > print_s64() to match, but don't convert any call sites. > > Cc: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> > Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk> Yes, this makes sense. Maybe there should be a print_luint for consistency. Also, I tried (in vain) to make a version that allows GCC to check the format string. But it was a struggle and just gave up. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH iproute2 v2] json_print: Fix hidden 64-bit type promotion 2018-04-25 14:55 ` Stephen Hemminger @ 2018-04-25 14:57 ` Toke Høiland-Jørgensen 2018-04-25 15:12 ` Stephen Hemminger 0 siblings, 1 reply; 9+ messages in thread From: Toke Høiland-Jørgensen @ 2018-04-25 14:57 UTC (permalink / raw) To: Stephen Hemminger; +Cc: netdev, Kevin Darbyshire-Bryant Stephen Hemminger <stephen@networkplumber.org> writes: > On Wed, 25 Apr 2018 16:30:22 +0200 > Toke Høiland-Jørgensen <toke@toke.dk> wrote: > >> print_uint() will silently promote its variable type to uint64_t, but there >> is nothing that ensures that the format string specifier passed along with >> it fits (and the function name suggest to pass "%u"). >> >> Fix this by changing print_uint() to use a native 'unsigned int' type, and >> introduce a separate print_u64() function for printing 64-bit values. All >> call sites that were actually printing 64-bit values using print_uint() are >> converted to use print_u64() instead. >> >> Since print_int() was already using native int types, just add a >> print_s64() to match, but don't convert any call sites. >> >> Cc: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> >> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk> > > Yes, this makes sense. Maybe there should be a print_luint for > consistency. I just realised I missed a few call sites, so I'll resend. Should I call the new function print_luint() instead of print_u64()? > Also, I tried (in vain) to make a version that allows GCC to check the > format string. But it was a struggle and just gave up. Yeah, no idea how to do this either... -Toke ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH iproute2 v2] json_print: Fix hidden 64-bit type promotion 2018-04-25 14:57 ` Toke Høiland-Jørgensen @ 2018-04-25 15:12 ` Stephen Hemminger 0 siblings, 0 replies; 9+ messages in thread From: Stephen Hemminger @ 2018-04-25 15:12 UTC (permalink / raw) To: Toke Høiland-Jørgensen; +Cc: netdev, Kevin Darbyshire-Bryant On Wed, 25 Apr 2018 16:57:52 +0200 Toke Høiland-Jørgensen <toke@toke.dk> wrote: > Stephen Hemminger <stephen@networkplumber.org> writes: > > > On Wed, 25 Apr 2018 16:30:22 +0200 > > Toke Høiland-Jørgensen <toke@toke.dk> wrote: > > > >> print_uint() will silently promote its variable type to uint64_t, but there > >> is nothing that ensures that the format string specifier passed along with > >> it fits (and the function name suggest to pass "%u"). > >> > >> Fix this by changing print_uint() to use a native 'unsigned int' type, and > >> introduce a separate print_u64() function for printing 64-bit values. All > >> call sites that were actually printing 64-bit values using print_uint() are > >> converted to use print_u64() instead. > >> > >> Since print_int() was already using native int types, just add a > >> print_s64() to match, but don't convert any call sites. > >> > >> Cc: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> > >> Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk> > > > > Yes, this makes sense. Maybe there should be a print_luint for > > consistency. > > I just realised I missed a few call sites, so I'll resend. Should I > call the new function print_luint() instead of print_u64()? Ideally, there would be both functions, and use based on what is being printed. > > Also, I tried (in vain) to make a version that allows GCC to check the > > format string. But it was a struggle and just gave up. > > Yeah, no idea how to do this either... Maybe some magic smatch or multi-line regex it would be possible to find all instances of print_uint, then look at format string of each and see if there is a single %u. Some added complexity since some places only print json and don't care and pass NULL for format. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH iproute2 v3] json_print: Fix hidden 64-bit type promotion 2018-04-25 14:30 ` [PATCH iproute2 v2] json_print: Fix hidden 64-bit " Toke Høiland-Jørgensen 2018-04-25 14:55 ` Stephen Hemminger @ 2018-04-25 15:28 ` Toke Høiland-Jørgensen 2018-04-25 18:11 ` Stephen Hemminger 1 sibling, 1 reply; 9+ messages in thread From: Toke Høiland-Jørgensen @ 2018-04-25 15:28 UTC (permalink / raw) To: netdev; +Cc: Toke Høiland-Jørgensen print_uint() will silently promote its variable type to uint64_t, but there is nothing that ensures that the format string specifier passed along with it fits (and the function name suggest to pass "%u"). Fix this by changing print_uint() to use a native 'unsigned int' type, and introduce a separate print_u64() function for printing 64-bit values. All call sites that were actually printing 64-bit values using print_uint() are converted to use print_u64() instead. Since print_int() was already using native int types, just add a print_s64() to match, but don't convert any call sites. For symmetry, also add a print_luint() method (with no users). Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk> --- v3: - Fix a few more call sites. - Also add a print_luint() function for symmetry with the existing ones. v2 (v1 was submitted by Kevin Darbyshire-Bryant): - Add print_u64() and convert call sites that were actually passing 64-bit values to print_uint(). include/json_print.h | 5 +++- include/json_writer.h | 15 ++++++++--- ip/ipaddress.c | 62 +++++++++++++++++++++---------------------- ip/ipmacsec.c | 8 +++--- ip/ipmroute.c | 6 ++--- ip/ipntable.c | 38 +++++++++++++------------- ip/iproute_lwtunnel.c | 6 ++--- ip/iptuntap.c | 4 +-- ip/tcp_metrics.c | 6 ++--- lib/json_print.c | 5 +++- lib/json_writer.c | 43 +++++++++++++++++++++++++++--- 11 files changed, 123 insertions(+), 75 deletions(-) diff --git a/include/json_print.h b/include/json_print.h index 2ca7830a..218fedc5 100644 --- a/include/json_print.h +++ b/include/json_print.h @@ -56,13 +56,16 @@ void close_json_array(enum output_type type, const char *delim); print_color_##type_name(t, COLOR_NONE, key, fmt, value); \ } _PRINT_FUNC(int, int); +_PRINT_FUNC(s64, int64_t); _PRINT_FUNC(bool, bool); _PRINT_FUNC(null, const char*); _PRINT_FUNC(string, const char*); -_PRINT_FUNC(uint, uint64_t); +_PRINT_FUNC(uint, unsigned int); +_PRINT_FUNC(u64, uint64_t); _PRINT_FUNC(hu, unsigned short); _PRINT_FUNC(hex, unsigned int); _PRINT_FUNC(0xhex, unsigned int); +_PRINT_FUNC(luint, unsigned long int); _PRINT_FUNC(lluint, unsigned long long int); _PRINT_FUNC(float, double); #undef _PRINT_FUNC diff --git a/include/json_writer.h b/include/json_writer.h index 4b4dec28..9ab88e1d 100644 --- a/include/json_writer.h +++ b/include/json_writer.h @@ -34,22 +34,29 @@ void jsonw_string(json_writer_t *self, const char *value); void jsonw_bool(json_writer_t *self, bool value); void jsonw_float(json_writer_t *self, double number); void jsonw_float_fmt(json_writer_t *self, const char *fmt, double num); -void jsonw_uint(json_writer_t *self, uint64_t number); +void jsonw_uint(json_writer_t *self, unsigned int number); +void jsonw_u64(json_writer_t *self, uint64_t number); void jsonw_xint(json_writer_t *self, uint64_t number); void jsonw_hu(json_writer_t *self, unsigned short number); -void jsonw_int(json_writer_t *self, int64_t number); +void jsonw_int(json_writer_t *self, int number); +void jsonw_s64(json_writer_t *self, int64_t number); void jsonw_null(json_writer_t *self); +void jsonw_luint(json_writer_t *self, unsigned long int num); void jsonw_lluint(json_writer_t *self, unsigned long long int num); /* Useful Combinations of name and value */ void jsonw_string_field(json_writer_t *self, const char *prop, const char *val); void jsonw_bool_field(json_writer_t *self, const char *prop, bool value); void jsonw_float_field(json_writer_t *self, const char *prop, double num); -void jsonw_uint_field(json_writer_t *self, const char *prop, uint64_t num); +void jsonw_uint_field(json_writer_t *self, const char *prop, unsigned int num); +void jsonw_u64_field(json_writer_t *self, const char *prop, uint64_t num); void jsonw_xint_field(json_writer_t *self, const char *prop, uint64_t num); void jsonw_hu_field(json_writer_t *self, const char *prop, unsigned short num); -void jsonw_int_field(json_writer_t *self, const char *prop, int64_t num); +void jsonw_int_field(json_writer_t *self, const char *prop, int num); +void jsonw_s64_field(json_writer_t *self, const char *prop, int64_t num); void jsonw_null_field(json_writer_t *self, const char *prop); +void jsonw_luint_field(json_writer_t *self, const char *prop, + unsigned long int num); void jsonw_lluint_field(json_writer_t *self, const char *prop, unsigned long long int num); void jsonw_float_field_fmt(json_writer_t *self, const char *prop, diff --git a/ip/ipaddress.c b/ip/ipaddress.c index aecc9a1d..75539e05 100644 --- a/ip/ipaddress.c +++ b/ip/ipaddress.c @@ -554,21 +554,21 @@ static void print_vf_stats64(FILE *fp, struct rtattr *vfstats) /* RX stats */ open_json_object("rx"); - print_uint(PRINT_JSON, "bytes", NULL, + print_u64(PRINT_JSON, "bytes", NULL, rta_getattr_u64(vf[IFLA_VF_STATS_RX_BYTES])); - print_uint(PRINT_JSON, "packets", NULL, + print_u64(PRINT_JSON, "packets", NULL, rta_getattr_u64(vf[IFLA_VF_STATS_RX_PACKETS])); - print_uint(PRINT_JSON, "multicast", NULL, + print_u64(PRINT_JSON, "multicast", NULL, rta_getattr_u64(vf[IFLA_VF_STATS_MULTICAST])); - print_uint(PRINT_JSON, "broadcast", NULL, + print_u64(PRINT_JSON, "broadcast", NULL, rta_getattr_u64(vf[IFLA_VF_STATS_BROADCAST])); close_json_object(); /* TX stats */ open_json_object("tx"); - print_uint(PRINT_JSON, "tx_bytes", NULL, + print_u64(PRINT_JSON, "tx_bytes", NULL, rta_getattr_u64(vf[IFLA_VF_STATS_TX_BYTES])); - print_uint(PRINT_JSON, "tx_packets", NULL, + print_u64(PRINT_JSON, "tx_packets", NULL, rta_getattr_u64(vf[IFLA_VF_STATS_TX_PACKETS])); close_json_object(); close_json_object(); @@ -608,69 +608,69 @@ static void __print_link_stats(FILE *fp, struct rtattr *tb[]) /* RX stats */ open_json_object("rx"); - print_uint(PRINT_JSON, "bytes", NULL, s->rx_bytes); - print_uint(PRINT_JSON, "packets", NULL, s->rx_packets); - print_uint(PRINT_JSON, "errors", NULL, s->rx_errors); - print_uint(PRINT_JSON, "dropped", NULL, s->rx_dropped); - print_uint(PRINT_JSON, "over_errors", NULL, s->rx_over_errors); - print_uint(PRINT_JSON, "multicast", NULL, s->multicast); + print_u64(PRINT_JSON, "bytes", NULL, s->rx_bytes); + print_u64(PRINT_JSON, "packets", NULL, s->rx_packets); + print_u64(PRINT_JSON, "errors", NULL, s->rx_errors); + print_u64(PRINT_JSON, "dropped", NULL, s->rx_dropped); + print_u64(PRINT_JSON, "over_errors", NULL, s->rx_over_errors); + print_u64(PRINT_JSON, "multicast", NULL, s->multicast); if (s->rx_compressed) - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "compressed", NULL, s->rx_compressed); /* RX error stats */ if (show_stats > 1) { - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "length_errors", NULL, s->rx_length_errors); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "crc_errors", NULL, s->rx_crc_errors); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "frame_errors", NULL, s->rx_frame_errors); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "fifo_errors", NULL, s->rx_fifo_errors); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "missed_errors", NULL, s->rx_missed_errors); if (s->rx_nohandler) - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "nohandler", NULL, s->rx_nohandler); } close_json_object(); /* TX stats */ open_json_object("tx"); - print_uint(PRINT_JSON, "bytes", NULL, s->tx_bytes); - print_uint(PRINT_JSON, "packets", NULL, s->tx_packets); - print_uint(PRINT_JSON, "errors", NULL, s->tx_errors); - print_uint(PRINT_JSON, "dropped", NULL, s->tx_dropped); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "bytes", NULL, s->tx_bytes); + print_u64(PRINT_JSON, "packets", NULL, s->tx_packets); + print_u64(PRINT_JSON, "errors", NULL, s->tx_errors); + print_u64(PRINT_JSON, "dropped", NULL, s->tx_dropped); + print_u64(PRINT_JSON, "carrier_errors", NULL, s->tx_carrier_errors); - print_uint(PRINT_JSON, "collisions", NULL, s->collisions); + print_u64(PRINT_JSON, "collisions", NULL, s->collisions); if (s->tx_compressed) - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "compressed", NULL, s->tx_compressed); /* TX error stats */ if (show_stats > 1) { - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "aborted_errors", NULL, s->tx_aborted_errors); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "fifo_errors", NULL, s->tx_fifo_errors); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "window_errors", NULL, s->tx_window_errors); - print_uint(PRINT_JSON, + print_u64(PRINT_JSON, "heartbeat_errors", NULL, s->tx_heartbeat_errors); if (carrier_changes) - print_uint(PRINT_JSON, "carrier_changes", NULL, + print_u64(PRINT_JSON, "carrier_changes", NULL, rta_getattr_u32(carrier_changes)); } diff --git a/ip/ipmacsec.c b/ip/ipmacsec.c index 38ec7136..4e4e158e 100644 --- a/ip/ipmacsec.c +++ b/ip/ipmacsec.c @@ -640,7 +640,7 @@ static void print_attrs(struct rtattr *attrs[]) } } -static __u64 getattr_uint(struct rtattr *stat) +static __u64 getattr_u64(struct rtattr *stat) { switch (RTA_PAYLOAD(stat)) { case sizeof(__u64): @@ -681,7 +681,7 @@ static void print_fp_stats(const char *prefix, pad = strlen(names[i]) + 1; if (stats[i]) - printf("%*llu", pad, getattr_uint(stats[i])); + printf("%*llu", pad, getattr_u64(stats[i])); else printf("%*c", pad, '-'); } @@ -697,8 +697,8 @@ static void print_json_stats(const char *names[], unsigned int num, if (!names[i] || !stats[i]) continue; - print_uint(PRINT_JSON, names[i], - NULL, getattr_uint(stats[i])); + print_u64(PRINT_JSON, names[i], + NULL, getattr_u64(stats[i])); } } diff --git a/ip/ipmroute.c b/ip/ipmroute.c index 59c5b771..cdb4d898 100644 --- a/ip/ipmroute.c +++ b/ip/ipmroute.c @@ -182,12 +182,12 @@ int print_mroute(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg) struct rta_mfc_stats *mfcs = RTA_DATA(tb[RTA_MFC_STATS]); print_string(PRINT_FP, NULL, "%s", _SL_); - print_uint(PRINT_ANY, "packets", " %"PRIu64" packets,", + print_u64(PRINT_ANY, "packets", " %"PRIu64" packets,", mfcs->mfcs_packets); - print_uint(PRINT_ANY, "bytes", " %"PRIu64" bytes", mfcs->mfcs_bytes); + print_u64(PRINT_ANY, "bytes", " %"PRIu64" bytes", mfcs->mfcs_bytes); if (mfcs->mfcs_wrong_if) - print_uint(PRINT_ANY, "wrong_if", + print_u64(PRINT_ANY, "wrong_if", ", %"PRIu64" arrived on wrong iif.", mfcs->mfcs_wrong_if); } diff --git a/ip/ipntable.c b/ip/ipntable.c index 82f40f87..4fae181d 100644 --- a/ip/ipntable.c +++ b/ip/ipntable.c @@ -392,7 +392,7 @@ static void print_ndtparams(struct rtattr *tpb[]) if (tpb[NDTPA_REACHABLE_TIME]) { __u64 reachable = rta_getattr_u64(tpb[NDTPA_REACHABLE_TIME]); - print_uint(PRINT_ANY, "reachable", + print_u64(PRINT_ANY, "reachable", "reachable %llu ", reachable); } @@ -400,14 +400,14 @@ static void print_ndtparams(struct rtattr *tpb[]) __u64 breachable = rta_getattr_u64(tpb[NDTPA_BASE_REACHABLE_TIME]); - print_uint(PRINT_ANY, "base_reachable", + print_u64(PRINT_ANY, "base_reachable", "base_reachable %llu ", breachable); } if (tpb[NDTPA_RETRANS_TIME]) { __u64 retrans = rta_getattr_u64(tpb[NDTPA_RETRANS_TIME]); - print_uint(PRINT_ANY, "retrans", "retrans %llu ", retrans); + print_u64(PRINT_ANY, "retrans", "retrans %llu ", retrans); } print_string(PRINT_FP, NULL, "%s ", _SL_); @@ -415,14 +415,14 @@ static void print_ndtparams(struct rtattr *tpb[]) if (tpb[NDTPA_GC_STALETIME]) { __u64 gc_stale = rta_getattr_u64(tpb[NDTPA_GC_STALETIME]); - print_uint(PRINT_ANY, "gc_stale", "gc_stale %llu ", gc_stale); + print_u64(PRINT_ANY, "gc_stale", "gc_stale %llu ", gc_stale); } if (tpb[NDTPA_DELAY_PROBE_TIME]) { __u64 delay_probe = rta_getattr_u64(tpb[NDTPA_DELAY_PROBE_TIME]); - print_uint(PRINT_ANY, "delay_probe", + print_u64(PRINT_ANY, "delay_probe", "delay_probe %llu ", delay_probe); } @@ -459,14 +459,14 @@ static void print_ndtparams(struct rtattr *tpb[]) if (tpb[NDTPA_ANYCAST_DELAY]) { __u64 anycast_delay = rta_getattr_u64(tpb[NDTPA_ANYCAST_DELAY]); - print_uint(PRINT_ANY, "anycast_delay", + print_u64(PRINT_ANY, "anycast_delay", "anycast_delay %llu ", anycast_delay); } if (tpb[NDTPA_PROXY_DELAY]) { __u64 proxy_delay = rta_getattr_u64(tpb[NDTPA_PROXY_DELAY]); - print_uint(PRINT_ANY, "proxy_delay", + print_u64(PRINT_ANY, "proxy_delay", "proxy_delay %llu ", proxy_delay); } @@ -479,7 +479,7 @@ static void print_ndtparams(struct rtattr *tpb[]) if (tpb[NDTPA_LOCKTIME]) { __u64 locktime = rta_getattr_u64(tpb[NDTPA_LOCKTIME]); - print_uint(PRINT_ANY, "locktime", "locktime %llu ", locktime); + print_u64(PRINT_ANY, "locktime", "locktime %llu ", locktime); } print_string(PRINT_FP, NULL, "%s", _SL_); @@ -490,31 +490,31 @@ static void print_ndtstats(const struct ndt_stats *ndts) print_string(PRINT_FP, NULL, " stats ", NULL); - print_uint(PRINT_ANY, "allocs", "allocs %llu ", ndts->ndts_allocs); - print_uint(PRINT_ANY, "destroys", "destroys %llu ", + print_u64(PRINT_ANY, "allocs", "allocs %llu ", ndts->ndts_allocs); + print_u64(PRINT_ANY, "destroys", "destroys %llu ", ndts->ndts_destroys); - print_uint(PRINT_ANY, "hash_grows", "hash_grows %llu ", + print_u64(PRINT_ANY, "hash_grows", "hash_grows %llu ", ndts->ndts_hash_grows); print_string(PRINT_FP, NULL, "%s ", _SL_); - print_uint(PRINT_ANY, "res_failed", "res_failed %llu ", + print_u64(PRINT_ANY, "res_failed", "res_failed %llu ", ndts->ndts_res_failed); - print_uint(PRINT_ANY, "lookups", "lookups %llu ", ndts->ndts_lookups); - print_uint(PRINT_ANY, "hits", "hits %llu ", ndts->ndts_hits); + print_u64(PRINT_ANY, "lookups", "lookups %llu ", ndts->ndts_lookups); + print_u64(PRINT_ANY, "hits", "hits %llu ", ndts->ndts_hits); print_string(PRINT_FP, NULL, "%s ", _SL_); - print_uint(PRINT_ANY, "rcv_probes_mcast", "rcv_probes_mcast %llu ", + print_u64(PRINT_ANY, "rcv_probes_mcast", "rcv_probes_mcast %llu ", ndts->ndts_rcv_probes_mcast); - print_uint(PRINT_ANY, "rcv_probes_ucast", "rcv_probes_ucast %llu ", + print_u64(PRINT_ANY, "rcv_probes_ucast", "rcv_probes_ucast %llu ", ndts->ndts_rcv_probes_ucast); print_string(PRINT_FP, NULL, "%s ", _SL_); - print_uint(PRINT_ANY, "periodic_gc_runs", "periodic_gc_runs %llu ", + print_u64(PRINT_ANY, "periodic_gc_runs", "periodic_gc_runs %llu ", ndts->ndts_periodic_gc_runs); - print_uint(PRINT_ANY, "forced_gc_runs", "forced_gc_runs %llu ", + print_u64(PRINT_ANY, "forced_gc_runs", "forced_gc_runs %llu ", ndts->ndts_forced_gc_runs); print_string(PRINT_FP, NULL, "%s", _SL_); @@ -607,7 +607,7 @@ static int print_ntable(const struct sockaddr_nl *who, if (tb[NDTA_GC_INTERVAL]) { __u64 gc_int = rta_getattr_u64(tb[NDTA_GC_INTERVAL]); - print_uint(PRINT_ANY, "gc_interval", "gc_int %llu ", gc_int); + print_u64(PRINT_ANY, "gc_interval", "gc_int %llu ", gc_int); } if (ret) diff --git a/ip/iproute_lwtunnel.c b/ip/iproute_lwtunnel.c index cde9b3d2..46a212c8 100644 --- a/ip/iproute_lwtunnel.c +++ b/ip/iproute_lwtunnel.c @@ -273,7 +273,7 @@ static void print_encap_ip(FILE *fp, struct rtattr *encap) parse_rtattr_nested(tb, LWTUNNEL_IP_MAX, encap); if (tb[LWTUNNEL_IP_ID]) - print_uint(PRINT_ANY, "id", "id %llu ", + print_u64(PRINT_ANY, "id", "id %llu ", ntohll(rta_getattr_u64(tb[LWTUNNEL_IP_ID]))); if (tb[LWTUNNEL_IP_SRC]) @@ -333,7 +333,7 @@ static void print_encap_ip6(FILE *fp, struct rtattr *encap) parse_rtattr_nested(tb, LWTUNNEL_IP6_MAX, encap); if (tb[LWTUNNEL_IP6_ID]) - print_uint(PRINT_ANY, "id", "id %llu ", + print_u64(PRINT_ANY, "id", "id %llu ", ntohll(rta_getattr_u64(tb[LWTUNNEL_IP6_ID]))); if (tb[LWTUNNEL_IP6_SRC]) @@ -347,7 +347,7 @@ static void print_encap_ip6(FILE *fp, struct rtattr *encap) rt_addr_n2a_rta(AF_INET6, tb[LWTUNNEL_IP6_DST])); if (tb[LWTUNNEL_IP6_HOPLIMIT]) - print_uint(PRINT_ANY, "hoplimit", + print_u64(PRINT_ANY, "hoplimit", "hoplimit %u ", rta_getattr_u8(tb[LWTUNNEL_IP6_HOPLIMIT])); diff --git a/ip/iptuntap.c b/ip/iptuntap.c index 6c5a7259..58996e6c 100644 --- a/ip/iptuntap.c +++ b/ip/iptuntap.c @@ -440,10 +440,10 @@ static int print_tuntap(const struct sockaddr_nl *who, "ifname", "%s:", name); print_flags(flags); if (owner != -1) - print_uint(PRINT_ANY, "user", + print_u64(PRINT_ANY, "user", " user %ld", owner); if (group != -1) - print_uint(PRINT_ANY, "group", + print_u64(PRINT_ANY, "group", " group %ld", group); if (show_details) { diff --git a/ip/tcp_metrics.c b/ip/tcp_metrics.c index 72dc980c..ad3d6f36 100644 --- a/ip/tcp_metrics.c +++ b/ip/tcp_metrics.c @@ -139,19 +139,19 @@ static void print_tcp_metrics(struct rtattr *a) print_uint(PRINT_JSON, name, NULL, val); print_string(PRINT_FP, NULL, " %s ", name); - print_uint(PRINT_FP, NULL, "%lu", val); + print_uint(PRINT_FP, NULL, "%u", val); } if (rtt) { print_float(PRINT_JSON, "rtt", NULL, (double)rtt / usec_per_sec); - print_uint(PRINT_FP, NULL, + print_u64(PRINT_FP, NULL, " rtt %luus", rtt); } if (rttvar) { print_float(PRINT_JSON, "rttvar", NULL, (double) rttvar / usec_per_sec); - print_uint(PRINT_FP, NULL, + print_u64(PRINT_FP, NULL, " rttvar %luus", rttvar); } } diff --git a/lib/json_print.c b/lib/json_print.c index bda72933..5dc41bfa 100644 --- a/lib/json_print.c +++ b/lib/json_print.c @@ -116,8 +116,11 @@ void close_json_array(enum output_type type, const char *str) } \ } _PRINT_FUNC(int, int); +_PRINT_FUNC(s64, int64_t); _PRINT_FUNC(hu, unsigned short); -_PRINT_FUNC(uint, uint64_t); +_PRINT_FUNC(uint, unsigned int); +_PRINT_FUNC(u64, uint64_t); +_PRINT_FUNC(luint, unsigned long int); _PRINT_FUNC(lluint, unsigned long long int); _PRINT_FUNC(float, double); #undef _PRINT_FUNC diff --git a/lib/json_writer.c b/lib/json_writer.c index 0ad04218..aa9ce1c6 100644 --- a/lib/json_writer.c +++ b/lib/json_writer.c @@ -220,7 +220,12 @@ void jsonw_hu(json_writer_t *self, unsigned short num) jsonw_printf(self, "%hu", num); } -void jsonw_uint(json_writer_t *self, uint64_t num) +void jsonw_uint(json_writer_t *self, unsigned int num) +{ + jsonw_printf(self, "%u", num); +} + +void jsonw_u64(json_writer_t *self, uint64_t num) { jsonw_printf(self, "%"PRIu64, num); } @@ -230,12 +235,22 @@ void jsonw_xint(json_writer_t *self, uint64_t num) jsonw_printf(self, "%"PRIx64, num); } +void jsonw_luint(json_writer_t *self, unsigned long int num) +{ + jsonw_printf(self, "%lu", num); +} + void jsonw_lluint(json_writer_t *self, unsigned long long int num) { jsonw_printf(self, "%llu", num); } -void jsonw_int(json_writer_t *self, int64_t num) +void jsonw_int(json_writer_t *self, int num) +{ + jsonw_printf(self, "%d", num); +} + +void jsonw_s64(json_writer_t *self, int64_t num) { jsonw_printf(self, "%"PRId64, num); } @@ -268,12 +283,18 @@ void jsonw_float_field_fmt(json_writer_t *self, jsonw_float_fmt(self, fmt, val); } -void jsonw_uint_field(json_writer_t *self, const char *prop, uint64_t num) +void jsonw_uint_field(json_writer_t *self, const char *prop, unsigned int num) { jsonw_name(self, prop); jsonw_uint(self, num); } +void jsonw_u64_field(json_writer_t *self, const char *prop, uint64_t num) +{ + jsonw_name(self, prop); + jsonw_u64(self, num); +} + void jsonw_xint_field(json_writer_t *self, const char *prop, uint64_t num) { jsonw_name(self, prop); @@ -286,6 +307,14 @@ void jsonw_hu_field(json_writer_t *self, const char *prop, unsigned short num) jsonw_hu(self, num); } +void jsonw_luint_field(json_writer_t *self, + const char *prop, + unsigned long int num) +{ + jsonw_name(self, prop); + jsonw_luint(self, num); +} + void jsonw_lluint_field(json_writer_t *self, const char *prop, unsigned long long int num) @@ -294,12 +323,18 @@ void jsonw_lluint_field(json_writer_t *self, jsonw_lluint(self, num); } -void jsonw_int_field(json_writer_t *self, const char *prop, int64_t num) +void jsonw_int_field(json_writer_t *self, const char *prop, int num) { jsonw_name(self, prop); jsonw_int(self, num); } +void jsonw_s64_field(json_writer_t *self, const char *prop, int64_t num) +{ + jsonw_name(self, prop); + jsonw_s64(self, num); +} + void jsonw_null_field(json_writer_t *self, const char *prop) { jsonw_name(self, prop); -- 2.17.0 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH iproute2 v3] json_print: Fix hidden 64-bit type promotion 2018-04-25 15:28 ` [PATCH iproute2 v3] " Toke Høiland-Jørgensen @ 2018-04-25 18:11 ` Stephen Hemminger 0 siblings, 0 replies; 9+ messages in thread From: Stephen Hemminger @ 2018-04-25 18:11 UTC (permalink / raw) To: Toke Høiland-Jørgensen; +Cc: netdev On Wed, 25 Apr 2018 17:28:57 +0200 Toke Høiland-Jørgensen <toke@toke.dk> wrote: > print_uint() will silently promote its variable type to uint64_t, but there > is nothing that ensures that the format string specifier passed along with > it fits (and the function name suggest to pass "%u"). > > Fix this by changing print_uint() to use a native 'unsigned int' type, and > introduce a separate print_u64() function for printing 64-bit values. All > call sites that were actually printing 64-bit values using print_uint() are > converted to use print_u64() instead. > > Since print_int() was already using native int types, just add a > print_s64() to match, but don't convert any call sites. For symmetry, > also add a print_luint() method (with no users). > > Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk> Looks good, applied. This also fixes glitches in mroute and neigh output. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2018-04-25 18:11 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-03-29 19:22 [PATCH iproute2] json_print: fix print_uint hidden type promotion Kevin Darbyshire-Bryant 2018-03-29 21:02 ` Stephen Hemminger 2018-03-29 21:29 ` Kevin Darbyshire-Bryant 2018-04-25 14:30 ` [PATCH iproute2 v2] json_print: Fix hidden 64-bit " Toke Høiland-Jørgensen 2018-04-25 14:55 ` Stephen Hemminger 2018-04-25 14:57 ` Toke Høiland-Jørgensen 2018-04-25 15:12 ` Stephen Hemminger 2018-04-25 15:28 ` [PATCH iproute2 v3] " Toke Høiland-Jørgensen 2018-04-25 18:11 ` Stephen Hemminger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).