Re: [Qemu-devel] Re: [RFC][PATCH v5 04/21] virtagent: transport definitions and job callbacks

[Jes Sorensen <Jes.Sorensen@redhat.com>]

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