Bug 82376 – [Coverity] Address uninit constructor issues

• Component: New Bugs
• Status: RESOLVED FIXED

[kmadhusu]

[Coverity] Address some uninitialized constructor values.

[kmadhusu]

Created attachment 134130 [details]
Patch

[WebKit Review Bot]

Attachment 134130 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1
Source/WebKit/chromium/ChangeLog:3:  Line contains tab character.  [whitespace/tab] [5]
Source/WebKit/chromium/ChangeLog:4:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:4:  Line contains tab character.  [whitespace/tab] [5]
Source/WebCore/ChangeLog:8:  You should remove the 'No new tests' and either add and list tests, or explain why no new tests were possible.  [changelog/nonewtests] [5]
Total errors found: 4 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[kmadhusu]

Created attachment 134134 [details]
Fixed style errors

[James Robinson]

Comment on attachment 134130 [details]
Patch

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

Most of these changes seem fine, but some do not (see inline comments). Also please fix the tab characters.

> Source/WebCore/html/shadow/MediaControlRootElementChromium.cpp:62
>  #endif
> +    , m_textTrackDisplay(0)

this should be inside the #if ENABLE() guard, not outside of it

> 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?

> Source/WebCore/platform/graphics/skia/ImageBufferSkia.cpp:130
> +    , m_resolutionScale(0)

i think 1 is a better value to initialize to

[James Robinson]

Comment on attachment 134134 [details]
Fixed style errors

Same comments apply

[kmadhusu]

Created attachment 134140 [details]
Addressed review comments.

[kmadhusu]

Comment on attachment 134130 [details]
Patch

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.

[James Robinson]

Comment on attachment 134140 [details]
Addressed review comments.

Cool, looks good.

[WebKit Review Bot]

Comment on attachment 134140 [details]
Addressed review comments.

Rejecting attachment 134140 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 1

ERROR: /mnt/git/webkit-commit-queue/Source/WebKit/chromium/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive).

Full output: http://queues.webkit.org/results/12141785

[kmadhusu]

jamesr@: Thanks for reviewing my code. I am not sure why the commit queue failed. Do I need to make any changes to the ChangeLog?

[James Robinson]

(In reply to comment #10)
> jamesr@: Thanks for reviewing my code. I am not sure why the commit queue failed. Do I need to make any changes to the ChangeLog?

I think it picked up the wrong patch because patch set 2 wasn't marked "obsolete".  Will try it again.

Are you using webkit-patch upload to upload these patches? It takes care of many of these things for you.

[James Robinson]

Comment on attachment 134140 [details]
Addressed review comments.

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

> Source/WebKit/chromium/ChangeLog:6
> +        https://bugs.webkit.org/show_bug.cgi?id=82376
> +
> +        New tests are not required since I didn't modify any code behavior. I just initalized the member variables.

actually i take that back - you need to have the "Reviewed by NOBODY (OOPS!)." line in this ChangeLog entry as well as the WebCore/ChangeLog one.

[kmadhusu]

Oh okay. I didn't  use webkit-patch upload script. Can I modify the ChangeLog now to include that text?

[James Robinson]

Yeah, just add those lines back and upload a new patch.

[kmadhusu]

Created attachment 134170 [details]
Patch

[WebKit Review Bot]

Comment on attachment 134170 [details]
Patch

Clearing flags on attachment: 134170

Committed r112343: <http://trac.webkit.org/changeset/112343>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.