Fix the following leaks found by the static analyzer in the TestNetscapePlugIn project: DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:808:16: warning: Potential leak of memory pointed to by 'path' return false; ^~~~~ DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:808:16: warning: Potential leak of memory pointed to by 'target' return false; ^~~~~ DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:808:16: warning: Potential leak of memory pointed to by 'url' return false; ^~~~~ The toCString() method returns malloc()ed memory, and the early returns in testPostURLFile() don't call free() on them.
Created attachment 266649 [details] Patch v1
Comment on attachment 266649 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=266649&action=review > Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:790 > +typedef std::unique_ptr<char, void (*)(void*)> CStringBuffer; > +static CStringBuffer toCString(const NPString& string) > { > - char* result = static_cast<char*>(malloc(string.UTF8Length + 1)); > - memcpy(result, string.UTF8Characters, string.UTF8Length); > - result[string.UTF8Length] = '\0'; > + size_t length = string.UTF8Length; > + char* buffer = static_cast<char*>(malloc(length + 1)); > + if (!buffer) > + return CStringBuffer(nullptr, free); > > - return result; > + memcpy(buffer, string.UTF8Characters, length); > + buffer[length] = '\0'; > + > + return CStringBuffer(buffer, free); I don't think there's a need for this to use free, can just use std::unique_ptr<char[]> (and std::make_unique).
(In reply to comment #2) > Comment on attachment 266649 [details] > Patch v1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=266649&action=review > > > Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:790 > > +typedef std::unique_ptr<char, void (*)(void*)> CStringBuffer; > > +static CStringBuffer toCString(const NPString& string) > > { > > - char* result = static_cast<char*>(malloc(string.UTF8Length + 1)); > > - memcpy(result, string.UTF8Characters, string.UTF8Length); > > - result[string.UTF8Length] = '\0'; > > + size_t length = string.UTF8Length; > > + char* buffer = static_cast<char*>(malloc(length + 1)); > > + if (!buffer) > > + return CStringBuffer(nullptr, free); > > > > - return result; > > + memcpy(buffer, string.UTF8Characters, length); > > + buffer[length] = '\0'; > > + > > + return CStringBuffer(buffer, free); > > I don't think there's a need for this to use free, can just use > std::unique_ptr<char[]> (and std::make_unique). The only downside here is that std::make_unique<> requires <WTF/StdLibExtras.h>, and based on the project sources, I don't think TestNetscapePlugin should depend on WTF.
Comment on attachment 266649 [details] Patch v1 then just use std::unique_ptr<char[]>(new char[string.UTF8Length + 1]);
Created attachment 266669 [details] Patch v2
Comment on attachment 266669 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=266669&action=review > Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:977 > + std::unique_ptr<char[]> message(nullptr); No need for the explicit nullptr here.
Comment on attachment 266669 [details] Patch v2 Clearing flags on attachment: 266669 Committed r193607: <http://trac.webkit.org/changeset/193607>
All reviewed patches have been landed. Closing bug.
(In reply to comment #6) > Comment on attachment 266669 [details] > Patch v2 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=266669&action=review > > > Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:977 > > + std::unique_ptr<char[]> message(nullptr); > > No need for the explicit nullptr here. Fixed in r193623: <http://trac.webkit.org/r193623>