705573 - Make mailnews compile with frozen linkage on debug Windows

Status: RESOLVED FIXED

(MailNews Core :: Build Config, defect)

Description

[[e-mail]]

+++ This bug was initially created as a clone of Bug #705553 +++

Just a few changes to help with the compilation process with frozen linkage.

Comment 1

[[e-mail]]

Attachment: Proposed patch

I wasn't actually able to get the Thunderbird version of nsMessengerWinIntegration to compile --with-libxul-sdk so these changes are only sufficient to get SeaMonkey to compile.

I didn't think it was worthwhile implementing nsPrintfCString. (I thought we'd done it somewhere else but I couldn't find it.)

The Makefile.in changes are because debug builds don't like to inline stuff and sometimes want to link to things that aren't even called. Sigh.

Comment 2

[Serge Gautherie (:sgautherie)]

Comment on attachment 577171 [details] [diff] [review]
Proposed patch

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

::: mailnews/base/util/nsStopwatch.cpp
@@ +162,3 @@
>    if (!ret)
>      NS_ERROR(nsPrintfCString("GetProcessTimes() failed, error=0x%lx.", GetLastError()).get());
>  #endif

Nit: Could that become
{{
  if (!ret) {
#ifdef MOZILLA_INTERNAL_API
    ... with GetLastError()
#else
    // A comment about nsPrintfCString() not being available.
    ... without GetLastError()
#end
  }
}}
fwiw ?
Or could the full error string be created in a different manner?

Comment 3

[[e-mail]]

(In reply to Serge Gautherie from comment #2)
> (From update of attachment 577171 [details] [diff] [review])
>     // A comment about nsPrintfCString() not being available.
Well, I could just NS_ERROR("GetProcessTimes() failed"); I suppose...

> Or could the full error string be created in a different manner?
Not really, because then you have to start fiddling around with #ifdef DEBUG.

Comment 4

[David :Bienvenu]

Comment on attachment 577171 [details] [diff] [review]
Proposed patch

debug builds and runs...

Comment 5

[[e-mail]]

Pushed changeset 9b9931394d81 to comm-central.

Comment 6

[Serge Gautherie (:sgautherie)]

Attachment: (Bv1) GetCPUTime(): Report error when not MOZILLA_INTERNAL_API too [Checked in: See comment 9]

Per comment 2 and 3.
This should additionally fix a new "unused var ret" warning ;->

Comment 7

[Serge Gautherie (:sgautherie)]

David, ping for review.

Comment 8

[Mark Banner (:standard8)]

Comment on attachment 578905 [details] [diff] [review]
(Bv1) GetCPUTime(): Report error when not MOZILLA_INTERNAL_API too
[Checked in: See comment 9]

> #ifdef DEBUG
>+  if (!ret)
> #ifdef MOZILLA_INTERNAL_API
>-  if (!ret)
>     NS_ERROR(nsPrintfCString("GetProcessTimes() failed, error=0x%lx.", GetLastError()).get());
>+#else
>+    // nsPrintfCString() is unavailable to report GetLastError().
>+    NS_ERROR("GetProcessTimes() failed.");
> #endif
> #endif

nit: I'd prefer for the if (!ret) to be there twice, as then it doesn't mess up indentation for editors that don't understand ifdefs.

r=me with that fixed.

Comment 9

[Serge Gautherie (:sgautherie)]

Comment on attachment 578905 [details] [diff] [review]
(Bv1) GetCPUTime(): Report error when not MOZILLA_INTERNAL_API too
[Checked in: See comment 9]

http://hg.mozilla.org/comm-central/rev/bc7c26552fe4
Bv1, with comment 8 suggestion(s).


"approval-comm-aurora=?":
AURORA_BASE_20111220 happened in the meantime :-/ No risk.

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 578905 [details] [diff] [review]
(Bv1) GetCPUTime(): Report error when not MOZILLA_INTERNAL_API too
[Checked in: See comment 9]

Default configurations will have this as internal API, and we don't even completely support external API on what is aurora now, plus it is debug only. Sorry, but this just isn't necessary on branch.