[webkit-reviews] review denied: [Bug 33943] Use fastStrDup instead of strdup : [Attachment 47107] Use fastStrDup instead of strdup
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Thu Jan 21 10:01:13 PST 2010
Darin Adler <darin at apple.com> has denied Kwang Yul Seo
<kwangyul.seo at gmail.com>'s request for review:
Bug 33943: Use fastStrDup instead of strdup
https://bugs.webkit.org/show_bug.cgi?id=33943
Attachment 47107: Use fastStrDup instead of strdup
https://bugs.webkit.org/attachment.cgi?id=47107&action=review
------- Additional Comments from Darin Adler <darin at apple.com>
> Index: WebCore/bridge/jni/JNIBridge.cpp
> ===================================================================
> --- WebCore/bridge/jni/JNIBridge.cpp (revision 53619)
> +++ WebCore/bridge/jni/JNIBridge.cpp (working copy)
> @@ -301,7 +301,7 @@ JavaMethod::JavaMethod(JNIEnv* env, jobj
> JavaMethod::~JavaMethod()
> {
> if (m_signature)
> - free(m_signature);
> + fastFree(m_signature);
> delete[] m_parameters;
> };
We could change this to be an OwnPtr now. Also, there is never a need for a
null check on a fastFree call.
> @@ -311,7 +311,7 @@ static void appendClassName(StringBuilde
> {
> ASSERT(JSLock::lockCount() > 0);
>
> - char* c = strdup(className);
> + char* c = fastStrDup(className);
Unfortunate for performance that this is always on the heap. It would be better
if this used Vector<char, 64> or something like that.
> JavaArray::~JavaArray()
> {
> - free(const_cast<char*>(m_type));
> + fastFree(const_cast<char*>(m_type));
> }
We could change this to be an OwnPtr now.
> @@ -409,7 +409,7 @@ void JavaArray::setValueAt(ExecState* ex
> // The type of the array will be something like:
> // "[Ljava.lang.string;". This is guaranteed, so no need
> // for extra sanity checks.
> - javaClassName = strdup(&m_type[2]);
> + javaClassName = fastStrDup(&m_type[2]);
> javaClassName[strchr(javaClassName, ';')-javaClassName] = 0;
> }
> jvalue aJValue = convertValueToJValue(exec, aValue, arrayType,
javaClassName);
> @@ -472,7 +472,7 @@ void JavaArray::setValueAt(ExecState* ex
> }
>
> if (javaClassName)
> - free(const_cast<char*>(javaClassName));
> + fastFree(const_cast<char*>(javaClassName));
> }
We could change this to be an OwnPtr now.
> JavaClass::~JavaClass()
> {
> - free(const_cast<char*>(m_name));
> + fastFree(const_cast<char*>(m_name));
We could change this to be an OwnPtr now.
> - free(m_url);
> + fastFree(m_url);
We could change this to be an OwnPtr now.
> Index: WebCore/platform/network/curl/ResourceHandleManager.cpp
> ===================================================================
> --- WebCore/platform/network/curl/ResourceHandleManager.cpp (revision
53619)
> +++ WebCore/platform/network/curl/ResourceHandleManager.cpp (working copy)
> @@ -135,13 +135,13 @@ ResourceHandleManager::~ResourceHandleMa
> curl_multi_cleanup(m_curlMultiHandle);
> curl_share_cleanup(m_curlShareHandle);
> if (m_cookieJarFileName)
> - free(m_cookieJarFileName);
> + fastFree(m_cookieJarFileName);
> curl_global_cleanup();
> }
We could change this to be an OwnPtr now.
> // url is in ASCII so latin1() will only convert it to char* without
character translation.
> - d->m_url = strdup(url.latin1().data());
> + d->m_url = fastStrDup(url.latin1().data());
> curl_easy_setopt(d->m_handle, CURLOPT_URL, d->m_url);
I don't see the change to the free call here. Is this introducing a mismatched
malloc/free pair?
review- because the try bots say this breaks the build
More information about the webkit-reviews
mailing list