[webkit-reviews] review denied: [Bug 181256] Enable -Wcast-qual for WebInspectorUI, WebKitLegacy, WebKit projects : [Attachment 330629] Patch v1

[bugzilla-daemon at webkit.org]

David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has denied  review:
Bug 181256: Enable -Wcast-qual for WebInspectorUI, WebKitLegacy, WebKit
projects
https://bugs.webkit.org/show_bug.cgi?id=181256

Attachment 330629: Patch v1

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




--- Comment #4 from David Kilzer (:ddkilzer) <ddkilzer at webkit.org> ---
Comment on attachment 330629
  --> https://bugs.webkit.org/attachment.cgi?id=330629
Patch v1

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

>> Source/WebKit/Platform/cocoa/WKCrashReporter.mm:48
>>	    free(oldMessage);
> 
> Not caused by your changed, but this code raises multiple red flags:
> - How do we know we can free this data, anyone outside of WebKit could have
called CRSetCrashLogMessage with their own string.
> - Why would we free the oldMessage here, but not care to free any potential
value in the earlier case we overwrite the value (line 41-42).

Yep, you're correct!  As currently written this would leak the previous message
if a NULL CFStringRef were passed in.

The code to free the old message should happen before the (!infoString) check.