[webkit-reviews] review granted: [Bug 174002] [Curl] Safe access and life cycle management of bare Curl handle by wrapping with C++ class : [Attachment 314865] implemented

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has granted Basuke Suzuki
<Basuke.Suzuki at sony.com>'s request for review:
Bug 174002: [Curl] Safe access and life cycle management of bare Curl handle by
wrapping with C++ class
https://bugs.webkit.org/show_bug.cgi?id=174002

Attachment 314865: implemented

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




--- Comment #3 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 314865
  --> https://bugs.webkit.org/attachment.cgi?id=314865
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.