Re: [Qemu-devel] [PATCH 1/3] 9pfs-proxy: simplify error handling

[Michael Tokarev <mjt@tls.msk.ru>]

08.03.2015 19:27, Aneesh Kumar K.V wrote:
> Michael Tokarev <mjt@tls.msk.ru> 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/87oav7iy5v.fsf@linux.vnet.ibm.com

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
>