[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC PATCH V4 2/5] qapi: add event helper functions
From: |
Wenchao Xia |
Subject: |
Re: [Qemu-devel] [RFC PATCH V4 2/5] qapi: add event helper functions |
Date: |
Wed, 02 Apr 2014 17:44:51 +0800 |
User-agent: |
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 |
δΊ 2014/3/28 16:21, Markus Armbruster ει:
> Eric Blake <address@hidden> 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 <address@hidden>
>>> ---
>>> 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!
>
Yep, in this way code looks better... I will respin it later with a
full conversion.
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [RFC PATCH V4 2/5] qapi: add event helper functions,
Wenchao Xia <=