Summary: | WebCore fails to compile with -Werror=maybe-uninitialized on GCC 4.8.0 | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Hugo Parente Lima <hugo.lima> | ||||||
Component: | WebCore Misc. | Assignee: | Hugo Parente Lima <hugo.lima> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | cdumez, commit-queue, darin, kenneth, laszlo.gombos | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Hugo Parente Lima
2013-05-17 12:06:53 PDT
Created attachment 202133 [details]
Patch
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. (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. (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; 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. Comment on attachment 202133 [details]
Patch
findStorageArea() looks like an awful mess :(
Can you fix that instead?
(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. Created attachment 202475 [details]
Patch
Comment on attachment 202475 [details]
Patch
Looks fine. r=me.
Comment on attachment 202475 [details] Patch Clearing flags on attachment: 202475 Committed r150513: <http://trac.webkit.org/changeset/150513> All reviewed patches have been landed. Closing bug. 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. (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 =] |