On 3/26/2015 6:50 AM, Kyle L. Huff wrote:
>> If a NO_FILESYSTEM really does require a CTX function (just my guess) then I
>> would do it differently. What I would do is combine your two patches into
>> one and redo it so you get rid of the CyaSSL_no_filesystem_verify in the
>> #else block and replace it with something like this:
>>
>> #ifndef NO_FILESYSTEM
>> existing stuff is here
>> #else
>> if(!data->set.ssl.fsslctx) {
>> failf(data,"SSL: CyaSSL no-filesystem requires
>> CURLOPT_SSL_CTX_FUNCTION");
>> return CURLE_SSL_CONNECT_ERROR;
>> }
>> #endif
> That seems to make the context callback functionality only available
> when using NO_FILESYSTEM. I imagine that there are other legitimate
> uses for a context callback, even when a file system is present.
> Additionally, as mentioned above, using NO_FILESYSTEM without a
> context callback is valid.

Well no that isn't what it does, it requires the callback if
NO_FILESYSTEM and is optional otherwise. Anyway the if statement is
wrong because I didn't check peer verify. I was basically suggesting you
refactor. Look at [1] to see what I mean.

> As for combining the patches, I can do that if you like. I only kept
> them separate as, I would consider the first to be a bug-fix, and the
> second to be a feature implementation.
>
> Please let me know your thoughts. I will provide patches (or a single
> patch, please confirm) for whatever changes are deemed necessary.

There is a feature freeze until the next release. Depends on if the
maintainers consider allowing the CTX callback in CyaSSL a bugfix for
the no-filesystem builds or a feature. If it's the latter it may be
better to leave the patches separate though I would put the
documentation changes in with the CTX enabling. Hopefully someone can
weigh in here.

[1]: https://github.com/jay/curl/commit/4e835eb

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html
Received on 2015-03-26