curl-library
Re: [PATCH] Addition of trailer headers in HTTP requests generated by libcurl
Date: Sun, 17 Feb 2013 23:19:03 +0100 (CET)
Hiya,
Thanks for the patch. Now we're getting somewhere. This version is much better
than previous versions!
I do have a few questions and tips:
0 - you didn't really follow our code style when it comes to indenting. A
minor nit, but still...
1 - Can you think of a good reason why the callback would need to get the
struct curl_slist * as its second argument? Won't that always just be
NULL?
2 - since the trailer headers now are set by a return code from a callback,
I don't think they should be part of the 'struct UserDefined' anymore.
In fact, is there a point to have it stored within the SessionHandle
struct at all?
3 - the memcpy() functions invoked after the callback has returned its list
of headers are not checking that the destination is big enough. It
uses the data set by the callback which risk overflowing the upload
buffer...
4 - I asked you before, but why is_trailerheaders_set? You can just as well
save 5 lines of code and some memory by checking 'trailerheaders_func'
instead!
5 - you modified a file in src/ without purpose.
See attached file for my cleaned up version of your patch.
-- / daniel.haxx.se
-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
- TEXT/x-diff attachment: 0001-CURLOPT_TRAILERFUNCTION-support-chunked-encoding-req.patch