Eric Blake <ebl...@redhat.com> writes:
> On 03/27/2014 02:03 AM, Wenchao Xia wrote:
>> This file holds some functions that do not need to be generated.
>>
>> Signed-off-by: Wenchao Xia <wenchaoq...@gmail.com>
>> ---
>> include/qapi/qmp-event.h | 27 +++++++++++++++++
>> qapi/Makefile.objs | 1 +
>> qapi/qmp-event.c | 70
>> ++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 98 insertions(+), 0 deletions(-)
>> create mode 100644 include/qapi/qmp-event.h
>> create mode 100644 qapi/qmp-event.c
>>
>
>> + err = qemu_gettimeofday(&tv);
>> + if (err < 0) {
>> + /* Put -1 to indicate failure of getting host time */
>> + tv.tv_sec = -1;
>> + tv.tv_usec = -1;
>
> You fixed the problem with C promotion here, but...
>
>> + }
>> +
>> + obj = qobject_from_jsonf("{ 'seconds': %" PRId64 ", "
>> + "'microseconds': %" PRId64 " }",
>> + (int64_t) tv.tv_sec, (int64_t) tv.tv_usec);
>
> ...here, C promotion rules bite once again :( If tv_usec is uint32_t,
> then it zero-extends rather than sign-extends into int64_t, and you may
> end up with 0xffffffff instead of the intended -1. When doing
> potentially widening casts, it is only safe if you know the signedness
> of the pre-cast value; but with struct timeval, POSIX doesn't make that
> easy.
>
> Maybe it's easier to just rewrite things with known types:
>
> int64_t sec;
> int usec;
> qemu_timval tv;
> err = qemu_gettimeofday(&tv);
> if (err < 0) {
> sec = -1;
> usec = -1;
> } else {
> sec = tv.tv_sec;
> usec = tv.tv_usec;
> }
> qobject_from_jsonf("... %"PRId64 ", ...%d", sec, usec)
>
> since 'int' is guaranteed to be large enough for all the usec values we
> care about on all platforms we compile on (that is, we require 32-bit
> int, even if C allows for a 16-bit int implementation).
If you go that route, then why not go all the way nad make both sec and
usec int64_t? Look ma, no type casts!
