We should eliminate the NS_COMFALSE madness once and for all. Here's a list of offending uses in your module -- please pass the bug along if there's someone else who should deal with it. mailnews/addrbook/src/nsAddrDatabase.cpp: View change log or Blame annotations line 255 line 2193 mailnews/base/search/src/nsMsgLocalSearch.cpp: View change log or Blame annotations line 486 line 520 line 593 line 600 mailnews/base/search/src/nsMsgSearchTerm.cpp: View change log or Blame annotations line 579 line 589 line 648 line 680 line 741 line 853 line 898 line 948 line 988 line 1008 line 1034 mailnews/base/src/nsMsgFolderCache.cpp: View change log or Blame annotations line 316 mailnews/db/msgdb/src/nsMsgDatabase.cpp: View change log or Blame annotations line 105 line 1529 line 1654 line 1835 line 1895 mailnews/db/msgdb/src/nsMsgThread.cpp: View change log or Blame annotations line 717

reassigning to bienvenu who owns most of these file and cc'ing chuang who owns one of these files.

woa, I need a little background. What should I replace it with? Did RDF stop caring about NS_COMFALSE? I don't want to blindly replace it with NS_OK or NS_FAILURE and break everything.

We should replace it with NS_OK/NS_ERROR_FAILURE. You'll need to coordinate with Waterson to change it. This is one of Brendan's architecture cleanup items.

Chris, any thoughts about this?

Without looking at specifics... At least some of our uses of NS_COMFALSE are functions that really just want to return PRBool. The 'right' way to fix them is to do: (in xpidl) PRBool Foo(); which becomes in C++ NS_IMETHOD Foo(PRBool *_result); So in calling it you do: PRBool ok; if(SUCCEEDED(bar->Foo(&ok)) && ok) // yada yada This is, of course, more work than using NS_COMFALSE. But with NS_COMFALSE we have the "NS_OK == PR_FALSE and NS_COMFALSE == PR_TRUE" problem. And also the problem that both of those return values pass the NS_SUCCEEDED(rv) test that we all love so much.

That's fine for my internal routines which I can fix right now, but when dealing with rdf enumerators, I don't have that kind of flexibility

jband, if you want to avoid warren's fate (losing in fisticuffs to waterson), you might rather write the exemplar this way: if (NS_FAILED(rv)) deal with failure, probably by early return; if (ok) boolean result was true; else boolean result was false; Conjoining NS_SUCCEEDED(rv) && ok may hide errors along with false-no-op cases. bienvenu, when I asked waterson about how his hoped-for conversion from nsIEnumerator to nsISimpleEnumerator might proceed, he came back with this thought: add boolean HasMoreElements(); and nsISupports GetNext(); methods to nsIEnumerator and its impl, incrementally convert from the First/Next/IsDone methods to these, then rename or equate typenames so nsIEnumerator becomes nsISimpleEnumerator. Does this sound like a plan? I'm cc'ing rjc in addition to waterson for RDF help. I'll seek comments in other fora from editor, widget, and other folks who currently use nsIEnumerator and not nsISimpleEnumerator. /be

Sounds like a good plan. I'm not real familiar with the db code that uses NS_COMFALSE, since Warren wrote it :-) but I'll work with Chris to clean it up.

fixed in almost every file in mailnews.

I noticed there are still uses of NS_COMFALSE in the address book backend, so I'm reopening and reassigning

Clearing Fixed resolution due to reopen.

Done on address book, will check in if tree open.

Done

Based on chuang's comment's, I mark this fixed in the Sept 2nd build.