WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
151881
TestNetscapePlugIn: Fix leaks found by static analyzer
https://bugs.webkit.org/show_bug.cgi?id=151881
Summary
TestNetscapePlugIn: Fix leaks found by static analyzer
David Kilzer (:ddkilzer)
Reported
2015-12-04 13:21:07 PST
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.
Attachments
Patch v1
(4.91 KB, patch)
2015-12-04 13:24 PST
,
David Kilzer (:ddkilzer)
no flags
Details
Formatted Diff
Diff
Patch v2
(4.62 KB, patch)
2015-12-04 15:34 PST
,
David Kilzer (:ddkilzer)
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
David Kilzer (:ddkilzer)
Comment 1
2015-12-04 13:24:10 PST
Created
attachment 266649
[details]
Patch v1
Anders Carlsson
Comment 2
2015-12-04 13:30:30 PST
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).
David Kilzer (:ddkilzer)
Comment 3
2015-12-04 14:08:38 PST
(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.
Anders Carlsson
Comment 4
2015-12-04 14:10:39 PST
Comment on
attachment 266649
[details]
Patch v1 then just use std::unique_ptr<char[]>(new char[string.UTF8Length + 1]);
David Kilzer (:ddkilzer)
Comment 5
2015-12-04 15:34:41 PST
Created
attachment 266669
[details]
Patch v2
Darin Adler
Comment 6
2015-12-06 18:40:27 PST
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.
WebKit Commit Bot
Comment 7
2015-12-06 19:27:49 PST
Comment on
attachment 266669
[details]
Patch v2 Clearing flags on attachment: 266669 Committed
r193607
: <
http://trac.webkit.org/changeset/193607
>
WebKit Commit Bot
Comment 8
2015-12-06 19:27:54 PST
All reviewed patches have been landed. Closing bug.
David Kilzer (:ddkilzer)
Comment 9
2015-12-07 05:50:34 PST
(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
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug