The default bug view has changed. See this FAQ.

Port bug 1193762 to Mailnews (bustage fix)

RESOLVED FIXED in Thunderbird 49.0

Status

MailNews Core
Database
RESOLVED FIXED
11 months ago
11 months ago

People

(Reporter: Jorg K (GMT+1), Assigned: Frank-Rainer Grahl)

Tracking

Trunk
Thunderbird 49.0

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

11 months ago
Now that bug 1193762 has landed, we have fresh bustage, for example:

c:/builds/moz2_slave/tb-c-cen-w32-ntly-000000000000/build/mailnews/db/msgdb/src/nsMsgDatabase.cpp(4765): error C2662: 'nsCOMPtr<nsIMsgThread>::operator T(void) const &&': cannot convert 'this' pointer from 'nsCOMPtr<nsIMsgThread>' to 'const nsCOMPtr<nsIMsgThread> &&'

/builds/slave/tb-c-cen-m64-ntly-000000000000/build/mailnews/db/msgdb/src/nsMsgDatabase.cpp:4764:17: error: conversion function from 'nsCOMPtr<nsIMsgThread>' to 'nsIMsgThread *'

Comment 1

11 months ago
It is not yet visible on trunk/try. How many occurrences are there?
And what is the pattern to convert there cases?
(Assignee)

Comment 2

11 months ago
Created attachment 8747670 [details] [diff] [review]
1269316-WIP.patch

Just tried to compile suite and stumbled over this. 

Fix for first three errors. Got the next one in ldap. 

My c++ knowledge is limited. Fix might be rubbish.

Comment 3

11 months ago
(In reply to Frank-Rainer Grahl from comment #2)
> Created attachment 8747670 [details] [diff] [review]
> 1269316-WIP.patch
> 
> Just tried to compile suite and stumbled over this. 
> 
> Fix for first three errors. Got the next one in ldap. 
> 
> My c++ knowledge is limited. Fix might be rubbish.

I'm sure you could f?aryeh if you have a patch.
Blocks: 1193762
(Assignee)

Comment 4

11 months ago
Created attachment 8747683 [details] [diff] [review]
1269316-c-c-nscomptr.patch

ldap blagg comparing an object to 0... Hope the is the right approach.

Seamonkey including lightning compiles and runs with this one. Only a quick test. Any other issues in TB or I could take the bug and ask for review?
Attachment #8747670 - Attachment is obsolete: true
(Reporter)

Comment 5

11 months ago
Thanks for taking this on so quickly. I'm not at my desk right now, so I can't try it in TB. I should be back in two hours to try it if no one else has done it by then.

Otherwise I'd f?:Aryeh Gregor.
(Reporter)

Comment 6

11 months ago
Comment on attachment 8747683 [details] [diff] [review]
1269316-c-c-nscomptr.patch

Looks good to me, I've kicked off a compile now.

Aryeh, do you have three minutes to look this over? All straight forward.
Attachment #8747683 - Flags: feedback?(ayg)
(Reporter)

Updated

11 months ago
Assignee: nobody → frgrahl
Status: NEW → ASSIGNED
(Reporter)

Comment 7

11 months ago
Well, there is at least one more:
 4:13.91 c:/mozilla-source/comm-central/mailnews/imap/src/nsImapMailFolder.cpp(2237): error C2666: 'operator !=': 3 over
loads have similar conversions

nsCOMPtr<nsIMsgFolder> trashFolder;
...
NS_ASSERTION(trashFolder != 0, "couldn't find trash");
(Assignee)

Comment 8

11 months ago
Hmm suite must not use this one. 

Couldn't this just be 

NS_ASSERTION(trashFolder, "couldn't find trash");

?

Seems to be a simple null pointer check but there we are with my c++ non knowledge :)

FRG
(Reporter)

Comment 9

11 months ago
(In reply to Frank-Rainer Grahl from comment #8)
> Couldn't this just be 
> NS_ASSERTION(trashFolder, "couldn't find trash");
Yep. I'll update the patch in a minute.
(Reporter)

Comment 10

11 months ago
Another one ;-)
 1:17.69 c:/mozilla-source/comm-central/calendar/base/backend/libical/calDateTime.cpp(477): error C2280: 'nsCOMPtr<calITimezone>::operator T(void) const &&': attempting to reference a deleted function
(Assignee)

Comment 11

11 months ago
Didn't do a full build. Looks like I should have. Not at the desktop right now. Should be similar to one of the first ones i the patch.
(Reporter)

Comment 12

11 months ago
Sure. I found more. I'll keep going and attach the patch when I'm done.
(Reporter)

Comment 13

11 months ago
Created attachment 8747853 [details] [diff] [review]
FRG's work plus a few more hunks he missed

One review will do. All straight forward.
Attachment #8747683 - Attachment is obsolete: true
Attachment #8747683 - Flags: feedback?(ayg)
Attachment #8747853 - Flags: review?(rkent)
Attachment #8747853 - Flags: review?(mkmelin+mozilla)

Comment 14

11 months ago
Comment on attachment 8747853 [details] [diff] [review]
FRG's work plus a few more hunks he missed

Review of attachment 8747853 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, r=mkmelin

::: ldap/xpcom/src/nsLDAPOperation.cpp
@@ +519,5 @@
>  {
>      nsresult rv;
>      nsresult retStatus = NS_OK;
>  
> +    if (!mMessageListener || mMsgID == 0 ) {

mind removing the space before ) too?
Attachment #8747853 - Flags: review?(rkent)
Attachment #8747853 - Flags: review?(mkmelin+mozilla)
Attachment #8747853 - Flags: review+
(Reporter)

Comment 15

11 months ago
Sure. I'll wait until Kent has finished building with it, then I push.
(Reporter)

Comment 16

11 months ago
https://hg.mozilla.org/comm-central/rev/66dea6d980f6
(Kent's build never worked, but Richard confirmed a working build.)
Status: ASSIGNED → RESOLVED
Last Resolved: 11 months ago
status-firefox49: affected → ---
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 49.0
(Reporter)

Comment 17

11 months ago
Thanks a lot to Frank-Rainer Grahl! I've just added a few hunks to his patch and it got landed in his name.

Comment 18

11 months ago
(In reply to Jorg K (GMT+2) from comment #17)
> Thanks a lot to Frank-Rainer Grahl! I've just added a few hunks to his patch
> and it got landed in his name.
And when Thunderbird emails your credit card information to the Russian Mafia, everyone will blame frg.
(Assignee)

Comment 19

11 months ago
>> And when Thunderbird emails your credit card information to the Russian Mafia, everyone will blame frg.

Nahh. I am good friends with them. They wouldn't do this to me.

FRG (now back to serious business)
You need to log in before you can comment on or make changes to this bug.