Closed Bug 504087 Opened 11 years ago Closed 11 years ago

NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION(IMETextTxn) incorrectly used NS_INTERFACE_MAP_END_INHERITING(EditTxn)

Categories

(Core :: DOM: Editor, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: timeless, Assigned: timeless)

References

(Blocks 1 open bug)

Details

(Keywords: coverity)

Attachments

(1 file, 1 obsolete file)

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 */
}
Attached patch add the missing else (obsolete) — Splinter Review
Attachment #388477 - Flags: review?(neil)
Attachment #388477 - Flags: review?(neil) → review+
Attachment #388477 - Attachment is obsolete: true
Attachment #388483 - Flags: review?(neil)
Attachment #388483 - Flags: review?(neil) → review+
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: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.