Bug 65807 – Should not use C-style cast in CSSParser.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Kenichi Ishibashi]

I submitted r92528, which adds a C-style cast to fix build failure.  However, I should have used C++-style cast.

[Kenichi Ishibashi]

Created attachment 103140 [details]
Patch

[Kenichi Ishibashi]

(Cc'ed Mark just in a case)

Hamaji-san, this is a trivial patch. Could you take a look when you have time?

Thanks,

[Shinichiro Hamaji]

Comment on attachment 103140 [details]
Patch

OK

[WebKit Review Bot]

Comment on attachment 103140 [details]
Patch

Clearing flags on attachment: 103140

Committed r92549: <http://trac.webkit.org/changeset/92549>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Alexey Proskuryakov]

Comment on attachment 103140 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=103140&action=review

> Source/WebCore/css/CSSParser.cpp:3287
> +        numSteps = static_cast<int>(min(v->fValue, (double)INT_MAX));

(double) is still a C-style cast.

[Kent Tamura]

Comment on attachment 103140 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=103140&action=review

> Source/WebCore/css/CSSParser.cpp:3287
> -        numSteps = (int) min(v->fValue, (double)INT_MAX);
> +        numSteps = static_cast<int>(min(v->fValue, (double)INT_MAX));

You might want to use clampToInteger() in wtf/MathExtras.h.

[Kenichi Ishibashi]

(In reply to comment #7)
> (From update of attachment 103140 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=103140&action=review
> 
> > Source/WebCore/css/CSSParser.cpp:3287
> > -        numSteps = (int) min(v->fValue, (double)INT_MAX);
> > +        numSteps = static_cast<int>(min(v->fValue, (double)INT_MAX));
> 
> You might want to use clampToInteger() in wtf/MathExtras.h.

Thank you for pointing this out. Reopening this bug and will fix it with clampToInteger().

[Kenichi Ishibashi]

Created attachment 103204 [details]
Patch V1

[Kent Tamura]

Comment on attachment 103204 [details]
Patch V1

View in context: https://bugs.webkit.org/attachment.cgi?id=103204&action=review

> Source/WebCore/css/CSSParser.cpp:6336
> -            tagValue = static_cast<int>(value->fValue);
> -            if (tagValue < 0)
> -                return false;
> +            tagValue = clampToInteger(value->fValue);

Do you mean clampToPositiveInteger()?

[Kent Tamura]

Comment on attachment 103204 [details]
Patch V1

View in context: https://bugs.webkit.org/attachment.cgi?id=103204&action=review

>> Source/WebCore/css/CSSParser.cpp:6336
>> +            tagValue = clampToInteger(value->fValue);
> 
> Do you mean clampToPositiveInteger()?

Is it ok not to return false in a case of negative value?

[Kenichi Ishibashi]

Created attachment 103207 [details]
Patch V2

[Kenichi Ishibashi]

Created attachment 103209 [details]
Patch V3

[Kenichi Ishibashi]

(In reply to comment #11)
> (From update of attachment 103204 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=103204&action=review
> 
> >> Source/WebCore/css/CSSParser.cpp:6336
> >> +            tagValue = clampToInteger(value->fValue);
> > 
> > Do you mean clampToPositiveInteger()?
> 
> Is it ok not to return false in a case of negative value?

Should return false in a case of negative value.  Revised the patch.  Thanks!

[Kent Tamura]

Comment on attachment 103209 [details]
Patch V3

ok

[WebKit Review Bot]

Comment on attachment 103209 [details]
Patch V3

Clearing flags on attachment: 103209

Committed r92588: <http://trac.webkit.org/changeset/92588>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.