crash [@ nsAbMDBCard::Equals(nsIAbCard*, int*)]
exactly one crash in 1 year
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
no idea why I filed this, but it still happens.
bp-433fb681-a96c-47ea-a191-dc7172110825 version 6
A null check on 'card' could help here :)
Created attachment 656908 [details] [diff] [review]
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.
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.
(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 on attachment 656908 [details] [diff] [review]
Review of attachment 656908 [details] [diff] [review]:
Looks good. Thanks aceman!