[Qemu-devel] [PATCH 40/56] json: Replace %I64d, %I64u by %PRId64, %PRIu64

Markus Armbruster posted 56 patches 7 years, 2 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 40/56] json: Replace %I64d, %I64u by %PRId64, %PRIu64
Posted by Markus Armbruster 7 years, 2 months ago
Support for %I64d got addded in commit 2c0d4b36e7f "json: fix PRId64
on Win32".  We had to hard-code I64d because we used the lexer's
finite state machine to check interpolations.  No more, so clean this
up.

Additional conversion specifications would be easy enough to implement
when needed.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qobject/json-parser.c | 10 ++++++----
 tests/check-qjson.c   | 10 ++++++++++
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index bd137399e5..350a9d267b 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -433,16 +433,18 @@ static QObject *parse_interpolation(JSONParserContext *ctxt, va_list *ap)
         return QOBJECT(qnum_from_int(va_arg(*ap, int)));
     } else if (!strcmp(token->str, "%ld")) {
         return QOBJECT(qnum_from_int(va_arg(*ap, long)));
-    } else if (!strcmp(token->str, "%lld") ||
-               !strcmp(token->str, "%I64d")) {
+    } else if (!strcmp(token->str, "%lld")) {
         return QOBJECT(qnum_from_int(va_arg(*ap, long long)));
+    } else if (!strcmp(token->str, "%" PRId64)) {
+        return QOBJECT(qnum_from_int(va_arg(*ap, int64_t)));
     } else if (!strcmp(token->str, "%u")) {
         return QOBJECT(qnum_from_uint(va_arg(*ap, unsigned int)));
     } else if (!strcmp(token->str, "%lu")) {
         return QOBJECT(qnum_from_uint(va_arg(*ap, unsigned long)));
-    } else if (!strcmp(token->str, "%llu") ||
-               !strcmp(token->str, "%I64u")) {
+    } else if (!strcmp(token->str, "%llu")) {
         return QOBJECT(qnum_from_uint(va_arg(*ap, unsigned long long)));
+    } else if (!strcmp(token->str, "%" PRIu64)) {
+        return QOBJECT(qnum_from_uint(va_arg(*ap, uint64_t)));
     } else if (!strcmp(token->str, "%s")) {
         return QOBJECT(qstring_from_str(va_arg(*ap, const char *)));
     } else if (!strcmp(token->str, "%f")) {
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 895be489b3..fbb607c227 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -919,9 +919,11 @@ static void keyword_literal(void)
 static void interpolation(void)
 {
     long long value_lld = 0x123456789abcdefLL;
+    int64_t value_d64 = value_lld;
     long value_ld = (long)value_lld;
     int value_d = (int)value_lld;
     unsigned long long value_llu = 0xfedcba9876543210ULL;
+    uint64_t value_u64 = value_llu;
     unsigned long value_lu = (unsigned long)value_llu;
     unsigned value_u = (unsigned)value_llu;
     double value_f = 2.323423423;
@@ -959,6 +961,10 @@ static void interpolation(void)
     g_assert_cmpint(qnum_get_int(qnum), ==, value_lld);
     qobject_unref(qnum);
 
+    qnum = qobject_to(QNum, qobject_from_jsonf_nofail("%" PRId64, value_d64));
+    g_assert_cmpint(qnum_get_int(qnum), ==, value_lld);
+    qobject_unref(qnum);
+
     qnum = qobject_to(QNum, qobject_from_jsonf_nofail("%u", value_u));
     g_assert_cmpuint(qnum_get_uint(qnum), ==, value_u);
     qobject_unref(qnum);
@@ -971,6 +977,10 @@ static void interpolation(void)
     g_assert_cmpuint(qnum_get_uint(qnum), ==, value_llu);
     qobject_unref(qnum);
 
+    qnum = qobject_to(QNum, qobject_from_jsonf_nofail("%" PRIu64, value_u64));
+    g_assert_cmpuint(qnum_get_uint(qnum), ==, value_llu);
+    qobject_unref(qnum);
+
     qnum = qobject_to(QNum, qobject_from_jsonf_nofail("%f", value_f));
     g_assert(qnum_get_double(qnum) == value_f);
     qobject_unref(qnum);
-- 
2.17.1


Re: [Qemu-devel] [PATCH 40/56] json: Replace %I64d, %I64u by %PRId64, %PRIu64
Posted by Eric Blake 7 years, 2 months ago
On 08/08/2018 07:03 AM, Markus Armbruster wrote:
> Support for %I64d got addded in commit 2c0d4b36e7f "json: fix PRId64

s/addded/added/

> on Win32".  We had to hard-code I64d because we used the lexer's
> finite state machine to check interpolations.  No more, so clean this
> up.
> 
> Additional conversion specifications would be easy enough to implement
> when needed.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   qobject/json-parser.c | 10 ++++++----
>   tests/check-qjson.c   | 10 ++++++++++
>   2 files changed, 16 insertions(+), 4 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

>          return QOBJECT(qnum_from_int(va_arg(*ap, long)));
> -    } else if (!strcmp(token->str, "%lld") ||
> -               !strcmp(token->str, "%I64d")) {
> +    } else if (!strcmp(token->str, "%lld")) {
>          return QOBJECT(qnum_from_int(va_arg(*ap, long long)));
> +    } else if (!strcmp(token->str, "%" PRId64)) {
> +        return QOBJECT(qnum_from_int(va_arg(*ap, int64_t)));

I had a double-take to make sure this still works on mingw. The trick 
used to be that the lexer had to parse the union of all forms understood 
by any libc (making Linux understand %I64d even though only mingw would 
generate it) then the parser had to accept all forms allowed through by 
the lexer.  Now the lexer accepts all forms with no effort (because it 
is no longer validates), and the parser is made stricter (%I64d no 
longer works on Linux, where we have two redundant 'if' clauses; but 
mingw has two distinct 'if' clauses and works as desired).

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 40/56] json: Replace %I64d, %I64u by %PRId64, %PRIu64
Posted by Markus Armbruster 7 years, 2 months ago
Eric Blake <eblake@redhat.com> writes:

> On 08/08/2018 07:03 AM, Markus Armbruster wrote:
>> Support for %I64d got addded in commit 2c0d4b36e7f "json: fix PRId64
>
> s/addded/added/

Fixing...

>> on Win32".  We had to hard-code I64d because we used the lexer's
>> finite state machine to check interpolations.  No more, so clean this
>> up.
>>
>> Additional conversion specifications would be easy enough to implement
>> when needed.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   qobject/json-parser.c | 10 ++++++----
>>   tests/check-qjson.c   | 10 ++++++++++
>>   2 files changed, 16 insertions(+), 4 deletions(-)
>>
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>
>>          return QOBJECT(qnum_from_int(va_arg(*ap, long)));
>> -    } else if (!strcmp(token->str, "%lld") ||
>> -               !strcmp(token->str, "%I64d")) {
>> +    } else if (!strcmp(token->str, "%lld")) {
>>          return QOBJECT(qnum_from_int(va_arg(*ap, long long)));
>> +    } else if (!strcmp(token->str, "%" PRId64)) {
>> +        return QOBJECT(qnum_from_int(va_arg(*ap, int64_t)));
>
> I had a double-take to make sure this still works on mingw. The trick
> used to be that the lexer had to parse the union of all forms
> understood by any libc (making Linux understand %I64d even though only
> mingw would generate it) then the parser had to accept all forms
> allowed through by the lexer.  Now the lexer accepts all forms with no
> effort (because it is no longer validates), and the parser is made
> stricter (%I64d no longer works on Linux, where we have two redundant
> 'if' clauses; but mingw has two distinct 'if' clauses and works as
> desired).

Exactly.  Thanks for checking!