Last Comment Bug 834612 - building Mozilla fails because of NS_ENSURE_SUCCESS in mailnews/import/oexpress/nsOEScanBoxes.cpp
: building Mozilla fails because of NS_ENSURE_SUCCESS in mailnews/import/oexpre...
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Import (show other bugs)
: Trunk
: x86 Windows 8
: -- major (vote)
: Thunderbird 22.0
Assigned To: Rolf Bode-Meyer
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-01-24 22:45 PST by Rolf Bode-Meyer
Modified: 2013-03-21 13:53 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch (12.75 KB, patch)
2013-01-30 11:57 PST, :aceman
no flags Details | Diff | Review
fixed patch with additional changes (14.79 KB, patch)
2013-02-02 03:24 PST, Rolf Bode-Meyer
standard8: review+
Details | Diff | Review

Description Rolf Bode-Meyer 2013-01-24 22:45:04 PST
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 2013-01-28 07:44:04 PST
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 2013-01-30 11:57:39 PST
Created attachment 708239 [details] [diff] [review]
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 2013-02-02 03:19:59 PST
(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 2013-02-02 03:24:33 PST
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 :aceman 2013-02-03 11:14:28 PST
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 2013-02-21 10:06:03 PST
Aceman, do know whom to ask for a review for this at best?
Comment 7 :aceman 2013-02-21 10:56:15 PST
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) 2013-03-21 09:52:17 PDT
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] 2013-03-21 13:53:53 PDT
https://hg.mozilla.org/comm-central/rev/f77f392df798

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