From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:42166) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UGZUx-0007zk-QI for qemu-devel@nongnu.org; Fri, 15 Mar 2013 14:37:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UGZUu-0004xa-TC for qemu-devel@nongnu.org; Fri, 15 Mar 2013 14:36:59 -0400 Received: from mx1.redhat.com ([209.132.183.28]:25621) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UGZUu-0004xS-KK for qemu-devel@nongnu.org; Fri, 15 Mar 2013 14:36:56 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id r2FIatu9016197 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 15 Mar 2013 14:36:55 -0400 Message-ID: <51436AC8.7030701@redhat.com> Date: Fri, 15 Mar 2013 19:39:04 +0100 From: Laszlo Ersek MIME-Version: 1.0 References: <87a9qg5xjf.fsf@blackfin.pond.sub.org> <1363273057-25850-1-git-send-email-kwolf@redhat.com> <5141F224.3010108@redhat.com> <20130315083706.GD2418@dhcp-200-207.str.redhat.com> <51435285.4010300@redhat.com> <20130315175550.GE2418@dhcp-200-207.str.redhat.com> In-Reply-To: <20130315175550.GE2418@dhcp-200-207.str.redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH] qemu-socket: Use local error variable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com On 03/15/13 18:55, Kevin Wolf wrote: > However this won't be the last time that I have to deal with an Error > object, so I thought I'd check what is good practice. Seems no such > thing has established yet, which is an answer, even though not the one I > was hoping for. What I've gathered from discussions with Luiz and Markus, there is indeed no official Error*-handling-style. FWIW personally I think that my suggestion was quite close to a good (I'd even hazard "elegant") approach. I did notice that it would look terrible in the function at hand if applied directly (I actually started to code it up as an "illustrative patch"). For the emergent fugliness I blamed inet_connect_opts()'s current structure (several exit points, transfer of ownership without documentation, etc). So for the illustration I would have had to restructure the function. That in turn would have depended on me understanding the non-trivial life cycle (ownership) of "connect_state" / "res" under the different return conditions. (That is, when we bail out due to "in progress", the "connect_state" and the rest of the addrinfo list is: - either referenced elsewhere, - or freed, - or leaked currently.) I didn't (don't) have time/energy for that -- my bad. In general, murky ownership transfers seem to be characteristic of qemu. When a function allocates dynamic memory, it should: (1) either free it unconditionally (temp working space), (2) free it on error, return it on success (constructor), (3) transfer the ownership by function call (huge comment or telling function name). This includes any refcount increments by the callee. ... The function name "inet_connect_addr" tells us nothing about qemu_set_fd_handler2() transferring the ownership of "connect_state" (and the off-hanging addrinfo list) to the global "io_handlers". inet_connect_opts inet_connect_addr qemu_set_fd_handler2 ownership transfer in one case release stuff in two other cases Thanks, Laszlo