Closed
Bug 450777
Opened 16 years ago
Closed 16 years ago
outparamdel nsNodeInfoManager::GetNodeInfo
Categories
(Developer Infrastructure :: Source Code Analysis, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: taras.mozilla, Assigned: taras.mozilla)
References
Details
Attachments
(1 file, 5 obsolete files)
52.14 KB,
patch
|
jst
:
review+
jst
:
superreview+
|
Details | Diff | Splinter Review |
patch coming soon
Assignee | ||
Comment 1•16 years ago
|
||
I'd like to land one of these patches. I've cleaned up the generated code a bit. Do you have any more suggestions as to what needs to be better(other than indentation) before this can land?
Assignee | ||
Comment 2•16 years ago
|
||
Would be landing with this as part of the bigger patch
Assignee | ||
Comment 3•16 years ago
|
||
sorry, forgot to qrefresh
Attachment #334031 -
Attachment is obsolete: true
Comment 4•16 years ago
|
||
Comment on attachment 334028 [details] [diff] [review] automagical patch + rv = nodeInfo ? NS_OK : NS_ERROR_FAILURE; NS_ENSURE_SUCCESS(rv, rv); It'd be nice if this could be changed to NS_ENSURE_TRUE(nodeInfo, NS_ERROR_FAILURE), and extra bonus points for automatically fixing up indentation too! :) r+sr=jst!
Attachment #334028 -
Flags: superreview+
Attachment #334028 -
Flags: review?(jst)
Attachment #334028 -
Flags: review+
Assignee | ||
Comment 5•16 years ago
|
||
Jim, I need your emacs powers!
Assignee | ||
Comment 6•16 years ago
|
||
another automatic rev
Attachment #334028 -
Attachment is obsolete: true
Assignee | ||
Comment 7•16 years ago
|
||
I'm frustrated with the outparamdelled getter nsNodeInfoManager::GetNodeInfo() as I don't know how to make xpcom not leak in that case.
Attachment #334033 -
Attachment is obsolete: true
Attachment #338196 -
Attachment is obsolete: true
Attachment #338207 -
Flags: review?(jst)
Comment 8•16 years ago
|
||
(In reply to comment #7) > Created an attachment (id=338207) [details] > full patch(auto + manual) > > I'm frustrated with the outparamdelled getter nsNodeInfoManager::GetNodeInfo() > as I don't know how to make xpcom not leak in that case. I think we'll inevitably need to make this return an already_AddRefed<nsINodeInfo> and have the places that use raw pointers with this return value use .get() on the return, i.e. nsINodeInfo *foo = nsNodeInfoManager::GetNodeInfo(...).get(), or something along those lines.
Assignee | ||
Comment 9•16 years ago
|
||
so the logic here if someone called the getter with getter_AddRefs(), then it becomes foo = getter() in other cases the form is foo = getter().get() I think this gets correct behavior for cases like nsNodeInfoManager::GetTextNodeInfo().
Attachment #338207 -
Attachment is obsolete: true
Attachment #338370 -
Flags: review?(jst)
Attachment #338207 -
Flags: review?(jst)
Updated•16 years ago
|
Attachment #338370 -
Flags: superreview+
Attachment #338370 -
Flags: review?(jst)
Attachment #338370 -
Flags: review+
Comment 10•16 years ago
|
||
Comment on attachment 338370 [details] [diff] [review] alreadyAddRefed patch Looks good! r+sr=jst
Assignee | ||
Comment 11•16 years ago
|
||
pushed http://hg.mozilla.org/mozilla-central/file/ec4733eb1bd8
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Comment 12•16 years ago
|
||
Comment on attachment 338370 [details] [diff] [review] alreadyAddRefed patch >+ NS_ENSURE_TRUE(newNodeInfo, NS_ERROR_FAILURE); > NS_ASSERTION(newNodeInfo, "GetNodeInfo lies"); This makes no sense...
Updated•7 years ago
|
Product: Core → Firefox Build System
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•