cURL / Mailing Lists / curl-library / Single Mail

curl-library

Re: [PATCH] cyassl: remove undefined reference to CyaSSL_no_filesystem_verify & add support for CURLOPT_SSL_CTX_FUNCTION

From: Kyle L. Huff <kyle.huff_at_curetheitch.com>
Date: Fri, 27 Mar 2015 07:41:48 -0400

On Thu, Mar 26, 2015 at 9:18 PM, Ray Satiro via curl-library
<curl-library_at_cool.haxx.se> wrote:
> On 3/26/2015 8:16 PM, Kyle L. Huff wrote:
>>
>> On Thu, Mar 26, 2015 at 2:46 PM, Ray Satiro via curl-library
>> <curl-library_at_cool.haxx.se> wrote:
> Nope it's just that, a refactor. The result should be the same.

Right, I get that. My point was that while they both have the same
result, the execution is different (and possibly more burdensome?).

My concern was with the additional check of `data->set.ssl.fsslctx`,
which, I don't know what kind of impact (if any) on execution that has
when byte-compiled, but my intent was to reach the result in the
fewest run-time switches possible (by offloading the logical switch to
a compile time `#ifdef` statement that enables an `else`). My
thinking is, using NO_FILESYSTEM may be due to embedded or low
resource hardware.

> As I said I
> would do it differently and that's my feedback. I am a contributor here you
> don't have to do what I would do. You wanted feedback, you got it :)

Well, hey now, don't marginalize your role! You may only be a
contributor, but having eyes on patches is crucial, and not everyone
is available or has comments. I greatly appreciate your feedback.

> I'm
> sure it will be accepted your way but as you mentioned you will need a
> failf. Also please review the coding guidelines [1]. The if statement should
> be indented 2-level (you have 3 for the else if) and there shouldn't be a
> space between the if and left parenthesis. In other words " else
> if(data->set.ssl.verifypeer) {".

Good catch. Fixed in the attached patches.

-------------------------------------------------------------------
List admin: http://cool.haxx.se/list/listinfo/curl-library
Etiquette: http://curl.haxx.se/mail/etiquette.html

Received on 2015-03-27