Closed Bug 122463 Opened 23 years ago Closed 23 years ago

xptiInterfaceInfo.cpp: Returning NS_OK when a failure occurs

Categories

(Core :: XPCOM, defect)

x86
Windows NT
defect
Not set
normal

Tracking

()

VERIFIED DUPLICATE of bug 121118

People

(Reporter: morse, Assigned: dbradley)

Details

This is an offshoot of bug 113519

The following code in xptiInterfaceInfo.cpp::GetEntryForParam is returning NS_OK 
when an error has occurred.

    NS_ASSERTION(theEntry, "bad state");
    *entry = theEntry;
    return NS_OK;

So even though we know that theEntry is incorrect (null), we go ahead and assign 
it to entry and return an NS_OK status.  The caller, GetIIDForParamNoAlloc, 
immediately dereferences entry and will crash.

The fix is to modify the above to read

    NS_ASSERTION(theEntry, "bad state");
    if (!theEntry) {
      return NS_ERROR_FAILURE;
    }
    *entry = theEntry;
    return NS_OK;

Assigning to XPCOM but I'm not sure whether or not that's the correct component.
Dissent.  Don't paper over whatever bug is being tickled by your cookie-IDL
patch, find out what it is and fix it right (assuming it's not just corrupt or
otherwise mis-built typelibs), at the source.  This routine gets called, I would
guess, hundreds of thousands of times during a typical Mozilla run, and yet we
have never needed that check before!

Unless we can see that there are otherwise-correct operations of the libraries
in question that result in a null pointer coming through here, I don't think we
want this sort of "bullet-proofing".
But why was that unexpected condition occurring?  Is there something incoherent
in your build?  What happens if you remove xpti.dat?

/be
Assignee: dougt → dbradley
Removing xpti.dat makes no difference -- it simply gets rebuilt again and the 
same assertion/crash occurs.
This is a dup. Waiting on review.

If you are hitting that problem then you are trying to *use* an interface info
that does not exist. It would be good to know what info that is and what code is
trying to use it. You may well need to fully declare an interface that is only
forward declared or ship an xpt file that is not being shipped.

The only reason *I* ran into the problem was that I wrote an xpt decompiler that
iterated over all the interfaces and all their param types. But if you are seing
this in normal usage then there is another problem.

*** This bug has been marked as a duplicate of 12118 ***
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → DUPLICATE
reopenning to set this as a dup of the *correct* bug.
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
dropped a digit on the bug number...

*** This bug has been marked as a duplicate of 121118 ***
Status: REOPENED → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → DUPLICATE
vrfy dupe
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.