Re: Peer review! SecureTransport (native SSL on iOS/OS X) patch
Date: Tue, 26 Jun 2012 12:17:32 -0600
On Jun 26, 2012, at 6:40 AM, Marc Hoersken wrote:
> Hello everyone,
> Nick, thanks for the great work on this. It's great to see native
> Windows and Mac support now.
> I would like to propose the following cleanup changes:
> - Rename st_ function prefix to darwinssl_
I'm okay with this. (See patch below.)
> - Rename Curl_st_ function prefix to Curl_darwinssl_
> - Move the duplicated ssl_connect_done out of the #ifdef in lib/urldata.h
Ah… I wasn't sure if that typedef was used only internally or not, and decided to play it safe. It does look like it's used only internally, though, so I made the change. SecureTransport, unlike OpenSSL, has only one would-block "error" condition, meaning it doesn't give the program any clue whether it was reading or writing, which is why I had to add a new state just to get non-blocking I/O working correctly.
> - In st_connect_step1 replace multiple calls to
> SSLSetProtocolVersionEnabled with an internal variable and just one
> call to SSLSetProtocolVersionEnabled.
That is unfortunately a necessary evil. I would have implemented this using a bit-mask if Apple had given me a way to do that, but they didn't, so we have to call it multiple times, first to turn off everything, and then again to turn on the ones we want to enable.
> I could create a patch for these changes, but since I am unable to
> test it myself and you may still be working on it, I went with this
In addition to implementing your suggestions, I also fixed a teensy little bug that made non-blocking connection attempts block, and made it so that it builds cleanly against the iOS 5.1 SDK. Daniel or Yang, please push this patch. Tack.
- application/octet-stream attachment: darwinssl.patch