From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56431) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YUe3q-0006kY-Ph for qemu-devel@nongnu.org; Sun, 08 Mar 2015 12:28:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YUe3n-0008WR-JM for qemu-devel@nongnu.org; Sun, 08 Mar 2015 12:28:14 -0400 Received: from e23smtp08.au.ibm.com ([202.81.31.141]:33494) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YUe3m-0008WJ-RW for qemu-devel@nongnu.org; Sun, 08 Mar 2015 12:28:11 -0400 Received: from /spool/local by e23smtp08.au.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 9 Mar 2015 02:28:07 +1000 Received: from d23relay09.au.ibm.com (d23relay09.au.ibm.com [9.185.63.181]) by d23dlp02.au.ibm.com (Postfix) with ESMTP id 86BAE2BB0040 for ; Mon, 9 Mar 2015 03:28:04 +1100 (EST) Received: from d23av02.au.ibm.com (d23av02.au.ibm.com [9.190.235.138]) by d23relay09.au.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t28GRtQE17498282 for ; Mon, 9 Mar 2015 03:28:04 +1100 Received: from d23av02.au.ibm.com (localhost [127.0.0.1]) by d23av02.au.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t28GRU2C003604 for ; Mon, 9 Mar 2015 03:27:30 +1100 From: "Aneesh Kumar K.V" In-Reply-To: <54FB775D.6080608@msgid.tls.msk.ru> References: <54FB6199.6030306@redhat.com> <54FB775D.6080608@msgid.tls.msk.ru> Date: Sun, 08 Mar 2015 21:57:11 +0530 Message-ID: <877fur4fq8.fsf@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Tokarev , Eric Blake Cc: qemu-devel@nongnu.org Michael Tokarev writes: > 07.03.2015 23:37, Eric Blake wrote: >> On 03/06/2015 02:43 PM, Michael Tokarev wrote: >>> All filesystem methods that call common v9fs_request() function >>> also convert return value to errno. Move this conversion to the >>> common function and remove redundand error handling in methods. >> >> s/redundand/redundant/ > > Heh. Is this all that I can say about this patch? ;) > > Actually, after reading almost whole 9pfs and fsdev code, I can > say with great confidence this code is nearly hopeless. Is that about the 9pfs-proxy code, or the rest of 9pfs. I understand that the error handling can definitely get some cleanup. I mentioned that in my previous mail mail. http://mid.gmane.org/87oav7iy5v.fsf@linux.vnet.ibm.com > Patch 3 shows just one (huge) example. There are so many issues > with this code, I'm afraid I don't have know the words to express > it. > > Again, patch 3 shows a good example. Another example: > > static int v9fs_receive_status(V9fsProxy *proxy, > struct iovec *reply, int *status) > ... > proxy_unmarshal(reply, 0, "dd", &header.type, &header.size); > if (header.size != sizeof(int)) { > *status = -ENOBUFS; > } > ... > proxy_unmarshal(reply, PROXY_HDR_SZ, "d", status); > > proxy_unmarshall(), for "d" element, expects an int32_t > pointer. Here we have int pointer, and compare its > size with sizeof(int). This is a generic problem of whole > v9fs_(un)marshall interface, which is in the core of 9pfs... > this is a status return, which is int32. > > Oh well. I've no idea how this code has been accepted. > It is absolute crap. > -aneesh