> On 29 Mar 2018, at 22:02, Stephen Hemminger <step...@networkplumber.org> > wrote: > > On Thu, 29 Mar 2018 20:22:20 +0100 > Kevin Darbyshire-Bryant <l...@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 <l...@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 :-)
