Summary: | TestNetscapePlugIn: Fix leaks found by static analyzer | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
Component: | Tools / Tests | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | andersca, commit-queue, darin, joepeck, lforschler | ||||||
Priority: | P2 | ||||||||
Version: | Other | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
David Kilzer (:ddkilzer)
2015-12-04 13:21:07 PST
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> |