Open Bug 710971 Opened 8 years ago Updated 8 years ago

Possible bad logic / type in nsStyleAnimation::AddWeighted()


(Core :: CSS Parsing and Computation, defect, P3)





(Reporter: Dolske, Unassigned)



(Whiteboard: [pvs-studio])


Example 5. Incomplete checking of input values

  if (unit[0] == eCSSUnit_Null || unit[1] == eCSSUnit_Null ||
      unit[0] == eCSSUnit_Null || unit[0] == eCSSUnit_URL) {
    return PR_FALSE;

PVS-Studio diagnostic message: V501 There are identical sub-expressions 'unit [0] == eCSSUnit_Null' to the left and to the right of the '||' operator. nsstyleanimation.cpp 1767

It seems to me that this code fragment contains 2 misprints simultaneously. I cannot tell for sure how exactly the code should look, but the developers most likely intended it to be written as follows:

if (unit[0] == eCSSUnit_Null || unit[1] == eCSSUnit_Null ||
    unit[0] == eCSSUnit_URL  || unit[1] == eCSSUnit_URL) {

The misprints may cause the function to process incorrect input values.
Blocks: 710966
We need to look at both ComputeDistance and AddWeighted.

There was originally only an nsCSSValuePair case, and it checked both units against null. added a URL check for the first unit.  Sort of a special case, but reasonable.

I think the mistakes come in the CSSValueTriplet case, from :
 * ComputeDistance and AddWeighted have different logic there
 * neither checks unit[2] against eCSSUnit_Null, though ComputeDistance almost did (triggering this warning)
 * I think, additionally, neither ValueTriplet case should check against eCSSUnit_URL

Nothing actually looks dangerous here, though.
Blocks: 505115
Priority: -- → P3
You need to log in before you can comment on or make changes to this bug.