cURL / Mailing Lists / curl-library / Single Mail

curl-library

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

From: Daniel Stenberg <daniel_at_haxx.se>
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

Received on 2013-02-17