On 11/17/2013 10:45 PM, Steve Holme wrote: > On Sun, 17 Nov 2013, Christian Grothoff wrote: > >> I personally think (1) is cleaner. I've attached a patch that implements >> your version (1) against current Git. > > I may have misunderstood my quick read of the thread but I just wanted to > chirp up as I am wanting to add the something similar at some point in the > future - the ability to return a list of server side capabilities for the > email protocols (as reported by the CAPABILITY, CAPA and EHLO commands for > IMAP, POP3 and SMTP respectively). > > Wouldn't option 2 be better as a) it is more in keeping with the existing > API where the application owns the memory and 2) the application can then > destroy it at its own leisure - potentially isn't there a scenario where > libcurl destroys the connection and the application still has a pointer to > the tlsinfo address that has now been destroyed?
Right now, the tlsinfo pointer's destination would be freed (on curl_easy_cleanup), in the other case, the TLS session information reachable via the application-owned tlsinfo memory would be freed (on curl_easy_cleanup). Thus, in both cases doing anything with that memory will lead to disaster. > Admittedly it may be bad > programming on the part of the application, as you would hope, they would > not keep the pointer lying about after the curl easy handle has been > destroyed but I still think this is potentially dangerous (Obviously the > internal SSL handle / context is still there and shoudn't be used). Yes, it is unsafe to use this information after curl_easy_cleanup; but there is no good way to make it safe. > At present I'm not sure whether I would use a dedicated "capability" > structure for the information I want to get hold of or the existing slist - > but if it is the former then I would like to follow suit (for consistency) > and if it is the latter then the application owns the memory and has to call > curl_slist_free_all() when it is done - a practice that libcurl programmers > are familiar with ;-) Note that the information returned is not an slist. > I'm not too sure if this is a benefit or not, but if the memory is owned by > the application (and as this structure is pretty lightweight) it also allows > the application to use either stack or heap based memory as well. > > Additionally, is the SSL backend type needed in this structure or should we > consider having it separate? I left is separate, as otherwise we need to include the header of the SSL backend in curl.h, which would be ugly given that there are many different backends. > Texturally that is already contained within the version information - from > curl_version() - isn't it? I appreciate it may be useful for the application > to obtain this without parsing the text from curl_version() but surely this > isn't specific to a TLS session and perhaps a CURLINFO_SSL_BACKEND option > for curl_easy_getinfo() would be better. That way the function can return a > simple long containing a backend specifier and CURLINFO_TLS_SESSION could > return a void* which would be the handle / context used in the session? The > downside of this is that it requires two calls to curl_easy_getinfo() for > the same info - but it gives the application the ability to obtain one > without the other. You are assuming that there will always be a 1:1 correspondence between SSL backends and returned void *-types. However, it might be that at some point we decide to return a different type depending on say the OpenSSL version in use (say OpeenSSL v2 offers a completely different kind of session handle). Then, with the enum, we can indicate this change easily by defining a second enum value for OpenSSL2, while possibly still returning the same name for the backend in curl_version and (if it is added) CURLINFO_SSL_BACKEND. I also don't like having another option just to get the backend, but that's a lesser reason for me. Happy hacking Christian ------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.html
