Closed Bug 1072605 Opened 10 years ago Closed 10 years ago

gfx/2d/Logging.h checks for PR_LOGGING before including prlog.h

Categories

(Core :: Graphics, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35

People

(Reporter: erahm, Assigned: erahm)

References

Details

Attachments

(1 file, 1 obsolete file)

|PR_LOGGING| is defined in "prlog.h", but we check for it prior to including "prlog.h" [1]

We should just include prlog.h outside of the #define scope and then change the various 
  |#if defined(DEBUG) || defined(PR_LOGGING)|
to just
  |#if defined(PR_LOGGING)|

[1] http://hg.mozilla.org/mozilla-central/annotate/5e704397529b/gfx/2d/Logging.h#l30
This is pretty basic. I unconditionally #include <prlog.h> and only look at PR_LOGGING.
Attachment #8495408 - Flags: review?(bas)
Assignee: nobody → erahm
Status: NEW → ASSIGNED
Blocks: OneLogger
Comment on attachment 8495408 [details] [diff] [review]
Just use PR_LOGGING to determine if logging is enabled

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

::: gfx/2d/Logging.h
@@ +9,5 @@
>  #include <string>
>  #include <sstream>
>  #include <stdio.h>
>  
> +#include <prlog.h>

You can't unconditionally include this file inside Moz2D. Since it's not available in the stand-alone build.
Attachment #8495408 - Flags: review?(bas) → review-
Comment on attachment 8495408 [details] [diff] [review]
Just use PR_LOGGING to determine if logging is enabled

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

::: gfx/2d/Logging.h
@@ +9,5 @@
>  #include <string>
>  #include <sstream>
>  #include <stdio.h>
>  
> +#include <prlog.h>

We currently do this in debug builds, so nothing has really changed. Unless we don't support stand-alone debug builds? If that's the case what would you suggest wrapping this include in?
Blocks: 806819
No longer blocks: OneLogger
Flags: needinfo?(bas)
Comment on attachment 8495408 [details] [diff] [review]
Just use PR_LOGGING to determine if logging is enabled

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

::: gfx/2d/Logging.h
@@ +9,5 @@
>  #include <string>
>  #include <sstream>
>  #include <stdio.h>
>  
> +#include <prlog.h>

This is a recent change, it's good you pointed it out, ut's a little silly, Debug stand-alone builds don't define DEBUG (rather _DEBUG). It's actually a bug already, can we come up with a sane way of fixing this include, then I can fix the defining of DEBUG :).
MOZ_LOGGING should be sufficiently mozilla specific to keep prlog.h out of the standalone build. This should also fix bustage related to DEBUG builds on non-mozilla targets.
Attachment #8500230 - Flags: review?(bas)
Attachment #8495408 - Attachment is obsolete: true
Comment on attachment 8500230 [details] [diff] [review]
Just use PR_LOGGING to determine if logging is enabled

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

Looks good to me!
Attachment #8500230 - Flags: review?(bas) → review+
https://hg.mozilla.org/mozilla-central/rev/ef7d6554b13d
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
You need to log in before you can comment on or make changes to this bug.