Bug 174002 – [Curl] Safe access and life cycle management of bare Curl handle by wrapping with C++ class

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Basuke Suzuki]

libcurl is a low-end C API library which is powerful, but easy to make a mistake and hard to find the source of issue. Wrapping by C++ class makes the code confident and accelerate the speed of refactoring, especially for upcoming NetworkLoad implementation.

[Basuke Suzuki]

Created attachment 314865 [details]
implemented

[Basuke Suzuki]

This is the first step to make libcurl native resources handling into wrapped class. More refactoring is applying by upcoming patches for other bugs.

[Alex Christensen]

Comment on attachment 314865 [details]
implemented

View in context: https://bugs.webkit.org/attachment.cgi?id=314865&action=review

Let's land this and do more improvements on top of this.  The curl code was a big mess and is getting much better.  Thanks!

> Source/WebCore/platform/network/curl/CookieJarCurl.cpp:271
> +    struct curl_slist* list = curlHandle.getCookieList();

auto* list

> Source/WebCore/platform/network/curl/CurlContext.cpp:334
> +        fastFree(m_url);

Let's use a MallocPtr or std::unique_ptr<char[]> for m_url instead of having to do manual memory management here.

> Source/WebCore/platform/network/curl/CurlContext.cpp:665
> +    // The size of a curl_off_t could be different in WebKit and in cURL depending on
> +    // compilation flags of both.

This doesn't seem good.  Maybe we should get rid of this and require consistent compilation flags.

> Source/WebCore/platform/network/curl/CurlContext.cpp:684
> +#ifndef NDEBUG
> +
> +void CurlHandle::enableVerboseIfUsed()
> +{
> +    if (CurlContext::singleton().isVerbose())
> +        curl_easy_setopt(m_handle, CURLOPT_VERBOSE, 1);
> +}

We could have the functions exist and do nothing in release builds to reduce the number of #ifndef checks.

> Source/WebCore/platform/network/curl/ResourceHandleManager.h:29
>  #ifndef ResourceHandleManager_h
>  #define ResourceHandleManager_h

#pragma once.

[WebKit Commit Bot]

Comment on attachment 314865 [details]
implemented

Clearing flags on attachment: 314865

Committed r219279: <http://trac.webkit.org/changeset/219279>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Basuke Suzuki]

Thanks Alex. This makes much safer for us to improve our code base.