Closed
Bug 968323
Opened 10 years ago
Closed 10 years ago
Debug builds with --disable-logging fail in /security due to MOZ_LOGGING / PR_LOGGING distinction
Categories
(Core :: Security, defect)
Core
Security
Tracking
()
RESOLVED
FIXED
mozilla30
People
(Reporter: dholbert, Assigned: dholbert)
Details
Attachments
(2 files, 1 obsolete file)
2.74 KB,
patch
|
briansmith
:
review+
|
Details | Diff | Splinter Review |
3.12 KB,
patch
|
Details | Diff | Splinter Review |
Builds with ac_add_options --disable-logging currently fail in /security, with "use of undeclared identifier" errors like: security/certverifier/CertVerifier.cpp:158:12: error: use of undeclared identifier 'gCertVerifierLog' ...and... security/manager/ssl/src/nsNSSComponent.cpp:249:8: error: use of undeclared identifier 'gPIPNSSLog' ...and... security/manager/ssl/src/PSMContentListener.cpp:279:10: error: use of undeclared identifier 'gPIPNSSLog This seems to be due to these variables being declared inside of a "#ifdef MOZ_LOGGING" chunk, and then *used* in a "#ifdef PR_LOGGING" chunk. (often implicitly via PR_LOG(), whose behavior depends on whether PR_LOGGING is defined) We should be declaring these variables inside of #ifdef PR_LOGGING, not MOZ_LOGGING.
Assignee | ||
Comment 1•10 years ago
|
||
This fixes the errors for me (and allows me to successfully get through a --disable-logging debug build).
Assignee | ||
Comment 2•10 years ago
|
||
Note: enable-debug disable-logging builds end up in the slightly-confusing state of having PR_LOGGING enabled, despite MOZ_LOGGING being disabled, because of this: > 162 #if defined(DEBUG) || defined(FORCE_PR_LOG) > 163 #define PR_LOGGING 1 http://mxr.mozilla.org/mozilla-central/source/nsprpub/pr/include/prlog.h#162 i.e. PR_LOGGING is forced-on in debug builds (which is reasonable). (I'm not immediately clear what purpose MOZ_LOGGING serves, but it's apparently not equivalent to PR_LOGGING, and hence shouldn't be used as a guard for declaring variables that are used by PR_LOG.)
Summary: --disable-logging builds fail in /security due to MOZ_LOGGING / PR_LOGGING distinction → Debug builds with --disable-logging fail in /security due to MOZ_LOGGING / PR_LOGGING distinction
Comment 3•10 years ago
|
||
Comment on attachment 8370890 [details] [diff] [review] fix Review of attachment 8370890 [details] [diff] [review]: ----------------------------------------------------------------- NIT: please use r=briansmith in the commit message. Thanks!
Attachment #8370890 -
Flags: review?(brian) → review+
Assignee | ||
Comment 4•10 years ago
|
||
Found one more chunk that needed updating, on a try run. Fixed here. As a sanity-check, I confirmed that all the remaining '#ifdef MOZ_LOGGING' lines in /security are only wrapping #defines of FORCE_PR_LOGGING -- not variable-declarations or usages. So I think this should do it.
Attachment #8371207 -
Flags: review+
Assignee | ||
Comment 5•10 years ago
|
||
[er, sorry, wrong patch. Here's the right one.]
Attachment #8371207 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Attachment #8371207 -
Attachment description: fix v2 → [wrong patch]
Attachment #8371207 -
Flags: review+
Assignee | ||
Comment 6•10 years ago
|
||
Landed: https://hg.mozilla.org/integration/mozilla-inbound/rev/ca49b7e89881
Flags: in-testsuite-
OS: Linux → All
Hardware: x86_64 → All
Comment 7•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/ca49b7e89881
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
You need to log in
before you can comment on or make changes to this bug.
Description
•