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

[Wenchao Xia]

于 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<address@hidden>
> > ---
> >   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<address@hidden>
> > + *
> > + * 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<address@hidden>
> > + *
> > + * 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.