building Mozilla fails because of NS_ENSURE_SUCCESS in mailnews/import/oexpress/nsOEScanBoxes.cpp

RESOLVED FIXED in Thunderbird 22.0


MailNews Core
4 years ago
4 years ago


(Reporter: Rolf Bode-Meyer, Assigned: Rolf Bode-Meyer)


Thunderbird 22.0
Windows 8

Firefox Tracking Flags

(Not tracked)



(1 attachment, 1 obsolete attachment)



4 years ago
Building mailnews in fails in nsOEScanBoxes.cpp for several lines (85, 118, 554, an 800) because return value from NS_ENSURE_SUCCESS(rv, rv) can't be converted to boolean.
This is understandable since the methods with those macro calls are declared to return bool. What I don't see is why the code seemingly compiles for others and (I asume) tinderboxes.

Build is on Windows 8 with Visual Studio 12 Express.
Ever confirmed: true

Comment 1

4 years ago
Maybe others use a different compilers or you have especially pedantic settings?
Anyway, what you say seems logical, those NS_ENSURE_SUCCESS(rv, rv) seem out of place as the rest of the functions have "if (NS_FAILED(rv) return false)' so the conversion should be straightforward. Will you do it, or should I make the patch? You'd still need to compile-test it after me as I can't build for Windows.

Comment 2

4 years ago
Created attachment 708239 [details] [diff] [review]

Rolf, can you try out this patch?

(That file could really use some cleanup. I've done only the most visible whitespace fixes.)
Assignee: nobody → acelists
Attachment #708239 - Flags: feedback?(robome)

Comment 3

4 years ago
(In reply to :aceman from comment #2)
> Created attachment 708239 [details] [diff] [review]
> Rolf, can you try out this patch?

That patch doesn't work, but it's only a minor problem. I'll attach a working version of it.

Comment 4

4 years ago
Created attachment 709369 [details] [diff] [review]
fixed patch with additional changes

This patch fixes the problem of the former patch (assuming a typo: "NS_SUCisDir").
I also added changes to the files nsEudoraWin32.cpp and nsOE5File.cpp which have problems similar to the ones in nsOEScanBoxes.cpp. Should those be handled in this bug or a separate one?

Comment 5

4 years ago
Yeah, that was some thinko, I probably wanted to rewrite it, but declined later.
I think you can fix all the files (in one patch) in this bug. Everything needed to fix the build can be done here in one patch.

Comment 6

4 years ago
Aceman, do know whom to ask for a review for this at best?

Comment 7

4 years ago
Comment on attachment 709369 [details] [diff] [review]
fixed patch with additional changes

Standard8 is the owner of the Import module.
Attachment #709369 - Flags: review?(mbanner)


4 years ago
Attachment #708239 - Attachment is obsolete: true
Attachment #708239 - Flags: feedback?(robome)
Comment on attachment 709369 [details] [diff] [review]
fixed patch with additional changes

Sorry for the delay, this looks fine r=me.
Attachment #709369 - Flags: review?(mbanner) → review+


4 years ago
Keywords: checkin-needed
Assignee: acelists → robome
Last Resolved: 4 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 22.0
You need to log in before you can comment on or make changes to this bug.