[webkit-reviews] review granted: [Bug 182979] [Curl] Split DNS cache expiration and connection timeout setting. : [Attachment 334768] FIX

[bugzilla-daemon at webkit.org]

Per Arne Vollan <pvollan at apple.com> has granted Basuke Suzuki
<Basuke.Suzuki at sony.com>'s request for review:
Bug 182979: [Curl] Split DNS cache expiration and connection timeout setting.
https://bugs.webkit.org/show_bug.cgi?id=182979

Attachment 334768: FIX

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




--- Comment #16 from Per Arne Vollan <pvollan at apple.com> ---
Comment on attachment 334768
  --> https://bugs.webkit.org/attachment.cgi?id=334768
FIX

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

R=me.

> Source/WebCore/platform/network/curl/CurlContext.cpp:67
> +class EnvironmentVariableReader {
> +public:
> +    template<typename T> std::optional<T> readAs(const char* name)
> +    {
> +	   if (const char* valueStr = ::getenv(name)) {
> +	       T value;
> +	       if (sscanf(valueStr, sscanTemplate<T>(), &value) == 1)
> +		   return value;
> +	   }
> +
> +	   return std::nullopt;
> +    }
> +
> +private:
> +    template<typename T> const char* sscanTemplate()
> +    {
> +	   ASSERT_NOT_REACHED();
> +	   return nullptr;
> +    }
> +
> +    template<> const char* sscanTemplate<unsigned>() { return "%u"; }
> +};
> +

This class could also be put in a header file for reuse. Perhaps we also could
consider using a namespace instead?

> Source/WebCore/platform/network/curl/CurlContext.cpp:483
> +    unsigned value = time >= 0.0 ? static_cast<unsigned>(time) : 0;

For the sake of clarity, I think we should use parentheses here: (time >= 0.0)