qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] Re: [RFC][PATCH v5 04/21] virtagent: transport definiti


From: Jes Sorensen
Subject: Re: [Qemu-devel] Re: [RFC][PATCH v5 04/21] virtagent: transport definitions and job callbacks
Date: Wed, 08 Dec 2010 20:16:31 +0100
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.12) Gecko/20101103 Fedora/1.0-0.33.b2pre.fc14 Lightning/1.0b3pre Thunderbird/3.1.6

On 12/07/10 18:19, Michael Roth wrote:
> On 12/07/2010 07:44 AM, Jes Sorensen wrote:
>>> +static int va_end_of_header(char *buf, int end_pos)
>>> +{
>>> +    return !strncmp(buf+(end_pos-2), "\n\r\n", 3);
>>> +}
>>
>> Maybe I am missing something here, but it looks like you do a strncmp to
>> a char that is one past the end of the buffer, or? If this is
>> intentional, please document it.
>>
> 
> buf+end_pos points to the last char we read (rather than being an offset
> to the current position). So it stops comparing when it reaches
> buf+end_pos (buf=0 + end_pos=2 implies 3 characters)
> 
> For some reason this confused the hell out of me when I looked over it
> again as well. Alternatively I can do:
> 
> static int va_end_of_header(char *buf, int end_pos)
> {
>     return !strncmp(buf+(end_pos-2), "\n\r\n", 3);
> }
> ...
> va_end_of_header(s->hdr, s->hdr_pos - 1)
> 
> ->
> 
> static int va_end_of_header(char *buf, int cur_pos)
> {
>     return !strncmp(buf+(cur_pos-3), "\n\r\n", 3);
> }
> ...
> va_end_of_header(s->hdr, s->hdr_pos);
> 
> It does seem easier to parse...

I would prefer this, somewhat easier to parse.

>> All this http parsing code leaves the question open why you do it
>> manually, instead of relying on a library? 
> Something like libcurl? At some point we didn't attempt to use libraries
> provide by xmlrpc-c (which uses libcurl for http transport) for the
> client and server. The problem there is that libcurl really wants and
> tcp socket read and write from, whereas we need to support tcp/unix
> sockets on the host side and isa/virtio serial ports on the guest side.
> 
> Even assuming we could hook in wrappers for these other types of
> sockets/channels, there's also the added complexity since dropping
> virtproxy of multiplexing HTTP/RPCs using a single stream, whereas
> something like libcurl would, understandably, assume it has a dedicated
> stream to read/write from. So we wouldn't really save any work or code,
> unfortunately.

I guess I am just a little worried that we end up with errors in the
code that could have been solved by using a maintainer http library, but
if it isn't feasible I guess not.

Cheers,
Jes





reply via email to

[Prev in Thread] Current Thread [Next in Thread]