I'm going to make a general audit of mailnews code (I've started such attempts before and just never finshed) to update the mailnews code to use the NS_WITH_SERVICE macro instead of: nsServiceManager::Get/ReleaseService. This will help improve stability and fix various ref counting bugs we may have where people are forgetting to release services when they were done or forgetting to release the service in error cases. It improves stability by simplifying the code, and making it hard to not correctly release the service when we're done with it.

Setting TFV M5 per our discussion this morning.

I'll take care of: mozilla/mailnews/news/* mozilla/mailnews/db/msgdb/src/nsNewsDatabase.cpp mozilla/mailnews/base/util/nsNewsSet.cpp mozilla/mailnews/base/util/base/util/nsNewsSummarySpec.cpp

I'll take care of mailnews/base/src/* and then mailnews/base/util/*

By the way: If a do_QueryInterface fails (returning a null pointer) please return NS_ERROR_NO_INTERFACE

guys, I'm actually doing all of mailnews and am almost done. I skipped the news changes because Seth already told me he did those in his tree. Apologies if this is a problem. (P.s. This bug is only NS_WITH_SERVICE. We need to set up another one for using com ptrs. The comment about returning NS_ERROR_NO_INTERFACE should probably be duplicated in that bug)

I've got the NS_WITH_SERVICE changes in my tree (for the news .cpp files) waiting to be checked in.

We're not going to take these changes for M5 so I'm going to reset for M6. Just to re-iterate outside of the news stuff which Seth has covered, I've completed an audit of all of mailnews. So no one should have to tweak the get service stuff. I'll check it in as soon as the tree opens for checkins.

I've finished this code audit.

The following files continue to have GetService and ReleaseService within the same function in atleast one function: D:\mozilla\mailnews\addrbook\build\nsAbFactory.cpp(222,265): D:\mozilla\mailnews\base\build\nsMsgFactory.cpp(344): D:\mozilla\mailnews\base\src\nsMessenger.cpp(249): D:\mozilla\mailnews\compose\build\nsMsgCompFactory.cpp(227): D:\mozilla\mailnews\db\msgdb\build\nsMsgDBFactory.cpp(216): D:\mozilla\mailnews\imap\build\nsImapFactory.cpp(209): D:\mozilla\mailnews\imap\tests\harness\imapProtocolTest.cpp(540): D:\mozilla\mailnews\local\build\nsMsgLocalFactory.cpp(257): D:\mozilla\mailnews\local\tests\mailbox\mailboxTest.cpp(315): D:\mozilla\mailnews\mime\cthandlers\calendar\nsCalendarFactory.cpp(198): D:\mozilla\mailnews\mime\cthandlers\vcard\nsVCardFactory.cpp(198): D:\mozilla\mailnews\mime\emitters\html\nsEmitterFactory.cpp(199): D:\mozilla\mailnews\mime\emitters\raw\nsEmitterFactory.cpp(199): D:\mozilla\mailnews\mime\emitters\xml\nsEmitterFactory.cpp(199): D:\mozilla\mailnews\mime\src\nsMimeFactory.cpp(228): D:\mozilla\mailnews\news\build\nsMsgNewsFactory.cpp(311): Reopen and keep priority at P3 Par

I just fixed D:\mozilla\mailnews\news\build\nsMsgNewsFactory.cpp Using NS_WITH_SERVICE allowed me to remove a whole bunch of gotos and the done: label. dystra (spelling) would be pleased.

Dijkstra: "Goto considered harmful" http://www.acm.org/classics/oct95/

I believe it's dijkstra =) Checked in fixes for: nsAbFactory.cpp nsMsgFactory.cpp nsMessenger.cpp nsMsgCompFactory.cpp nsMsgDBFactory.cpp nsImapFactory.cpp imapProtocolTest.cpp nsMsgLocalFactory.cpp nsCalendarFactory.cpp nsVCardFactory.cpp html\nsEmitterFactory.cpp raw\nsEmitterFactory.cpp nsMimeFactory.cpp xml\nsEmitterFactory.cpp These changes with Seth's changes should close out the bug again. Marking as fixed.

changing target milestonen to M7 since the final fixes are in M7.

My scan still shows the following files have not been converted yet: D:\mozilla\mailnews\base\src\nsMessenger.cpp: D:\mozilla\mailnews\local\tests\mailbox\mailboxTest.cpp: Please check again

Yup I did miss two places. Good catch. I checked in fixes for those 2 files.

All Clear