[webkit-reviews] review denied: [Bug 74965] [Coverity] Fix uninitialized constructor defects in .../html : [Attachment 120110] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Dec 20 16:48:09 PST 2011


Alexey Proskuryakov <ap at webkit.org> has denied Greg Billock
<gbillock at google.com>'s request for review:
Bug 74965: [Coverity] Fix uninitialized constructor defects in .../html
https://bugs.webkit.org/show_bug.cgi?id=74965

Attachment 120110: Patch
https://bugs.webkit.org/attachment.cgi?id=120110&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=120110&action=review


It's great that Coverity found a real bug. We shouldn't take any complaint it
makes as a requirement though. The changes in WebGLGetInfo look clearly against
the intended design of this class.

> Source/WebCore/html/StepRange.cpp:42
> -    if (element->hasAttribute(precisionAttr)) {
> -	   step = 1.0;
> +    step = 1.0;
> +    if (element->hasAttribute(precisionAttr))
>	   hasStep = !equalIgnoringCase(element->getAttribute(precisionAttr),
"float");
> -    } else
> +    else
>	   hasStep = element->getAllowedValueStep(&step);

Is this change testable?

Although you're just moving this code, please note that WebKit coding style
asks to not use .0 suffix on floating point literals. The same issue is present
in some code you added below.

> Source/WebCore/html/canvas/CanvasStyle.h:91
> +	       CMYKAValues() : c(0.0), m(0.0), y(0.0), k(0.0), a(0.0) { }

No .0, again.

> Source/WebCore/html/track/WebVTTParser.cpp:113
> +    , m_currentStartTime(0.0)
> +    , m_currentEndTime(0.0)

No .0.


More information about the webkit-reviews mailing list