RESOLVED FIXED 116340
WebCore fails to compile with -Werror=maybe-uninitialized on GCC 4.8.0
https://bugs.webkit.org/show_bug.cgi?id=116340
Summary WebCore fails to compile with -Werror=maybe-uninitialized on GCC 4.8.0
Hugo Parente Lima
Reported 2013-05-17 12:06:53 PDT
WebCore fails to compile with -Werror=maybe-uninitialized on GCC 4.8.0
Attachments
Patch (2.03 KB, patch)
2013-05-17 12:10 PDT, Hugo Parente Lima
no flags
Patch (2.25 KB, patch)
2013-05-21 15:49 PDT, Hugo Parente Lima
no flags
Hugo Parente Lima
Comment 1 2013-05-17 12:10:14 PDT
Chris Dumez
Comment 2 2013-05-17 12:27:30 PDT
Comment on attachment 202133 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=202133&action=review > Source/WebCore/inspector/InspectorDOMStorageAgent.cpp:157 > RefPtr<StorageArea> storageArea = findStorageArea(0, storageId, frame); Could we maybe initialize frame in findStorageArea() error cases instead? This would avoid double initialization in normal cases.
Hugo Parente Lima
Comment 3 2013-05-17 12:43:52 PDT
(In reply to comment #2) > (From update of attachment 202133 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=202133&action=review > > > Source/WebCore/inspector/InspectorDOMStorageAgent.cpp:157 > > RefPtr<StorageArea> storageArea = findStorageArea(0, storageId, frame); > > Could we maybe initialize frame in findStorageArea() error cases instead? This would avoid double initialization in normal cases. Could be, let me see is g++ is smart enough.
Hugo Parente Lima
Comment 4 2013-05-17 12:47:38 PDT
(In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 202133 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=202133&action=review > > > > > Source/WebCore/inspector/InspectorDOMStorageAgent.cpp:157 > > > RefPtr<StorageArea> storageArea = findStorageArea(0, storageId, frame); > > > > Could we maybe initialize frame in findStorageArea() error cases instead? This would avoid double initialization in normal cases. > > Could be, let me see is g++ is smart enough. Hmmm, it isn't, findStorageArea() already does that: line 237: targetFrame = frame;
Chris Dumez
Comment 5 2013-05-17 12:49:29 PDT
Comment on attachment 202133 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=202133&action=review >>>> Source/WebCore/inspector/InspectorDOMStorageAgent.cpp:157 >>>> RefPtr<StorageArea> storageArea = findStorageArea(0, storageId, frame); >>> >>> Could we maybe initialize frame in findStorageArea() error cases instead? This would avoid double initialization in normal cases. >> >> Could be, let me see is g++ is smart enough. > > Hmmm, it isn't, findStorageArea() already does that: > > line 237: targetFrame = frame; No, it doesn't in the errors cases (when it returns 0). Which is why I assume g++ complains.
Benjamin Poulain
Comment 6 2013-05-17 13:51:33 PDT
Comment on attachment 202133 [details] Patch findStorageArea() looks like an awful mess :( Can you fix that instead?
Hugo Parente Lima
Comment 7 2013-05-20 12:41:46 PDT
(In reply to comment #6) > (From update of attachment 202133 [details]) > findStorageArea() looks like an awful mess :( > Can you fix that instead? Just a mess, awful mess is too much for a little piece of code :-). Yes, this function could be refactored but the warning probably will be triggered anyway, so I'm start to think that the better way to fix it is to just disable this warning "maybe-uninitialized" due to the amount of false-positives generated by it.
Hugo Parente Lima
Comment 8 2013-05-21 15:49:51 PDT
Chris Dumez
Comment 9 2013-05-21 22:45:00 PDT
Comment on attachment 202475 [details] Patch Looks fine. r=me.
WebKit Commit Bot
Comment 10 2013-05-22 06:44:51 PDT
Comment on attachment 202475 [details] Patch Clearing flags on attachment: 202475 Committed r150513: <http://trac.webkit.org/changeset/150513>
WebKit Commit Bot
Comment 11 2013-05-22 06:44:53 PDT
All reviewed patches have been landed. Closing bug.
Darin Adler
Comment 12 2013-05-22 11:40:46 PDT
Another way to have fixed this warning would be to have just added this line to the top of the function: targetFrame = 0; We could have left the rest alone.
Hugo Parente Lima
Comment 13 2013-05-22 11:53:31 PDT
(In reply to comment #12) > Another way to have fixed this warning would be to have just added this line to the top of the function: > > targetFrame = 0; > > We could have left the rest alone. OTOH causes a double initialization in normal cases, of a cheap pointer, but causes =]
Note You need to log in before you can comment on or make changes to this bug.