On 3/27/2015 7:41 AM, Kyle L. Huff wrote:
> 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.

Your patches looked fine but I had some minor nits and submitted altered
patches to the list earlier today without realizing they were already
applied. I have since separated those changes and submitted them
separately as a pull request [1].

As to why I suggested you do it working off the existing NO_FILESYSTEM
check, I am in the habit of grouping and fail early for less complexity.
For example consider

#ifndef NO_FILESYSTEM
if verify then load certs
#else
// bad code removed
if verify and no ctx then notify user certs can't be loaded because no
filesystem.
#endif

rather than come in later. This is not an optimization issue as far as
I'm concerned, I think it makes the code easier to understand. Both
sides are related to cert loading (even though one can't). Let the
compiler worry about optimization. I was taught a great saying when I
learned C, don't lie to your compiler. There's no lying here but I
mention that because every time I think about trying to outsmart to
prevent branching or something I always remember that and come to my
senses. Well, almost always :)

> 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.

Thanks and thanks for the changes.

[1]: https://github.com/bagder/curl/pull/185

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