[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling
From: |
Michael Tokarev |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling |
Date: |
Tue, 10 Mar 2015 07:23:30 +0300 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Icedove/31.4.0 |
08.03.2015 19:27, Aneesh Kumar K.V wrote:
> Michael Tokarev <address@hidden> writes:
[]
>> 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
Well. the marshal/unmarshal interface is in core code as far as
I can see, and it is very fragile at best, as the below example of
its usage shows. I haven't dug deeper. So far, it was only the
9pfs proxy code.
> that the error handling can definitely get some cleanup. I mentioned
> that in my previous mail
> mail. http://mid.gmane.org/address@hidden
I've shown probs in the code itself, not the visible behavour.
Visible behavour is much easier to fix here.
Thanks,
/mjt
>> 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
>
- [Qemu-devel] [PATCH v3 0/3] RFC: 9pfs-proxy: simplify/cleanup, Michael Tokarev, 2015/03/06
- [Qemu-devel] [PATCH 2/3] fsdev: introduce v9fs_vmarshal() and v9fs_vunmarshal(), Michael Tokarev, 2015/03/06
- [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling, Michael Tokarev, 2015/03/06
- Re: [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling, Eric Blake, 2015/03/07
- Re: [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling, Michael Tokarev, 2015/03/07
- Re: [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling, Michael Tokarev, 2015/03/07
- Re: [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling, Aneesh Kumar K.V, 2015/03/08
- Re: [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling,
Michael Tokarev <=
- Re: [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling, Aneesh Kumar K.V, 2015/03/10
- Re: [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling, Michael Tokarev, 2015/03/11
- Re: [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling, Aneesh Kumar K.V, 2015/03/11
- Re: [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling, Michael Tokarev, 2015/03/11
- Re: [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling, Michael Tokarev, 2015/03/11
Re: [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling, Aneesh Kumar K.V, 2015/03/08
[Qemu-devel] [PATCH 3/3] 9pfs-proxy: remove one half of redundrand code, Michael Tokarev, 2015/03/06
Re: [Qemu-devel] [PATCH v3 0/3] RFC: 9pfs-proxy: simplify/cleanup, Michael Tokarev, 2015/03/10