WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
34705
[BREWMP] Add a function to create a BREW instance without local variable declarations.
https://bugs.webkit.org/show_bug.cgi?id=34705
Summary
[BREWMP] Add a function to create a BREW instance without local variable decl...
Kwang Yul Seo
Reported
2010-02-08 08:02:50 PST
It is too verbose to create a BREW instance. For example, to create a IFileMgr instance, it takes three lines: IFileMgr* fileMgr; IShell* shell = reinterpret_cast<AEEApplet*>(GETAPPINSTANCE())->m_pIShell; ISHELL_CreateInstance(shell, AEECLSID_FILEMGR, reinterpret_cast<void**>(&fileMgr)); This pattern is repeated all over the code, so we need a shortcut here. Add a template function to create a BREW instance in one line. IFileMgr* fileMgr = createInstanceBrew<IFileMgr>(AEECLSID_FILEMGR); This can be nicely combined with OwnPtrBrew. OwnPtrBrew<IFileMgr> fileMgr(createInstanceBrew<IFileMgr>(AEECLSID_FILEMGR));
Attachments
Patch
(3.05 KB, patch)
2010-02-08 08:08 PST
,
Kwang Yul Seo
no flags
Details
Formatted Diff
Diff
Patch
(3.03 KB, patch)
2010-02-08 09:34 PST
,
Kwang Yul Seo
eric
: review-
Details
Formatted Diff
Diff
Patch
(3.03 KB, patch)
2010-03-07 01:34 PST
,
Kwang Yul Seo
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Kwang Yul Seo
Comment 1
2010-02-08 08:08:31 PST
Created
attachment 48333
[details]
Patch ShellUtilBrew.h is currently under wtf/brew, but I am not sure if this is the right place to put platform-specific utility functions.
Darin Adler
Comment 2
2010-02-08 09:13:06 PST
(In reply to
comment #1
)
> ShellUtilBrew.h is currently under wtf/brew, but I am not sure if this is the > right place to put platform-specific utility functions.
It’s the right directory for low-level functions of this type. But “utility” is not a word we ever use, and we certainly frown on “Util” in a file name.
Kwang Yul Seo
Comment 3
2010-02-08 09:34:35 PST
Created
attachment 48338
[details]
Patch Remove Util from the filename and prefix the function with shell createInstanceBrew -> shellCreateInstanceBrew
Eric Seidel (no email)
Comment 4
2010-02-08 19:20:10 PST
Should this return a PassOwnPtrBrew?
Kwang Yul Seo
Comment 5
2010-02-08 19:46:50 PST
(In reply to
comment #4
)
> Should this return a PassOwnPtrBrew?
These is no PassOwnPtrBrew class. I just wanted to simplify the creation of a BREW instance. As OwnPtrBrew takes a raw pointer in the constructor, we can use an idiom like the following: OwnPtrBrew<IFileMgr> fileMgr(createInstanceBrew<IFileMgr>(AEECLSID_FILEMGR)); Do you suggest that this should return a PassOwnPtrBrew?
Adam Barth
Comment 6
2010-02-09 12:40:40 PST
> Do you suggest that this should return a PassOwnPtrBrew?
In general, createMumble methods return a PassFooBar object to ensure we don't leak the allocated memory.
Eric Seidel (no email)
Comment 7
2010-02-17 15:54:47 PST
Comment on
attachment 48338
[details]
Patch Why would this be part of WTF? This seems like port-specific goop. We don't need to have PassOwnPtrBrew to approve this, but eventually it would be nice to have such a class to make leaking memory returned from this function impossible.
Kwang Yul Seo
Comment 8
2010-03-07 01:34:28 PST
Created
attachment 50169
[details]
Patch Make the function return PassOwnPtr. This prevents memory leaking.
Eric Seidel (no email)
Comment 9
2010-03-15 16:21:30 PDT
Comment on
attachment 50169
[details]
Patch LGTM.
WebKit Commit Bot
Comment 10
2010-03-15 21:07:06 PDT
Comment on
attachment 50169
[details]
Patch Clearing flags on attachment: 50169 Committed
r56031
: <
http://trac.webkit.org/changeset/56031
>
WebKit Commit Bot
Comment 11
2010-03-15 21:07:13 PDT
All reviewed patches have been landed. Closing bug.
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