Re: [PATCH] Addition of trailer headers in HTTP requests generated by libcurl
Date: Mon, 11 Mar 2013 18:09:29 +0200
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
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.
> 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)
- application/octet-stream attachment: 0003-CURLOPT_TRAILERFUNCTION-support-chunked-encoding-req.patch