From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39799) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WRt8i-0002Wq-1V for qemu-devel@nongnu.org; Sun, 23 Mar 2014 20:53:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WRt8Z-0001J4-L6 for qemu-devel@nongnu.org; Sun, 23 Mar 2014 20:53:19 -0400 Received: from mail-ie0-x232.google.com ([2607:f8b0:4001:c03::232]:39572) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WRt8Z-0001Ir-FY for qemu-devel@nongnu.org; Sun, 23 Mar 2014 20:53:11 -0400 Received: by mail-ie0-f178.google.com with SMTP id lx4so4619864iec.23 for ; Sun, 23 Mar 2014 17:53:10 -0700 (PDT) Message-ID: <532F81ED.7010309@gmail.com> Date: Mon, 24 Mar 2014 08:53:01 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1395206201-22999-1-git-send-email-wenchaoqemu@gmail.com> <1395206201-22999-3-git-send-email-wenchaoqemu@gmail.com> <532B7160.9020201@redhat.com> In-Reply-To: <532B7160.9020201@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [RFC PATCH V3 2/5] qapi: add event helper functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: mdroth@linux.vnet.ibm.com, lcapitulino@redhat.com, qemu-devel@nongnu.org, armbru@redhat.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 >> --- >> 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 >> + * >> + * 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 >> + * >> + * 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.