json-writer: incorrect format specifier
cover json-writer: incorrect format specifier
Building the pu branch at commit 8b49f5c076c using Travis-CI all of the linux builds are green but the two OSX builds are red[1] and the logs show compile errors:
CC ident.o
CC json-writer.o
json-writer.c:123:38: error: format specifies type 'uintmax_t' (aka 'unsigned long') but the argument has type 'uint64_t' (aka 'unsigned long long') [-Werror,-Wformat]
strbuf_addf(&jw->json, ":%"PRIuMAX, value);
~~ ^~~~~json-writer.c:228:37: error: format specifies type 'uintmax_t' (aka 'unsigned long') but the argument has type 'uint64_t' (aka 'unsigned long long') [-Werror,-Wformat] [0m
strbuf_addf(&jw->json, "%"PRIuMAX, value);
~~ ^~~~~2 errors generated.
make: *** [json-writer.o] Error 1
make: *** Waiting for unfinished jobs....
[RFC Patch 1/1] changes the PRIuMax to PRIu64 to correct the compile error and now Travis-CI is green [2].
[1]: https://travis-ci.org/winksaville/git/builds/357660624
[2]: https://travis-ci.org/winksaville/git/builds/357681929
Wink Saville (1):
json-writer: incorrect format specifier
json-writer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Signature · 1 line
2.16.2
RFC PATCH 1/1 json-writer: incorrect format specifier
In routines jw_object_uint64 and jw_object_double strbuf_addf is invoked with strbuf_addf(&jw->json, ":%"PRIuMAX, value) where value is a uint64_t. This causes a compile error on OSX.
The correct format specifier is PRIu64 instead of PRIuMax.
--- json-writer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
json-writer.c +2 -2
@@ -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); } void jw_object_double(struct json_writer *jw, const char *fmt,
@@ -225,7 +225,7 @@ void jw_array_uint64(struct json_writer *jw, uint64_t value)assert_in_array (jw);maybe_add_comma (jw); -strbuf_addf (&jw->json,"%" PRIuMAX, value); +strbuf_addf (&jw->json,"%" PRIu64, value); }void jw_array_double (struct json_writer *jw,const char *fmt,double value)
Signature · 1 line
2.16.2
Quoted 7 lines · Wink Saville
In routines
jw_object_uint64andjw_object_doublestrbuf_addfis invoked withstrbuf_addf(&jw->json, ":%"PRIuMAX, value)where value is auint64_t. This causes a compile error on OSX.The correct format specifier is PRIu64 instead of PRIuMax.
Signed-off-by Wink Saville <[email protected]>
That's odd.
A grep on the Git source tree did not find a "PRIu64" symbol. Searching public-inbox only found one message [1] talking about it (other than the ones associated with your messages here).
I have to wonder if that is defined in a OSX header file and you're getting it from there [2]. (I don't have a MAC in front of me, so I can't verify what's in that header.) But [2] defines PRIuMAX as PRIu64, so we shouldn't need to make that change in json-writer -- unless something is getting lost in the #ifdefs.
Could you double check this in the header files on your system? Any chance you are doing a 32-bit build?
Thanks
Jeff
[2] https://opensource.apple.com/source/gcc/gcc-926/inttypes.h.auto.html
Quoted 27 lines
--- json-writer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)json-writer.c +2
-2@@ -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); } void jw_object_double(struct json_writer *jw, const char *fmt,@@ -225,7 +225,7 @@ void jw_array_uint64(struct json_writer *jw, uint64_t value)assert_in_array (jw);maybe_add_comma (jw); -strbuf_addf (&jw->json,"%" PRIuMAX, value); +strbuf_addf (&jw->json,"%" PRIu64, value); }void jw_array_double (struct json_writer *jw,const char *fmt,double value)
Quoted 16 lines · Wink Saville
In routines
jw_object_uint64andjw_object_doublestrbuf_addfis invoked withstrbuf_addf(&jw->json, ":%"PRIuMAX, value)where value is auint64_t. This causes a compile error on OSX.The correct format specifier is PRIu64 instead of PRIuMax.
Signed-off-by Wink Saville <[email protected]>--- json-writer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)json-writer.c +0
-0
json-writer.c void jw_object_uint64(struct json_writer *jw, const char *key, uint64_t value) · line 123
Quoted 5 lines
json-writer.c +1
-1@@ -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);
ATB,
Ramsay Jones
Quoted 21 lines · Ramsay Jones
Quoted 18 lines · Wink Saville
In routines
jw_object_uint64andjw_object_doublestrbuf_addfis invoked withstrbuf_addf(&jw->json, ":%"PRIuMAX, value)where value is auint64_t. This causes a compile error on OSX.The correct format specifier is PRIu64 instead of PRIuMax.
Signed-off-by Wink Saville <[email protected]>--- json-writer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)json-writer.c +0
-0@@ -120,7 +120,7 @@ void jw_object_uint64(struct json_writer *jw, const char *key, uint64_t value)maybe_add_comma (jw);
Quoted 7 lines
json-writer.c void jw_object_uint64(struct json_writer *jw, const char *key, uint64_t value) · line 123
Quoted 3 lines
json-writer.c +1
-1append_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. Well, except that you may have to add a 'fallback' definition of PRIu64 to one of the 'compat/mingw.h', 'compat/msvc.h' or 'git-compat-util.h' header files. (see e.g. PRId64 at compat/mingw.h:429).
[About a decade ago, I heard microsoft were implementing C99 'real soon now' ;-) ]
ATB,
Ramsay Jones
cover json-writer: add cast to uintmax_t
In reply to Wink SavilleBuilding the pu branch at commit 8b49f5c076c using Travis-Ci all linux builds worked but the two OSX builds failed with:
CC ident.o
CC json-writer.o
json-writer.c:123:38: error: format specifies type 'uintmax_t' (aka 'unsigned long') but the argument has type 'uint64_t' (aka 'unsigned long long') [-Werror,-Wformat]
strbuf_addf(&jw->json, ":%"PRIuMAX, value);
json-writer.c:228:37: error: format specifies type 'uintmax_t' (aka 'unsigned long') but the argument has type 'uint64_t' (aka 'unsigned long long') [-Werror,-Wformat]
strbuf_addf(&jw->json, "%"PRIuMAX, value);
Corrected in Patch 1/1 by casting value to uintmax_t.
Wink Saville (1):
json-writer: add cast to uintmax_t
json-writer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Signature · 1 line
2.16.2
RFC PATCH v2 1/1 json-writer: add cast to uintmax_t
Correct a compile error on Mac OSX by adding a cast to uintmax_t in calls to strbuf_addf.
--- json-writer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
json-writer.c +2 -2
@@ -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,":%" PRIuMAX, (uintmax_t )value); } void jw_object_double(struct json_writer *jw, const char *fmt,
@@ -225,7 +225,7 @@ void jw_array_uint64(struct json_writer *jw, uint64_t value)assert_in_array (jw);maybe_add_comma (jw); -strbuf_addf (&jw->json,"%" PRIuMAX, value); +strbuf_addf (&jw->json,"%" PRIuMAX, (uintmax_t )value); }void jw_array_double (struct json_writer *jw,const char *fmt,double value)
Signature · 1 line
2.16.2
Quoted 9 lines · Ramsay Jones
Quoted 9 lines
json-writer.c void jw_object_uint64(struct json_writer *jw, const char *key, uint64_t value) · line 123
Quoted 6 lines
json-writer.c +1
-1@@ -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:
Quoted 7 lines
Quoted 1 line
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?
Re: [RFC PATCH v2 1/1] json-writer: add cast to uintmax_t
In reply to Wink SavilleQuoted 25 lines · Wink Saville
Correct a compile error on Mac OSX by adding a cast to
uintmax_tin calls tostrbuf_addf.Helped-by Ramsay Jones <[email protected]>Tested-by travis-ciSigned-off-by Wink Saville <[email protected]>--- json-writer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)json-writer.c +1
-1@@ -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,":%" PRIuMAX, (uintmax_t )value); } void jw_object_double(struct json_writer *jw, const char *fmt,
json-writer.c void jw_array_uint64(struct json_writer *jw, uint64_t value) · line 231
Quoted 8 lines
json-writer.c +1
-1@@ -225,7 +225,7 @@ void jw_array_uint64(struct json_writer *jw, uint64_t value)assert_in_array (jw);maybe_add_comma (jw); -strbuf_addf (&jw->json,"%" PRIuMAX, value); +strbuf_addf (&jw->json,"%" PRIuMAX, (uintmax_t )value); }void jw_array_double (struct json_writer *jw,const char *fmt,double value)
FYI. I included and squashed this change into V4 of my json-writer series.
Jeff
Quoted 16 lines · Junio C Hamano
Quoted 14 lines · Ramsay Jones
Quoted 10 lines
json-writer.c void jw_object_uint64(struct json_writer *jw, const char *key, uint64_t value) · line 123
Quoted 6 lines
json-writer.c +1
-1@@ -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 ...
Quoted 9 lines
Quoted 2 lines
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_twould be a more appropriate type, no?
[Somehow the conversation forked and this compiler warning appeared in both the json-writer and the rebase-interactive threads. I'm copying here the response that I already made on the latter.]
I defined that routine to take a uint64_t because I wanted to pass a nanosecond value received from getnanotime() and that's what it returns.
My preference would be to change the PRIuMAX to PRIu64, but there aren't any other references in the code to that symbol and I didn't want to start a new trend here.
I am concerned that the above compiler error message says that uintmax_t is defined as an "unsigned long" (which is defined as *at least* 32 bits, but not necessarily 64. But a uint64_t is defined as a "unsigned long long" and guaranteed as a 64 bit value.
So while I'm not really worried about 128 bit integers right now, I'm more concerned about 32 bit compilers truncating that value without any warnings.
Jeff
Quoted 16 lines · Junio C Hamano
Quoted 14 lines · Ramsay Jones
Quoted 10 lines
json-writer.c void jw_object_uint64(struct json_writer *jw, const char *key, uint64_t value) · line 123
Quoted 6 lines
json-writer.c +1
-1@@ -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 ...
Quoted 8 lines
Quoted 2 lines
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_twould 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
Quoted 2 lines · Ramsay Jones
Quoted 1 line · Junio C Hamano
Ramsay Jones <[email protected]> writes:
[...]
Quoted 5 lines
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 ]
[...]
My current goals are to add telemetry in a friendly way and have events written in JSON to some audit destination. Something like:
{ "argv":["./git","status"],
"pid":84941,
"exit-code":0,
"elapsed-time":0.011121,
"version":"2.16.2.5.g71445db.dirty",
... }Later, we could add a JSON formatter to a command like "status" and then do things like:
$ git status --json | python '... json.load ...'
and eliminate the need to write custom parsers for normal or porcelain formats. There are other commands that could be similarly adapted and save callers a lot of screen-scraping code. But that is later.
Thanks,
Jeff