Re: [RFC PATCH 1/1] json-writer: incorrect format specifier

From: Ramsay Jones <ramsay@ramsayjones.plus.com>
Date: Mar 27, 2018, 03:26 UTC
Message-ID: <9512a1eb-2b9d-fda5-f2a5-c9f59dff972c@ramsayjones.plus.com>
In-Reply-To: <xmqqr2o6dayt.fsf@gitster-ct.c.googlers.com>

On 26/03/18 18:04, Junio C Hamano wrote:

Show 24 quoted lines

> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
> 
>>>> @@ -120,7 +120,7 @@ void jw_object_uint64(struct json_writer *jw, const char *key, uint64_t value)
>>>>  	maybe_add_comma(jw);
>>>>  
>>>>  	append_quoted_string(&jw->json, key);
>>>> -	strbuf_addf(&jw->json, ":%"PRIuMAX, value);
>>>> +	strbuf_addf(&jw->json, ":%"PRIu64, value);
>>>
>>> In this code-base, that would normally be written as:
>>>
>>> 	strbuf_addf(&jw->json, ":%"PRIuMAX, (uintmax_t) value);
>>
>> heh, I should learn not to reply in a hurry, just before
>> going out ...
>>
>> I had not noticed that 'value' was declared with an 'sized type'
>> of uint64_t, so using PRIu64 should be fine.
> 
> But why is this codepath using a sized type in the first place?  It
> is not like it wants to read/write a fixed binary file format---it
> just wants to use an integer type that is wide enough to handle any
> inttype the platform uses, for which uintmax_t would be a more
> appropriate type, no?

I must confess to not having given any thought to the wider implications of the code. I don't really know what this code is going to be used for. [Although I did shudder when I read some mention of a 'universal interchange format' - I still have nightmares about XML :-D ]

ATB, Ramsay Jones