| Message ID | 1479922617-4400-2-git-send-email-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Eric Blake <eblake@redhat.com> writes: > The qobject_from_jsonf() function implements a pseudo-printf > language for creating a QObject; however, it is hard-coded to > only parse a subset of formats understood by -Wformat, and is > not a straight synonym to bare printf(). In particular, any > use of an int64_t integer works only if the system's > definition of PRId64 matches what the parser expects; which > works on glibc (%lld or %ld depending on 32- vs. 64-bit) and > mingw (%I64d), but not on Mac OS (%qd). Rather than enhance > the parser, it is just as easy to use 'long long', which we > know always works. There are few enough callers of > qobject_from_json[fv]() that it is easy to audit that this is > the only non-testsuite caller that was actually relying on > this particular conversion. > > Reported by: G 3 <programmingkidx@gmail.com> > Signed-off-by: Eric Blake <eblake@redhat.com> > > --- > v2: keep qobject_from_jsonf for now, but switch to %lld > --- > qapi/qmp-event.c | 17 ++++------------- > 1 file changed, 4 insertions(+), 13 deletions(-) > > diff --git a/qapi/qmp-event.c b/qapi/qmp-event.c > index 8bba165..e7c8755 100644 > --- a/qapi/qmp-event.c > +++ b/qapi/qmp-event.c > @@ -35,21 +35,12 @@ static void timestamp_put(QDict *qdict) > int err; > QObject *obj; > qemu_timeval tv; > - int64_t sec, usec; > > err = qemu_gettimeofday(&tv); > - if (err < 0) { > - /* Put -1 to indicate failure of getting host time */ > - sec = -1; > - usec = -1; > - } else { > - sec = tv.tv_sec; > - usec = tv.tv_usec; > - } > - > - obj = qobject_from_jsonf("{ 'seconds': %" PRId64 ", " > - "'microseconds': %" PRId64 " }", > - sec, usec); > + /* Put -1 to indicate failure of getting host time */ > + obj = qobject_from_jsonf("{ 'seconds': %lld, 'microseconds': %lld }", > + err < 0 ? -1LL : tv.tv_sec, > + err < 0 ? -1LL : tv.tv_usec); > qdict_put_obj(qdict, "timestamp", obj); > } The ternaries must both yield long long for this to work. They will, unless their last operand's rank is greater than long long's, or their last operand is unsigned long long. The latter case should be harmless in practice. The former case would be weird. Hmm. I think it's best to be a bit more explicit here. Let's cast the last operands to long long, or use long long variables. Your choice.
On 11/24/2016 05:00 AM, Markus Armbruster wrote: > Eric Blake <eblake@redhat.com> writes: > >> + /* Put -1 to indicate failure of getting host time */ >> + obj = qobject_from_jsonf("{ 'seconds': %lld, 'microseconds': %lld }", >> + err < 0 ? -1LL : tv.tv_sec, >> + err < 0 ? -1LL : tv.tv_usec); >> qdict_put_obj(qdict, "timestamp", obj); >> } > > The ternaries must both yield long long for this to work. They will, > unless their last operand's rank is greater than long long's, or their > last operand is unsigned long long. The latter case should be harmless > in practice. The former case would be weird. Hmm. > > I think it's best to be a bit more explicit here. Let's cast the last > operands to long long, or use long long variables. Your choice. I think I favor the cast, as in: err < 0 ? -1LL : (long long) tv.tv_sec
diff --git a/qapi/qmp-event.c b/qapi/qmp-event.c index 8bba165..e7c8755 100644 --- a/qapi/qmp-event.c +++ b/qapi/qmp-event.c @@ -35,21 +35,12 @@ static void timestamp_put(QDict *qdict) int err; QObject *obj; qemu_timeval tv; - int64_t sec, usec; err = qemu_gettimeofday(&tv); - if (err < 0) { - /* Put -1 to indicate failure of getting host time */ - sec = -1; - usec = -1; - } else { - sec = tv.tv_sec; - usec = tv.tv_usec; - } - - obj = qobject_from_jsonf("{ 'seconds': %" PRId64 ", " - "'microseconds': %" PRId64 " }", - sec, usec); + /* Put -1 to indicate failure of getting host time */ + obj = qobject_from_jsonf("{ 'seconds': %lld, 'microseconds': %lld }", + err < 0 ? -1LL : tv.tv_sec, + err < 0 ? -1LL : tv.tv_usec); qdict_put_obj(qdict, "timestamp", obj); }
The qobject_from_jsonf() function implements a pseudo-printf language for creating a QObject; however, it is hard-coded to only parse a subset of formats understood by -Wformat, and is not a straight synonym to bare printf(). In particular, any use of an int64_t integer works only if the system's definition of PRId64 matches what the parser expects; which works on glibc (%lld or %ld depending on 32- vs. 64-bit) and mingw (%I64d), but not on Mac OS (%qd). Rather than enhance the parser, it is just as easy to use 'long long', which we know always works. There are few enough callers of qobject_from_json[fv]() that it is easy to audit that this is the only non-testsuite caller that was actually relying on this particular conversion. Reported by: G 3 <programmingkidx@gmail.com> Signed-off-by: Eric Blake <eblake@redhat.com> --- v2: keep qobject_from_jsonf for now, but switch to %lld --- qapi/qmp-event.c | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-)