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

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

On 12/03/10 19:03, Michael Roth wrote:
> +static void va_server_read_cb(const char *content, size_t content_len)
> +{
> +    xmlrpc_mem_block *resp_xml;
> +    VAServerData *server_data = &va_state->server_data;
> +    int ret;
> +
> +    TRACE("called");
> +    resp_xml = xmlrpc_registry_process_call(&server_data->env,
> +                                            server_data->registry,
> +                                            NULL, content, content_len);
> +    if (resp_xml == NULL) {
> +        LOG("error processing RPC request");
> +        goto out_bad;
> +    }
> +
> +    ret = va_server_job_add(resp_xml);
> +    if (ret != 0) {
> +        LOG("error adding server job: %s", strerror(ret));
> +    }
> +
> +    return;
> +out_bad:
> +    /* TODO: should reset state here */
> +    return;

Looks like some missing error handling is needed here?

> +static void va_rpc_parse_hdr(VAHTState *s)
> +{
> +    int i, line_pos = 0;
> +    bool first_line = true;
> +    char line_buf[4096];

In 03/21 you defined VA_HDR_LEN_MAX to 4096, here you hard code the
value .... sounds like something begging to go wrong.

> +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.

All this http parsing code leaves the question open why you do it
manually, instead of relying on a library?

Cheers,
Jes