From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59686) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VY5Vc-0000dB-Sb for qemu-devel@nongnu.org; Sun, 20 Oct 2013 22:46:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VY5VT-0001m0-72 for qemu-devel@nongnu.org; Sun, 20 Oct 2013 22:46:20 -0400 Received: from e23smtp03.au.ibm.com ([202.81.31.145]:51742) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VY5VS-0001lm-Gg for qemu-devel@nongnu.org; Sun, 20 Oct 2013 22:46:11 -0400 Received: from /spool/local by e23smtp03.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 21 Oct 2013 12:46:06 +1000 Received: from d23relay03.au.ibm.com (d23relay03.au.ibm.com [9.190.235.21]) by d23dlp01.au.ibm.com (Postfix) with ESMTP id A23882CE8053 for ; Mon, 21 Oct 2013 13:46:02 +1100 (EST) Received: from d23av04.au.ibm.com (d23av04.au.ibm.com [9.190.235.139]) by d23relay03.au.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r9L2jpd133095718 for ; Mon, 21 Oct 2013 13:45:51 +1100 Received: from d23av04.au.ibm.com (localhost [127.0.0.1]) by d23av04.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r9L2k2Eu026234 for ; Mon, 21 Oct 2013 13:46:02 +1100 Message-ID: <52649569.2050909@linux.vnet.ibm.com> Date: Mon, 21 Oct 2013 10:46:01 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1382058681-14957-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1382058681-14957-6-git-send-email-xiawenc@linux.vnet.ibm.com> <52610165.3040809@redhat.com> <87sivyu8lw.fsf@blackfin.pond.sub.org> In-Reply-To: <87sivyu8lw.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=GB2312 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 05/13] error: define struct Error in only one place List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster , Paolo Bonzini Cc: kwolf@redhat.com, stefanha@gmail.com, qemu-devel@nongnu.org, lcapitulino@redhat.com ÓÚ 2013/10/18 19:22, Markus Armbruster дµÀ: > Paolo Bonzini writes: > >> Il 18/10/2013 03:11, Wenchao Xia ha scritto: >>> Signed-off-by: Wenchao Xia >>> --- >>> include/qapi/error.h | 5 ++++- >>> qobject/qerror.c | 7 ------- >>> util/error.c | 6 ------ >>> 3 files changed, 4 insertions(+), 14 deletions(-) >>> >>> diff --git a/include/qapi/error.h b/include/qapi/error.h >>> index 7d4c696..8688aaf 100644 >>> --- a/include/qapi/error.h >>> +++ b/include/qapi/error.h >>> @@ -20,7 +20,10 @@ >>> * A class representing internal errors within QEMU. An error has a ErrorClass >>> * code and a human message. >>> */ >>> -typedef struct Error Error; >>> +typedef struct Error { >>> + char *msg; >>> + ErrorClass err_class; >>> +} Error; >> Please add a comment that it should be treated as an opaque type. > Or keep it opaque here, and complete the type in an internal header. > But see below. > >> Paolo >> >>> >>> /** >>> * Set an indirect pointer to an error given a ErrorClass value and a >>> diff --git a/qobject/qerror.c b/qobject/qerror.c >>> index 3aee1cf..5b487f3 100644 >>> --- a/qobject/qerror.c >>> +++ b/qobject/qerror.c >>> @@ -97,13 +97,6 @@ void qerror_report(ErrorClass eclass, const char *fmt, ...) >>> } >>> } >>> >>> -/* Evil... */ >>> -struct Error >>> -{ >>> - char *msg; >>> - ErrorClass err_class; >>> -}; >>> - >>> void qerror_report_err(Error *err) >>> { >>> QError *qerr; > qerr = qerror_new(); > loc_save(&qerr->loc); > qerr->err_msg = g_strdup(err->msg); > qerr->err_class = err->err_class; > > if (monitor_cur_is_qmp()) { > monitor_set_error(cur_mon, qerr); > } else { > qerror_print(qerr); > QDECREF(qerr); > } > } > > This is the only use of the "evil" duplicate. I suspect it could be > cleaned up like this: > > qerr->err_msg = g_strdup(error_get_pretty(err)); > qerr->err_class = error_get_class(err); > > If that's true, the duplicate goes away, and we can keep the type > opaque. seems a smart idea, will use it. >>> diff --git a/util/error.c b/util/error.c >>> index ec0faa6..da0d221 100644 >>> --- a/util/error.c >>> +++ b/util/error.c >>> @@ -17,12 +17,6 @@ >>> #include "qapi-types.h" >>> #include "qapi/qmp/qerror.h" >>> >>> -struct Error >>> -{ >>> - char *msg; >>> - ErrorClass err_class; >>> -}; >>> - >>> void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...) >>> { >>> Error *err; >>>