[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