Fix "variable is used uninitialized whenever switch default is taken" warnings in nsRuleNode.cpp

RESOLVED DUPLICATE of bug 918176

Status

()

defect
RESOLVED DUPLICATE of bug 918176
7 years ago
6 years ago

People

(Reporter: jwatt, Unassigned)

Tracking

(Blocks 1 bug)

Trunk
x86
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

There's a "variable is used uninitialized whenever switch default is taken" warnings in nsRuleNode.cpp that we should fix.
Posted patch patchSplinter Review
Attachment #719730 - Flags: review?(dholbert)
Hmm.  So if this were an enum, we could just drop the "default" case entirely, and the compiler would take care of warning us if we add a new enum type later on without adding it to this switch statement.

I'm wary about using MOZ_NOT_REACHED, because behavior is literally undefined when we hit that statement, and triggering undefined behavior is scary.  It's rarely the right thing to use -- see bug 820686 comment 0 and bug 820686 comment 8.

I'd feel better about this if we had NS_RUNTIMEABORT() (or the MOZ_ equivalent) before the MOZ_NOT_REACHED, to ensure that we safely crash, even in opt builds, rather than triggering undefined behavior.

Even with that, though, I still feel a bit hesitant -- a runtime abort is a bit heavy-handed, just to fix a build warning.  (Plus, we don't really have any type-enforced guarantees that the value we're switching over will be in the expected range -- it's just an int with some #define'd values, not an enum or anything.)

I'd rather we just initialized the variable to some reasonable value in the "default" case (maybe whatever the property's initial value is).  We'll still hit the NS_ERROR(), so debug builds will complain, but our actual release builds won't crash & will have defined (& likely-reasonable) behavior.

How does that sound?
Attachment #719730 - Flags: review?(dholbert) → review-
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 918176
Assignee: jwatt → nobody
You need to log in before you can comment on or make changes to this bug.