Closed
Bug 504087
Opened 15 years ago
Closed 15 years ago
NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(IMETextTxn) incorrectly used NS_INTERFACE_MAP_END_INHERITING(EditTxn)
Categories
(Core :: DOM: Editor, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: timeless, Assigned: timeless)
References
(Blocks 1 open bug)
Details
(Keywords: coverity)
Attachments
(1 file, 1 obsolete file)
1.04 KB,
patch
|
neil
:
review+
|
Details | Diff | Splinter Review |
So, trying to understand Coverity reports can be painful. In order to explain what's going on here, i've marked each macro expansion with its own letter, the b218..b219 is the definition and expansion of NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION, and the line before it, a67, is the line in the code which used that macro. The problem is that: d538 defines foundInterface. i205 sets it then the else if from i206+a68 is skipped because it's an else, which takes us to a72, the next line is k576 which nulls out foundInterface. Coverity complains that upon reaching k578, the effort taken in i205 was wasted because it was unconditionally cleared by k576. /*a67 NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(IMETextTxn) */ /*b218 #define NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(_class) */ /*b219 NS_INTERFACE_MAP_BEGIN(_class) */ /*c615 #define NS_INTERFACE_MAP_BEGIN(_implClass) */ /*c NS_IMPL_QUERY_HEAD(_implClass) */ /*d533 #define NS_IMPL_QUERY_HEAD(_class) */ /*d534 */ NS_IMETHODIMP _class::QueryInterface(REFNSIID aIID, void** aInstancePtr) /*d535 */ { /*d536 */ NS_ASSERTION(aInstancePtr, /*d537 */ "QueryInterface requires a non-NULL destination!"); /*d538 */ nsISupports* foundInterface; /*b220 NS_INTERFACE_MAP_ENTRIES_CYCLE_COLLECTION(_class) */ /*e214 #define NS_INTERFACE_MAP_ENTRIES_CYCLE_COLLECTION(_class) */ /*e215 NS_INTERFACE_MAP_ENTRY_CYCLE_COLLECTION(_class) */ /*f208 #define NS_INTERFACE_MAP_ENTRY_CYCLE_COLLECTION(_class) */ /*f209 NS_IMPL_QUERY_CYCLE_COLLECTION(_class) */ /*g197 #define NS_IMPL_QUERY_CYCLE_COLLECTION(_class) */ /*g198 */ if ( aIID.Equals(NS_GET_IID(nsXPCOMCycleCollectionParticipant)) ) { /*g199 */ *aInstancePtr = & NS_CYCLE_COLLECTION_NAME(_class); /*g200 */ return NS_OK; /*g201 */ } else /*e216 NS_INTERFACE_MAP_ENTRY_CYCLE_COLLECTION_ISUPPORTS(_class) */ /*h211 #define NS_INTERFACE_MAP_ENTRY_CYCLE_COLLECTION_ISUPPORTS(_class) */ /*h212 NS_IMPL_QUERY_CYCLE_COLLECTION_ISUPPORTS(_class) */ /*i203 #define NS_IMPL_QUERY_CYCLE_COLLECTION_ISUPPORTS(_class) */ /*i204 */ if ( aIID.Equals(NS_GET_IID(nsCycleCollectionISupports)) ) /*i205 */ foundInterface = NS_CYCLE_COLLECTION_CLASSNAME(_class)::Upcast(this); /*i206 */ else /*a68 */ if (aIID.Equals(IMETextTxn::GetCID())) { /*a69 */ *aInstancePtr = (void*)(IMETextTxn*)this; /*a70 */ NS_ADDREF_THIS(); /*a71 */ return NS_OK; /*a72 */ } /*a73 NS_INTERFACE_MAP_END_INHERITING(EditTxn) */ /*j625 #define NS_INTERFACE_MAP_END_INHERITING(_baseClass) */ /*j626 NS_IMPL_QUERY_TAIL_INHERITING(_baseClass) */ /*k575 #define NS_IMPL_QUERY_TAIL_INHERITING(_baseclass) */ /*k576 */ foundInterface = 0; /*k577 */ nsresult status; /*k578 */ if ( !foundInterface ) /*k579 */ status = _baseclass::QueryInterface(aIID, (void**)&foundInterface); /*k580 */ else /*k581 */ { /*k582 */ NS_ADDREF(foundInterface); /*k583 */ status = NS_OK; /*k584 */ } /*k585 */ *aInstancePtr = foundInterface; /*k586 */ return status; /*k587 */ }
Attachment #388477 -
Flags: review?(neil)
Updated•15 years ago
|
Attachment #388477 -
Flags: review?(neil) → review+
Attachment #388477 -
Attachment is obsolete: true
Attachment #388483 -
Flags: review?(neil)
Updated•15 years ago
|
Attachment #388483 -
Flags: review?(neil) → review+
Blocks: 488799
Did you want to fix EditAggregateTxn as well?
Er, never mind... I guess I removed that bit before I landed the original patch, since it was unused.
Er, actually, I removed it when I landed bug 489851, which was after I posted the patch to the bug but before I landed it; I think I remember having a little fun merging that.
http://hg.mozilla.org/mozilla-central/rev/40bc10840e6d
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•6 years ago
|
Blocks: coverity-analysis
You need to log in
before you can comment on or make changes to this bug.
Description
•