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

Status: RESOLVED FIXED

(MailNews Core :: Import, defect)

Description

[Rolf Bode-Meyer]

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.

Comment 1

[:aceman]

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

[:aceman]

Attachment: patch

Rolf, can you try out this patch?

(That file could really use some cleanup. I've done only the most visible whitespace fixes.)

Comment 3

[Rolf Bode-Meyer]

(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

[Rolf Bode-Meyer]

Attachment: 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

[:aceman]

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

[Rolf Bode-Meyer]

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

Comment 7

[:aceman]

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

Standard8 is the owner of the Import module.

Comment 8

[Mark Banner (:standard8)]

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

Sorry for the delay, this looks fine r=me.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/f77f392df798