curl-library
Re: Getting and setting cookie list from structs instead of netscape formated strings
Date: Sun, 28 Mar 2010 22:41:02 +0200 (CEST)
On Sun, 28 Mar 2010, Claes Jakobsson wrote:
> This gist, http://gist.github.com/346813, is a first patch for it - needs
> some fixing tho and testing.
Great, thanks for working on this! I'm generally in favour of adding this
feature, and I have some feedback on the current patch (in a somewhat random
order):
1. The multi-debugcallback.c change seems unrelated.
2. The bool type should be around by including 'setup.h' which cookie.c
already does. We cannot unconditionally use stdbool.h for that, as that's a C99ism.
3. Regarding the two new public functions I think we need some docs for them
too, to fully be able to evaluate and understand them. For example, the
curl_cookie struct can point to a lot of data, how is that data allocated
and freed? Perhaps libcurl should strdup() the strings passed to it within
the struct?
4. Related to (3), we need some test cases added to verify this new
functionality.
5. There's an $Id$ line reintroduced in getinfo.c that you can skip.
6. Regarding the struct curl_sslist definition. It seems a bit strange to
me to have 'type' in each struct and not in a top-level "header struct"
since surely all nodes within the linked list will have the same type?
Maybe it should work more like the 'struct curl_certinfo' instead and have
some information in a top struct, which itself points to the actual list.
Even for the case when an application creates the data and passes it to
libcurl it makes little sense with a 'type' in each struct.
7. The CURLINFO_SSLIST type isn't necessary. The existing CURLINFO_SLIST is
already (ab)used for generic struct data pointer returns so it would be
better to just create an alias like CURLINFO_STRUCTPOINT and use instead.
-- / daniel.haxx.se ------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.htmlReceived on 2010-03-28