From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:52750) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDFaK-0006co-OC for qemu-devel@nongnu.org; Wed, 06 Mar 2013 09:44:55 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UDFaE-00012s-8G for qemu-devel@nongnu.org; Wed, 06 Mar 2013 09:44:48 -0500 Received: from mx1.redhat.com ([209.132.183.28]:20076) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UDFaD-00012m-V3 for qemu-devel@nongnu.org; Wed, 06 Mar 2013 09:44:42 -0500 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r26Eifw0021913 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 6 Mar 2013 09:44:41 -0500 Message-ID: <513756D5.1020506@redhat.com> Date: Wed, 06 Mar 2013 15:46:45 +0100 From: Laszlo Ersek MIME-Version: 1.0 References: <1362566886-14073-1-git-send-email-kwolf@redhat.com> <513722BD.6010503@redhat.com> <20130306111126.GA2285@dhcp-200-207.str.redhat.com> In-Reply-To: <20130306111126.GA2285@dhcp-200-207.str.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] qemu-sockets: Fix assertion failure List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: Paolo Bonzini , qemu-devel@nongnu.org, Luiz Capitulino On 03/06/13 12:11, Kevin Wolf wrote: > Am 06.03.2013 um 12:04 hat Paolo Bonzini geschrieben: >> Il 06/03/2013 11:48, Kevin Wolf ha scritto: >>> inet_connect_opts() tries all possible addrinfos returned by >>> getaddrinfo(). If one fails with an error, the next one is tried. In >>> this case, the Error should be discarded because the whole operation is >>> successful if another addrinfo from the list succeeds; and if it >>> doesn't, setting an already set Error will trigger an assertion failure. >>> >>> Signed-off-by: Kevin Wolf >>> --- >>> util/qemu-sockets.c | 8 ++++++++ >>> 1 file changed, 8 insertions(+) >>> >>> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c >>> index 1350ccc..32e609a 100644 >>> --- a/util/qemu-sockets.c >>> +++ b/util/qemu-sockets.c >>> @@ -373,6 +373,14 @@ int inet_connect_opts(QemuOpts *opts, Error **errp, >>> } >>> >>> for (e = res; e != NULL; e = e->ai_next) { >>> + >>> + /* Overwriting errors isn't allowed, so clear any error that may have >>> + * occured in the previous iteration */ >>> + if (error_is_set(errp)) { >>> + error_free(*errp); >>> + *errp = NULL; >>> + } >>> + >>> if (connect_state != NULL) { >>> connect_state->current_addr = e; >>> } >>> >> >> Should we also do nothing if errp is not NULL on entry? > > We could assert(!error_is_set(errp)) if we wanted. As soon as you've got > an Error, you must return instead of calling more functions with the > same error pointer. I think Luiz would suggest (*) to receive any error into a NULL-initialized local_err pointer; do the logic above on local_err, and just before returning, error_propagate() it to errp. (*) I hope you can see what I did there: if you disagree, you get to take that to Luiz, even though he didn't say anything. I'm getting better at working this list! :) Laszlo