1108932 - dom/security/nsCSPParser.cpp: error: unused variable 'COMMA' [-Werror,-Wunused-const-variable]

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: CSPParser_Wunused-const-variable.patch

1. Remove unused COMMA constant to fix -Wunused-const-variable warning-as-error in non-unified build on OS X.
2. Add some more const attributes.
3. Mark some local functions as static.
4. Remove some extra whitespace.

Comment 1

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8533528 [details] [diff] [review]
CSPParser_Wunused-const-variable.patch

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

Thanks Chris! COMMA is part of the RFC production of paths but can only appear pct-encoded. Should have removed it from the list. Thanks again for cleaning that up.

Comment 2

[Daniel Holbert [:dholbert]]

cpeterson: look like you marked this as depending on the general FAIL_ON_WARNINGS bug -- was that intentional? (I don't see any direct connection between this bug & that bug.)

Comment 3

[Chris Peterson [:cpeterson]]

(In reply to Daniel Holbert [:dholbert] from comment #2)
> cpeterson: look like you marked this as depending on the general
> FAIL_ON_WARNINGS bug -- was that intentional? (I don't see any direct
> connection between this bug & that bug.)

I made this bug depend on the FAIL_ON_WARNINGS bug because this bug was a warning-as-error build break with non-unified builds (because clang seems to do a better job tracking unused variables in smaller translation units). If that is not how you use the FAIL_ON_WARNINGS bug to track warnings-as-errors, just let me know. :)

Comment 4

[Daniel Holbert [:dholbert]]

Gotcha.

So, I've been using the FAIL_ON_WARNINGS bug *just* as a dependency for bugs that add new usages of FAIL_ON_WARNINGS. (So, bug A adds a feature; bugs X,Y,Z use the feature & hence depend on A.) I think that makes sense & is scalable.

I don't think it's as scalable to have all new build-bustage bugs marked as depending on that bug. (It does seem likely-useful to track such bugs somehow, but I'm not sure what the best way is. For now, "bugs marked as blocking buildwarning & which have 'error' in the summary" might be a good ad-hoc way to track such bugs.)

Comment 5

[Daniel Holbert [:dholbert]]

(So, I'd prefer to have Werror-build-bustage bugs not depend on FAIL_ON_WARNINGS. I think it makes the most sense for them to block (not depend on): (a) the bug that's "guilty" of causing the warning, and perhaps also (b) the bug that annotated the directory as FAIL_ON_WARNINGS, and perhaps (c) we can use some other keyword/metabug to track this class of bugs if that's useful.)

Comment 6

[Chris Peterson [:cpeterson]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/927963e7b037

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/927963e7b037