[webkit-reviews] review granted: [Bug 28171] [cURL] Support https protocol in cURL builds : [Attachment 34603] Revised to improve efficiency and thread safety.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Aug 11 15:26:37 PDT 2009

Adam Roben (aroben) <aroben at apple.com> has granted Brent Fulgham
<bfulgham at webkit.org>'s request for review:
Bug 28171: [cURL] Support https protocol in cURL builds

Attachment 34603: Revised to improve efficiency and thread safety.

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
> +static CString certificateBundlePath()

"certificateBundlePath" is a little confusing. This doesn't return a path to a
"certificate bundle"; it returns a path to a certificate within the WebKit
bundle. I think "certificatePath" would be clearer.

> +{
> +    CFBundleRef webKitBundle =
> +    RetainPtr<CFURLRef> certURLRef(AdoptCF,
CFBundleCopyResourceURL(webKitBundle, CFSTR("cacert"), CFSTR("pem"),
> +    if (certURLRef) {
> +	   char path[MAX_PATH];
> +	   CFURLGetFileSystemRepresentation(certURLRef.get(), false,
(UInt8*)path, MAX_PATH);

reinterpret_cast is preferred to a C-style cast.

> +    , m_certPath (certificateBundlePath())

You have an extra space after "m_certPath".

> +    if (!m_certPath.isNull())
> +	  curl_easy_setopt(d->m_handle, CURLOPT_CAINFO, m_certPath.data());

Checking isEmpty() seems a bit better than checking isNull().

> +    const CString m_certPath;

m_certificatePath seems a little clearer. I don't think the abbreviation adds
anything here.


More information about the webkit-reviews mailing list