Re: [Qemu-devel] [RFC PATCH V3 2/5] qapi: add event helper functions

[Wenchao Xia <wenchaoqemu@gmail.com>]

于 2014/3/21 6:53, Eric Blake 写道:
> On 03/18/2014 11:16 PM, Wenchao Xia wrote:
>> This file hold some functions that do not need to be generated.
> s/hold/holds/
>
>> Signed-off-by: Wenchao Xia<wenchaoqemu@gmail.com>
>> ---
>>   include/qapi/qmp-event.h |   25 ++++++++++++++++
>>   qapi/Makefile.objs       |    1 +
>>   qapi/qmp-event.c         |   71 ++++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 97 insertions(+), 0 deletions(-)
>>   create mode 100644 include/qapi/qmp-event.h
>>   create mode 100644 qapi/qmp-event.c
>>
>> diff --git a/include/qapi/qmp-event.h b/include/qapi/qmp-event.h
>> new file mode 100644
>> index 0000000..fdf1a7f
>> --- /dev/null
>> +++ b/include/qapi/qmp-event.h
>> @@ -0,0 +1,25 @@
>> +/*
>> + * QMP Event related
>> + *
>> + * Authors:
>> + *  Wenchao Xia<wenchaoqemu@gmail.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>> + * See the COPYING.LIB file in the top-level directory.
> For the [L]GPL to work, someone must assert copyright.
   Will fix.

>> +++ b/qapi/qmp-event.c
>> @@ -0,0 +1,71 @@
>> +/*
>> + * QMP Event related
>> + *
>> + * Authors:
>> + *  Wenchao Xia<wenchaoqemu@gmail.com>
>> + *
>> + * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
>> + * See the COPYING.LIB file in the top-level directory.
> Again, missing an actual use of the word "Copyright".
>
>> +
>> +typedef struct QMPEventFunctions {
>> +    QMPEventFuncEmit emit;
>> +} QMPEventFunctions;
>> +
>> +QMPEventFunctions qmp_event_functions;
>> +
>> +void qmp_event_set_func_emit(QMPEventFuncEmit emit)
>> +{
>> +    qmp_event_functions.emit = emit;
>> +}
>> +
>> +QMPEventFuncEmit qmp_event_get_func_emit(void)
>> +{
>> +    return qmp_event_functions.emit;
>> +}
> Is this struct a bit overkill, or do you extend it to include other
> fields later?
   No other fields will be added in this series, it allow different emit 
function hooked.
Do you mean remove it and put it into generated qapi-event.c?


>
>> +    err = qemu_gettimeofday(&tv);
>> +    if (err<  0) {
>> +        /* Put -1 to indicate failure of getting host time */
>> +        tv.tv_sec = tv.tv_usec = -1;
> Believe it or not, this is NOT portable.  Let's consider what happens
> when tv_sec is int64_t and tv_usec is uint32_t.  Assignments happen
> right to left, so tv_usec gets the unsigned value 0xffffffff, then since
> all uint32_t values fit in int64_t, integer promotion says that the
> value is 0-extended (not sign-extended), and tv_sec is NOT assigned -1.
>   Solution: break this into two separate statements:
>
> tv.tv_sec = -1;
> tv.tv_usec = -1;
   Good catch, thanks!


>> +    }
>> +
>> +    obj = qobject_from_jsonf("{ 'seconds': %" PRId64 ", "
>> +                                "'microseconds': %" PRId64 " }",
>> +                                (int64_t) tv.tv_sec, (int64_t) tv.tv_usec);
> Indentation is odd, but that's cosmetic.
   Will fix.