cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: [PATCH] Addition of trailer headers in HTTP requests generated by libcurl

From: Chrysovaladis Datsios <cdatsios_at_gmail.com>
Date: Mon, 11 Mar 2013 18:09:29 +0200

Hi,
changes are in the attached patch.

> Where does the magic number 3 come from?
I've changed it to 2. One endofline_native for each trailer header and
another at the end.

>
> I'm not sure CURLE_OUT_OF_MEMORY is the
> best error code here--the system is technically not out of memory. Maybe
> CURLE_BAD_FUNCTION_ARGUMENT?
I'm returning CURLE_BAD_FUNCTION_ARGUMENT.

> This will leak trailer_headers_buf.
Made some changes so as not to leak.

> These headers have to be sent through the CURL_DOES_CONVERSIONS code if
> enabled, which means you want endofline_native here instead.
Ok.

> I don't think there is any code that frees this pointer anywhere, which means
> it will leak.
>
>> + curl_slist_free_all(list_head);
I set pointer to NULL also.

>
> I'm not sure it's the best to free this list here. The caller may want to
> keep it around, and it can always free it if it wants.
The trailer headers list is a local variable and I don't think I
should let the user free it when he wants.

> Why not just increase the value of nread? Adding trlen everywhere complicates
> the code, and makes it ease to forget to add it. Such as in the
> CURL_DOES_CONVERSIONS section (which breaks because it was forgotten).
I removed trlen and now I update nread.

I also moved the flag upper:
/* mark this as done once this chunk is transferred */
data->req.upload_done = TRUE;
because after the insertion of trailer headers the statement
"if((nread - hexlen) == 0)" wont be true anymore.(nread has changed)

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html

Received on 2013-03-11