If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Fix warnings in security/sandbox and mark as FAIL_ON_WARNINGS

RESOLVED FIXED in Firefox 34

Status

()

Core
Security
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: cpeterson, Assigned: cpeterson)

Tracking

(Blocks: 1 bug)

unspecified
mozilla34
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox33 unaffected, firefox34 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Created attachment 8470980 [details] [diff] [review]
fix-sandbox-warnings.patch

Fix clang warnings on OS X:

> security/sandbox/mac/Sandbox.mm:42:7 [-Wlogical-not-parentheses] logical not is only applied to the left hand side of this comparison                    
> security/sandbox/mac/Sandbox.mm:63:14 [-Wformat-security] format string is not a string literal (potentially insecure)

Suppress MSVC (false-positive) warning in third-party Chromium code:

> security\sandbox\chromium\base\shim\base\strings\string_piece.h(15) : warning C4717: '`anonymous namespace'::IsStringASCII' : recursive on all control paths, function will cause runtime stack overflow

Suppress MSVC warning in Microsoft header file about DLL exports. TBH, I'm not sure how serious this warning is but we have suppressed it in other bugs like bug 556886:

> c:\tools\msvs10\vc\include\typeinfo(157) : warning C4275: non dll-interface class 'stdext::exception' used as base for dll-interface class 'std::bad_cast'
Attachment #8470980 - Flags: review?(smichaud)
(Assignee)

Updated

3 years ago
Assignee: nobody → cpeterson
Status: NEW → ASSIGNED
(Nit: This should be "depends on" bug 557566 (the bug that added FAIL_ON_WARNINGS support), not blocking it.

If we had a meta-bug tracking "add FAIL_ON_WARNINGS to directories", like the metabug for "fix build warnings", then it would make sense to have this block *that* bug. But bug 557566 is not that bug -- it's where we added the ability to annotate directories.  And bugs like this one intuitively *depend* on that feature; they don't block it.)
No longer blocks: 557566
Depends on: 557566
Comment on attachment 8470980 [details] [diff] [review]
fix-sandbox-warnings.patch

This looks fine to me.

-  if (!aInfo.type == MacSandboxType_Plugin) {
+  if (aInfo.type != MacSandboxType_Plugin) {

Thanks especially for catching this!  It's rather embarrassing :-(
Attachment #8470980 - Flags: review?(smichaud) → review+
(Assignee)

Comment 3

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/953635c5e08b
status-firefox34: affected → fixed

Comment 4

3 years ago
https://hg.mozilla.org/mozilla-central/rev/953635c5e08b
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
Depends on: 1055541
You need to log in before you can comment on or make changes to this bug.