OpenSSL and DarwinSSL w/ suggestions from Steve re: style.

Comments welcome.

-bch

<< patch attached >>


On 3/8/15, bch <[email protected]> wrote:
> On 3/8/15, Steve Holme <[email protected]> wrote:
>> On Fri, 6 Mar 2015, bch wrote:
>>
>>> Is the inline patch I posted above acceptable, or should it be
>>> re-submitted as a collection of patches or something different ?
>>
>> I'm personally a Windows man so I would like attachments if possible -
>> trying to apply an inline git patch is a nightmare (just doesn't work due
>> to
>> line endings and Outlook / Hotmail messing about with the formatting!)
>>
>> I know others here prefer inline so I would always recommend sending in
>> both
>> ;-)
>
> very fair...
>
>> From my brief review I would recommend the following naming as they are
>> more
>> in keepingwith the existing SSL informational items:
>>
>>> +  CURLINFO_NEGOTIATED_SSL   = CURLINFO_LONG + 44,
>>
>> CURLINFO_SSL_NEGOTIATED_VERSION
>
> Useful...
>
>> ...and for consistency:
>>
>>> +  info->negotiated_ssl = -1L;
>
> ditto
>
>> info->ssl_negotiated_version
>>
>> A couple of small nits:
>>
>> * I would recommend removing such lines as "/* bch ref -- NEGOTIATED_SSL
>> info here (?) */"
>
> This wouldn't make it into anything I'd submit -- it's note-taking
> for myself if the approach I put forward was deemed reasonable.
>
>> * Open braces after a function declaration should be on the following
>> line
>> not as per conditional code like you appear to have done with
>> "set_ssl_version_long(SSL *ssl, struct connectdata *conn) {"
>
> Noted...
>
>> * I would also recommend that the return type be on the beginning rather
>> than on a separate line - I appreciate we have a mix of this at present
>> :(
>
> Noted...
>
>
> If the approach in general looks sane, I'll make the formatting
> adjustments and start fleshing out other SSL engines as well. Thanks
> for the feedback.
>
>> Kind Regards
>>
>> Steve
>
> Cheers,
>
> -bch
>

Attachment: negotiated.patch
Description: Binary data

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

Reply via email to