[Coverity] Address some uninitialized constructor values.
Created attachment 134130 [details] Patch
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.
Created attachment 134134 [details] Fixed style errors
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
Comment on attachment 134134 [details] Fixed style errors Same comments apply
Created attachment 134140 [details] Addressed review comments.
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.
Comment on attachment 134140 [details] Addressed review comments. Cool, looks good.
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
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?
(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.
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.
Oh okay. I didn't use webkit-patch upload script. Can I modify the ChangeLog now to include that text?
Yeah, just add those lines back and upload a new patch.
Created attachment 134170 [details] Patch
Comment on attachment 134170 [details] Patch Clearing flags on attachment: 134170 Committed r112343: <http://trac.webkit.org/changeset/112343>
All reviewed patches have been landed. Closing bug.