409760 - Zero-tolerance for warnings in mozilla/camino

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Mark Mentovai]

There are so few warnings in Camino code now.  We should clean up the few that remain and turn on -Werror to treat any future warnings as errors.

In this bug, we'll zap compiler warnings.  Linker warnings are a separate deal - we're already mostly clear on that front, except for 10.3.9 SDK stupidity and bug 409640.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Woo hoo!

Comment 2

[Reed Loden [:reed]]

Good job, Camino. :)

Comment 3

[Mark Mentovai]

Attachment: I got asked why I gave such a few warnings (trunk)

I found that we lost -Wall when we moved to xcconfig files, there were a handful more warnings lurking than it seemed.  No matter, I've fixed those too, and turned -Wall back on.

I tested these patches in a good variety of builds.  Feel free to test them in your own.

Comment 4

[Mark Mentovai]

Attachment: I got asked why I gave such a few warnings (Cm1.6-M1.8 branch)

Comment 5

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 294522 [details] [diff] [review]
I got asked why I gave such a few warnings (Cm1.6-M1.8 branch)

r=ardissone, provided someone else is also happy with the code changes.  I do get compiler-warning-free builds of x86-shared-debug, x86-static-opt, and ppc-static-opt with Xcode 3 toolchain :)

Out of curiosity (and since it was about the only thing I could follow in the patch), why are you removing

> -const int kDefaultExpireDays = 9;

from General; is it unused in General? I notice we declare that in 4 files on the branch: prefPanes Downloads.mm, General.mm, and History.mm, and src/history/HistoryDataSource.mm

Comment 6

[Mark Mentovai]

It's unused in General and Downloads.

In History, it should either
 - (better) be changed to static, or
 - (best) be changed to extern,
   and the one in HistoryDataSource should be changed to export (not static),
   so that the constant is properly shared.  Thanks, -bundle_loader!

There are other instances where we have constants that can be shared.  Even the static strings that I've done in General can be in the application itself and shared.

Comment 7

[Stuart Morgan]

Comment on attachment 294520 [details] [diff] [review]
I got asked why I gave such a few warnings (trunk)

>-WARNING_FLAGS = -Wall -Wno-four-char-constants
>+WARNING_CFLAGS = -Wall -Wno-four-char-constants

Doh! Sorry about that.

r=me

Comment 8

[Mark Mentovai]

Checked in on the trunk and MOZILLA_1_8_BRANCH for 1.6b1.

Comment 9

[Mark Mentovai]

Bug 410280 for ongoing data sharing between the application and preference panes per comments 5 and 6.