Closed Bug 1233423 Opened 4 years ago Closed 4 years ago

[Static Analysis][Logically dead code] In function nsStyleUtil::AppendStepsTimingFunction from nsStyleUtil.cpp

Categories

(Core :: CSS Parsing and Computation, defect)

defect
Not set

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox46 --- affected

People

(Reporter: andi, Assigned: andi)

References

(Blocks 1 open bug)

Details

(Keywords: coverity, Whiteboard: CID 1325692)

Attachments

(1 file)

The Static Analysis tool Coverity added in case when aSyntax == nsTimingFunction::StepSyntax::Keyword the following code is useless:

>>    case nsTimingFunction::StepSyntax::Keyword:
>>      // handled above
>>      break;

because earlier in function this code block is present

>>  if (aSyntax == nsTimingFunction::StepSyntax::Keyword) {
>>    if (aType == nsTimingFunction::Type::StepStart) {
>>      aResult.AppendLiteral("step-start");
>>    } else {
>>      aResult.AppendLiteral("step-end");
>>    }
>>    return;
>>  }

I propose to eliminate the useless code.
Attached patch Bug 1233423.diffSplinter Review
Attachment #8699479 - Flags: review?(dbaron)
Comment on attachment 8699479 [details] [diff] [review]
Bug 1233423.diff

This code probably exists to silence a compiler warning.

I probably favor leaving it as it is, since I consider compiler warnings more valuable than Coverity warnings.
Attachment #8699479 - Flags: review?(dbaron) → review-
OK thank you David!
Yes, that produces a warning (and an error if --enable-warnings-as-errors) due to not handling all enum values in the switch statement.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.