On Fri, 19 Mar 2010, Kamil Dudka wrote:
I think it would work. The only problem is we need to rewrite all the SSL backends at one time. The proposed patch touches only the backend(s) which really benefits from it. To be honest, I am not even able to test all of them afterwards. Nevertheless if you are ready for such a change, I can prepare the patch.
I think that's the nicer approach, even if it means slightly more work and testing. I can rather easily test with all the three major SSL libs, so it's not that painful IMHO.
At first glance I don't see any advantage in adding the argument over adding a field into struct ssl_connect_data. We will still have two place to look for the error, but need to rewrite all the SSL backends anyhow.
My biggest concern with adding the field to the struct is that we grow the struct for a very specific and narrow use-case that will be used very rarely. I know we have the same kind of thing on numerous places, but I still would like to see this kind of constructs in the code reduced rather than increased. It isn't only the binary size of the struct that is trouble-some, but the shear amount of fields within the struct that at times need to be understood and parsed by human eyes.
Putting it as an extra argument will add a local argument on the stack but will keep the main struct untouched.
-- / daniel.haxx.se ------------------------------------------------------------------- List admin: http://cool.haxx.se/list/listinfo/curl-library Etiquette: http://curl.haxx.se/mail/etiquette.html
