[PATCH] json_print: fix print_uint with helper type extensions

[PATCH] json_print: fix print_uint with helper type extensions

[Kevin Darbyshire-Bryant]

Introduce print helper functions for int, uint, explicit int32, uint32,
int64 & uint64.

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 and hence produced wildly incorrect values in tc qdisc
output.

print_u/int now stick with native int size.  print_u/int32 & print
u/int64 functions offer explicit integer sizes.

To portably use these formats you should use the relevant PRIdN or PRIuN
formats as defined in inttypes.h

e.g.

print_uint64(PRINT_ANY, "refcnt", "refcnt %" PRIu64 " ", t->tcm_info)

Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
---
 include/json_print.h | 6 +++++-
 lib/json_print.c     | 6 +++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/include/json_print.h b/include/json_print.h
index 2ca7830a..fb62b142 100644
--- a/include/json_print.h
+++ b/include/json_print.h
@@ -56,10 +56,14 @@ 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(int32, int32_t);
+_PRINT_FUNC(uint32, uint32_t);
+_PRINT_FUNC(int64, int64_t);
+_PRINT_FUNC(uint64, 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..12ee26df 100644
--- a/lib/json_print.c
+++ b/lib/json_print.c
@@ -117,8 +117,12 @@ 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(int32, int32_t);
+_PRINT_FUNC(uint32, uint32_t);
+_PRINT_FUNC(int64, int64_t);
+_PRINT_FUNC(uint64, uint64_t);
 _PRINT_FUNC(lluint, unsigned long long int);
 _PRINT_FUNC(float, double);
 #undef _PRINT_FUNC
-- 
2.14.3 (Apple Git-98)

Re: [PATCH] json_print: fix print_uint with helper type extensions

[Stephen Hemminger]

On Thu, 15 Mar 2018 20:07:20 +0000
Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> wrote:

> Introduce print helper functions for int, uint, explicit int32, uint32,
> int64 & uint64.
> 
> 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 and hence produced wildly incorrect values in tc qdisc
> output.
> 
> print_u/int now stick with native int size.  print_u/int32 & print
> u/int64 functions offer explicit integer sizes.
> 
> To portably use these formats you should use the relevant PRIdN or PRIuN
> formats as defined in inttypes.h
> 
> e.g.
> 
> print_uint64(PRINT_ANY, "refcnt", "refcnt %" PRIu64 " ", t->tcm_info)
> 
> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>

I am fine with this. But since there is no code using it yet, it should
go net-next branch.

Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [PATCH] json_print: fix print_uint with helper type extensions

[Kevin Darbyshire-Bryant]

[-- Attachment #1: Type: text/plain, Size: 853 bytes --]



> On 16 Mar 2018, at 20:34, Stephen Hemminger <stephen@networkplumber.org> wrote:
>> 
>> print_uint64(PRINT_ANY, "refcnt", "refcnt %" PRIu64 " ", t->tcm_info)
>> 
>> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
> 
> I am fine with this. But since there is no code using it yet, it should
> go net-next branch.
> 
> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>

Existing code is tripping up over the hidden uint - > uint64_t promotion in print_uint in iproute2 v4.15, that’s how I fell over the issue.  Should I split the patch?  One fixing the uint->uint64_t and the other offering the explicit type length options.

Obviously I now realise that the email header should have iproute2 in it.  Learning, slowly :-)

Cheers,

Kevin D-B

012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A


[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH] json_print: fix print_uint with helper type extensions

[David Ahern]

On 3/18/18 2:40 AM, Kevin Darbyshire-Bryant wrote:
> 
> 
>> On 16 Mar 2018, at 20:34, Stephen Hemminger <stephen@networkplumber.org> wrote:
>>>
>>> print_uint64(PRINT_ANY, "refcnt", "refcnt %" PRIu64 " ", t->tcm_info)
>>>
>>> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
>>
>> I am fine with this. But since there is no code using it yet, it should
>> go net-next branch.
>>
>> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> Existing code is tripping up over the hidden uint - > uint64_t promotion in print_uint in iproute2 v4.15, that’s how I fell over the issue.  Should I split the patch?  One fixing the uint->uint64_t and the other offering the explicit type length options.
> 
> Obviously I now realise that the email header should have iproute2 in it.  Learning, slowly :-)
> 

Kevin: I guess you need to split the patch. Extract the bug fix piece
and send for iproute2; enhancements go to iproute2-next.

Re: [PATCH] json_print: fix print_uint with helper type extensions

[Kevin Darbyshire-Bryant]

[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]



> On 26 Mar 2018, at 16:03, David Ahern <dsahern@gmail.com> wrote:
> 
> On 3/18/18 2:40 AM, Kevin Darbyshire-Bryant wrote:
>> 
>> 
>>> On 16 Mar 2018, at 20:34, Stephen Hemminger <stephen@networkplumber.org> wrote:
>>>> 
>>>> print_uint64(PRINT_ANY, "refcnt", "refcnt %" PRIu64 " ", t->tcm_info)
>>>> 
>>>> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
>>> 
>>> I am fine with this. But since there is no code using it yet, it should
>>> go net-next branch.
>>> 
>>> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>
>> 
>> Existing code is tripping up over the hidden uint - > uint64_t promotion in print_uint in iproute2 v4.15, that’s how I fell over the issue.  Should I split the patch?  One fixing the uint->uint64_t and the other offering the explicit type length options.
>> 
>> Obviously I now realise that the email header should have iproute2 in it.  Learning, slowly :-)
>> 
> 
> Kevin: I guess you need to split the patch. Extract the bug fix piece
> and send for iproute2; enhancements go to iproute2-next.

Done that - hopefully done it right or at least better.


012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A


[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 833 bytes --]