Last Comment Bug 531716 - crash [@ nsAbMDBCard::Equals(nsIAbCard*, int*)]
: crash [@ nsAbMDBCard::Equals(nsIAbCard*, int*)]
: crash
Product: MailNews Core
Classification: Components
Component: Address Book (show other bugs)
: 1.9.1 Branch
: x86 Windows XP
-- critical (vote)
: Thunderbird 18.0
Assigned To: :aceman
Depends on:
  Show dependency treegraph
Reported: 2009-11-29 18:25 PST by Wayne Mery (:wsmwk, NI for questions)
Modified: 2012-08-31 17:15 PDT (History)
4 users (show)
ryanvm: in‑testsuite-
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

patch (963 bytes, patch)
2012-08-30 09:26 PDT, :aceman
mconley: review+
Details | Diff | Splinter Review

Description User image Wayne Mery (:wsmwk, NI for questions) 2009-11-29 18:25:45 PST
crash [@ nsAbMDBCard::Equals(nsIAbCard*, int*)]
exactly one crash in 1 year

bp-b5857ac0-9702-4184-8c75-088642091124 v3.0b4
0  	thunderbird.exe  	nsAbMDBCard::Equals  	 mailnews/addrbook/src/nsAbMDBCard.cpp:72
1 	thunderbird.exe 	nsAbView::FindIndexForCard 	mailnews/addrbook/src/nsAbView.cpp:969
2 	thunderbird.exe 	nsAbView::OnItemPropertyChanged 	mailnews/addrbook/src/nsAbView.cpp:985
3 	thunderbird.exe 	nsAbManager::NotifyItemPropertyChanged 	mailnews/addrbook/src/nsAbManager.cpp:372
4 	thunderbird.exe 	nsAbMDBDirectory::NotifyItemChanged 	mailnews/addrbook/src/nsAbMDBDirectory.cpp:250
5 	thunderbird.exe 	nsAbMDBDirectory::OnCardEntryChange 	mailnews/addrbook/src/nsAbMDBDirectory.cpp:872
6 	thunderbird.exe 	nsAddrDatabase::NotifyCardEntryChange 	mailnews/addrbook/src/nsAddrDatabase.cpp:244
7 	thunderbird.exe 	nsAddrDatabase::EditCard 	mailnews/addrbook/src/nsAddrDatabase.cpp:1945
8 	thunderbird.exe 	nsAbMDBDirectory::ModifyCard 	mailnews/addrbook/src/nsAbMDBDirectory.cpp:727
9 	thunderbird.exe 	nsMsgCompose::CheckAndPopulateRecipients 	mailnews/compose/src/nsMsgCompose.cpp:4727
Comment 1 User image Wayne Mery (:wsmwk, NI for questions) 2011-12-03 10:09:47 PST
no idea why I filed this, but it still happens.
bp-433fb681-a96c-47ea-a191-dc7172110825 version 6
Comment 2 User image :aceman 2012-08-30 06:58:52 PDT
A null check on 'card' could help here :)

Comment 3 User image :aceman 2012-08-30 09:26:00 PDT
Created attachment 656908 [details] [diff] [review]
Comment 4 User image Kent James (:rkent) 2012-08-30 13:03:53 PDT
 NS_IMETHODIMP nsAbMDBCard::Equals(nsIAbCard *card, bool *result)

You could return false if card is null - a null card does not equal a real card.
Comment 5 User image :aceman 2012-08-30 13:14:19 PDT
Yes, I thought about that, but maybe it is better to tell the developer he sends garbage, with an error?

That is a semantic decision so let's wait for mconley.
Comment 6 User image Mike Conley (:mconley) 2012-08-31 07:53:23 PDT
(In reply to :aceman from comment #5)
> Yes, I thought about that, but maybe it is better to tell the developer he
> sends garbage, with an error?
> That is a semantic decision so let's wait for mconley.

Hm. So the point of this function is to compare two nsIAbCard's for equality. A nullptr (nee nsnull) can be assigned to an nsIAbCard pointer, but is definitely not an nsIAbCard.

So I think I'd rather the caller ensure that the card they're passing is not nsnull before checking equality. So I'm going to side with aceman on this.
Comment 7 User image Mike Conley (:mconley) 2012-08-31 07:54:36 PDT
Comment on attachment 656908 [details] [diff] [review]

Review of attachment 656908 [details] [diff] [review]:

Looks good. Thanks aceman!
Comment 8 User image Ryan VanderMeulen [:RyanVM] 2012-08-31 17:15:08 PDT

Note You need to log in before you can comment on or make changes to this bug.