found by coverity this can be fixed with bug 333304. and it's possible i've already filed a bug about the signature problem since i recall changing it in some tree. the signature problem is this: if something is an interface reachable method, which RegisterComponent is, then it should be NS_IMETHODIMP. if it's private, it can be nsresult or whatever it's declared to be. This method is declared in idl and therefore it should be properly tagged. which among other things allows one to clearly recognize that this boundary is not being properly handled.

There was one [notxpcom] function which I didn't quite understand, so I didn't change the nsresult for it: http://landfill.mozilla.org/mxr-test/mozilla/source/xpcom/components/nsIComponentManagerObsolete.idl#77

Comment on attachment 217778 [details] [diff] [review] fix signatures and prevent the crash using NS_ENSURE_ARG_POINTER since this used to crash in that case anyway, this sounds reasonable to me.

timeless suggested to use NS_IMETHODIMP for the notxpcom function too (because xpidl defines it as NS_IMETHOD in the generated header)

Comment on attachment 217791 [details] [diff] [review] fix signatures (+1 more) and prevent the crash using NS_ENSURE_ARG_POINTER Hmm, the fact that our calling convention for nsIComponentManagerObsolete has been wrong for so long, doesn't that suggest that we could just remove it? Or, is this code trying to emmulate an older calling convention?

NS_IMETHODIMP on the implementation doesn't make any difference, as long as the function is declared correctly: the declaration controls the calling convention.

Comment on attachment 217791 [details] [diff] [review] fix signatures (+1 more) and prevent the crash using NS_ENSURE_ARG_POINTER good, sr=darin

Checking in xpcom/components/nsComponentManager.cpp; /cvsroot/mozilla/xpcom/components/nsComponentManager.cpp,v <-- nsComponentMana ger.cpp new revision: 1.278; previous revision: 1.277 done

For reference, this patch accidently included a part from bug 333303.