[webkit-reviews] review denied: [Bug 204618] [GTK][WebInspector] Support for saving remote data : [Attachment 384351] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has denied Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 204618: [GTK][WebInspector] Support for saving remote data
https://bugs.webkit.org/show_bug.cgi?id=204618

Attachment 384351: Patch

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




--- Comment #2 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 384351
  --> https://bugs.webkit.org/attachment.cgi?id=384351
Patch

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

> Source/WebKit/UIProcess/gtk/RemoteWebInspectorProxyGtk.cpp:127
> +	   data = out.data();

You can't do this, out is going to be freed after the if, so data will point to
freed data.

> Source/WebKit/UIProcess/gtk/RemoteWebInspectorProxyGtk.cpp:130
> +	   data = content.utf8().data();

More ore less the same here, because the CString returned by utf8() is a
temporary.

> Source/WebKit/UIProcess/gtk/RemoteWebInspectorProxyGtk.cpp:131
> +	   dataLength = content.utf8().length();

And we don't want to convert to utf8 twice either.

> Source/WebKit/UIProcess/gtk/RemoteWebInspectorProxyGtk.cpp:137
> +    if (g_file_set_contents(path.get(), data, dataLength, nullptr))
> +	  
m_inspectorPage->process().send(Messages::RemoteWebInspectorUI::DidSave(path.ge
t()), m_inspectorPage->webPageID());

You can use g_file_replace_contents_async(), instead of doing this
synchronously.