From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:32911) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwX1B-0002ic-CB for qemu-devel@nongnu.org; Wed, 01 Aug 2012 07:23:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SwX15-0004Di-KJ for qemu-devel@nongnu.org; Wed, 01 Aug 2012 07:23:09 -0400 Received: from mail-yw0-f45.google.com ([209.85.213.45]:45116) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SwX15-0004DY-Fu for qemu-devel@nongnu.org; Wed, 01 Aug 2012 07:23:03 -0400 Received: by yhpp34 with SMTP id p34so6908728yhp.4 for ; Wed, 01 Aug 2012 04:23:02 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20120801105011.GC17816@stefanha-thinkpad.localdomain> References: <1342539951-30915-1-git-send-email-akong@redhat.com> <1343811543-12137-2-git-send-email-akong@redhat.com> <20120801105011.GC17816@stefanha-thinkpad.localdomain> Date: Wed, 1 Aug 2012 12:23:02 +0100 Message-ID: From: Peter Maydell Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [Qemu-trivial] [RESEND PATCH 1/3] socket: remove redundant check List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: qemu-trivial@nongnu.org, aliguori@us.ibm.com, Amos Kong , qemu-devel@nongnu.org, quintela@redhat.com On 1 August 2012 11:50, Stefan Hajnoczi wrote: > This isn't obvious. It looks like the intent of the if (!e->ai_next) is > to suppress the error so that the next iteration of the *outer* loop can > succeed. Yeah, we only call it an error on the last time round. This loop is a bit confusingly structured, since we're effectively handling the failure in several places at once: we always fprintf() something, then we set the error on the last time round the loop, then at the end of the loop we fprintf again. We also duplicate the loop termination condition. It might be better to have an Error *local_err in this function. Then we could unconditionally call error_set() for any failures, passing &local_err. Then at the end of the loop we can call error_propagate(errp, local_err) to pass an error up if we didn't succeed at all. Unfortunately you'd have to do if (error_is_set(&local_err)) { error_free(&local_err); } error_set(&local_err, QERR_whatever); for the error setting, since error_set() will assert if you try to set an error twice. [Another demonstration of the fprintf errors being much more useful than the error_set mechanisms at the moment, incidentally. We can get away with the fprintfs because the only caller of this function which passes in a non-NULL errp is the migration code called from vl.c, which is just going to fprintf and exit on error anyway.] -- PMM