From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=39699 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PJ7Bw-0001oj-Pd for qemu-devel@nongnu.org; Thu, 18 Nov 2010 11:18:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PJ7Bv-0004Sv-Gi for qemu-devel@nongnu.org; Thu, 18 Nov 2010 11:18:32 -0500 Received: from e9.ny.us.ibm.com ([32.97.182.139]:52730) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PJ7Bv-0004SU-DX for qemu-devel@nongnu.org; Thu, 18 Nov 2010 11:18:31 -0500 Received: from d01relay06.pok.ibm.com (d01relay06.pok.ibm.com [9.56.227.116]) by e9.ny.us.ibm.com (8.14.4/8.13.1) with ESMTP id oAIFukCw016711 for ; Thu, 18 Nov 2010 10:56:46 -0500 Received: from d01av01.pok.ibm.com (d01av01.pok.ibm.com [9.56.224.215]) by d01relay06.pok.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id oAIGIT2C1286214 for ; Thu, 18 Nov 2010 11:18:29 -0500 Received: from d01av01.pok.ibm.com (loopback [127.0.0.1]) by d01av01.pok.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id oAIGISNp017813 for ; Thu, 18 Nov 2010 11:18:29 -0500 Message-ID: <4CE551D4.1090202@linux.vnet.ibm.com> Date: Thu, 18 Nov 2010 10:18:28 -0600 From: Michael Roth MIME-Version: 1.0 Subject: Re: [Qemu-devel] [RFC][PATCH v3 11/21] virtproxy: add handler for control packet References: <1289870175-14880-1-git-send-email-mdroth@linux.vnet.ibm.com> <1289870175-14880-12-git-send-email-mdroth@linux.vnet.ibm.com> <4CE50F89.6040007@redhat.com> In-Reply-To: <4CE50F89.6040007@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jes Sorensen Cc: agl@linux.vnet.ibm.com, abeekhof@redhat.com, qemu-devel@nongnu.org, aliguori@linux.vnet.ibm.com, ryanh@us.ibm.com, amit.shah@redhat.com On 11/18/2010 05:35 AM, Jes Sorensen wrote: > On 11/16/10 02:16, Michael Roth wrote: >> Process control packets coming in over the channel. This entails setting >> up/tearing down connections to local services initiated from the other >> end of the channel. >> >> Signed-off-by: Michael Roth >> --- >> virtproxy.c | 154 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 files changed, 154 insertions(+), 0 deletions(-) > > [snip] > >> + >> + qemu_opts_print(iforward->socket_opts, NULL); >> + if (qemu_opt_get(iforward->socket_opts, "host") != NULL) { >> + server_fd = inet_connect_opts(iforward->socket_opts); >> + } else if (qemu_opt_get(iforward->socket_opts, "path") != NULL) { >> + server_fd = unix_connect_opts(iforward->socket_opts); >> + } else { >> + LOG("unable to find listening socket host/addr info"); >> + return -1; >> + } > > This patch is a perfect example of why -1 as an error message is suboptimal. > >> + closesocket(fd); >> + vp_set_fd_handler(fd, NULL, NULL, conn); >> + QLIST_REMOVE(conn, next); >> + qemu_free(conn); >> + break; >> + } >> + } > > You should never have two closing braces in the same column like this - > something is wrong with the formatting. That's from using a block for the case switch () { case: { ... } } Alternative is to indent the "case:", which is "right" I think, but aligning those with switch() seems to be pretty standard to conserve space. > > Cheers, > Jes