[webkit-reviews] review requested: [Bug 82376] [Coverity] Address uninit constructor issues : [Attachment 134130] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Mar 27 14:48:28 PDT 2012
kmadhusu <kmadhusu at chromium.org> has asked for review:
Bug 82376: [Coverity] Address uninit constructor issues
https://bugs.webkit.org/show_bug.cgi?id=82376
Attachment 134130: Patch
https://bugs.webkit.org/attachment.cgi?id=134130&action=review
------- Additional Comments from kmadhusu <kmadhusu at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=134130&action=review
Addressed review comments. Please review the latest patch. Thanks.
>> Source/WebCore/ChangeLog:4
>> + https://bugs.webkit.org/show_bug.cgi?id=82376
>
> Line contains tab character. [whitespace/tab] [5]
Fixed.
>> Source/WebCore/ChangeLog:8
>> + No new tests. (OOPS!)
>
> You should remove the 'No new tests' and either add and list tests, or
explain why no new tests were possible. [changelog/nonewtests] [5]
Done
>> Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp:62
>> + , m_textTrackDisplay(0)
>
> this should be inside the #if ENABLE() guard, not outside of it
oops. Fixed.
>> Source/WebCore/inspector/InspectorIndexedDBAgent.cpp:569
>> + , m_enabled(false)
>
> I'm not sure this is correct - this bool doesn't seem to actually be used.
maybe it should be deleted instead?
Yeah. Even I feel it should be deleted. So I made code changes accordingly.
>> Source/WebCore/platform/graphics/skia/ImageBufferSkia.cpp:130
>> + , m_resolutionScale(0)
>
> i think 1 is a better value to initialize to
Sure. Done
>> Source/WebKit/chromium/ChangeLog:3
>> + [Coverity] Address uninit constructor issues.
>
> Line contains tab character. [whitespace/tab] [5]
Fixed.
>> Source/WebKit/chromium/ChangeLog:4
>> + https://bugs.webkit.org/show_bug.cgi?id=82376
>
> Line contains tab character. [whitespace/tab] [5]
Fixed.
More information about the webkit-reviews
mailing list