netdev - Re: [PATCH iproute2-next] json_print: fix print

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f4c29679-28be-ab89-b1f1-0460e1651d68@gmail.com>
Date:   Fri, 30 Mar 2018 08:57:10 -0600
From:   David Ahern <dsahern@...il.com>
To:     Stephen Hemminger <stephen@...workplumber.org>,
        Kevin Darbyshire-Bryant <ldir@...byshire-bryant.me.uk>
Cc:     netdev@...r.kernel.org
Subject: Re: [PATCH iproute2-next] json_print: fix print_uint with helper type
 extensions

On 3/29/18 3:03 PM, Stephen Hemminger wrote:
> On Thu, 29 Mar 2018 20:32:10 +0100
> Kevin Darbyshire-Bryant <ldir@...byshire-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.
>>
>> 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@...byshire-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 bda72933..1194a6ec 100644
>> --- a/lib/json_print.c
>> +++ b/lib/json_print.c
>> @@ -116,8 +116,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
> 
> You sent patches to both trees. That is not the correct protocol.
> Choose one, get it reviewed.  iproute2-next will get merged from master (in fact
> dave should be doing it regularly).
> 

My comment was to send separate patches - bug fix only for iproute2
master and then a separate one for enhancements going to -next. If the
enhancements overlap the bug fix then it needs to wait for the merge.
This is really no different than what is often needed for net and net-next.