116340 2013-05-17 12:06:53 -0700 WebCore fails to compile with -Werror=maybe-uninitialized on GCC 4.8.0 2013-05-22 11:53:31 -0700 1 1 1 Unclassified WebKit WebCore Misc. 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 hugo.lima hugo.lima cdumez commit-queue darin kenneth laszlo.gombos oldest_to_newest 890568 0 hugo.lima 2013-05-17 12:06:53 -0700 WebCore fails to compile with -Werror=maybe-uninitialized on GCC 4.8.0 890569 1 202133 hugo.lima 2013-05-17 12:10:14 -0700 Created attachment 202133 Patch 890584 2 202133 cdumez 2013-05-17 12:27:30 -0700 Comment on attachment 202133 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. 890591 3 hugo.lima 2013-05-17 12:43:52 -0700 (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. 890600 4 hugo.lima 2013-05-17 12:47:38 -0700 (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; 890603 5 202133 cdumez 2013-05-17 12:49:29 -0700 Comment on attachment 202133 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. 890653 6 202133 benjamin 2013-05-17 13:51:33 -0700 Comment on attachment 202133 Patch findStorageArea() looks like an awful mess :( Can you fix that instead? 891300 7 hugo.lima 2013-05-20 12:41:46 -0700 (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. 892035 8 202475 hugo.lima 2013-05-21 15:49:51 -0700 Created attachment 202475 Patch 892124 9 202475 cdumez 2013-05-21 22:45:00 -0700 Comment on attachment 202475 Patch Looks fine. r=me. 892269 10 202475 commit-queue 2013-05-22 06:44:51 -0700 Comment on attachment 202475 Patch Clearing flags on attachment: 202475 Committed r150513: <http://trac.webkit.org/changeset/150513> 892270 11 commit-queue 2013-05-22 06:44:53 -0700 All reviewed patches have been landed. Closing bug. 892469 12 darin 2013-05-22 11:40:46 -0700 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. 892474 13 hugo.lima 2013-05-22 11:53:31 -0700 (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 =] 202133 2013-05-17 12:10:14 -0700 2013-05-21 15:49:41 -0700 Patch bug-116340-20130517160742.patch text/plain 2078 hugo.lima U3VidmVyc2lvbiBSZXZpc2lvbjogMTUwMjcxCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYzM0NjJhNDFlZWJhNDJk YWVhNjRiM2RkNTJlMWNkNWEyOTIxZjgzMy4uYWI5ZWU5NDg5MjY0ZWU5M2UyYWNlZTllMWFjYzcw NmYxODVlOGFiOSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDEzLTA1LTE3ICBIdWdv IFBhcmVudGUgTGltYSAgPGh1Z28ubGltYUBvcGVuYm9zc2Eub3JnPgorCisgICAgICAgIFdlYkNv cmUgZmFpbHMgdG8gY29tcGlsZSB3aXRoIC1XZXJyb3I9bWF5YmUtdW5pbml0aWFsaXplZCBvbiBH Q0MgNC44LjAKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTExNjM0MAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IE5vIG5ldyB0ZXN0cywganVzdCBhIHNpbXBsZSBidWlsZCBmaXguCisKKyAgICAgICAgKiBpbnNw ZWN0b3IvSW5zcGVjdG9yRE9NU3RvcmFnZUFnZW50LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Oklu c3BlY3RvckRPTVN0b3JhZ2VBZ2VudDo6c2V0RE9NU3RvcmFnZUl0ZW0pOiBJbml0IGZyYW1lIHZh cmlhYmxlLgorICAgICAgICAoV2ViQ29yZTo6SW5zcGVjdG9yRE9NU3RvcmFnZUFnZW50OjpyZW1v dmVET01TdG9yYWdlSXRlbSk6IEluaXQgZnJhbWUgdmFyaWFibGUuCisKIDIwMTMtMDUtMTcgIERh cmluIEFkbGVyICA8ZGFyaW5AYXBwbGUuY29tPgogCiAgICAgICAgIFtFRkxdIE1vdmUgRUZMIHBv cnQgb2ZmIGxlZ2FjeSBjbGlwYm9hcmQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2luc3Bl Y3Rvci9JbnNwZWN0b3JET01TdG9yYWdlQWdlbnQuY3BwIGIvU291cmNlL1dlYkNvcmUvaW5zcGVj dG9yL0luc3BlY3RvckRPTVN0b3JhZ2VBZ2VudC5jcHAKaW5kZXggODZiZDY0YjY1NjhjNTg4NmM5 NTNhOTdiODYwZGYzNjNlYzEwMzQ5My4uNDAzOGYwMDRjOGFiM2VjYzkzNzRkYmY1MzA3MmZmNTQz NTcyMWQxOCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvaW5zcGVjdG9yL0luc3BlY3RvckRP TVN0b3JhZ2VBZ2VudC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvaW5zcGVjdG9yL0luc3BlY3Rv ckRPTVN0b3JhZ2VBZ2VudC5jcHAKQEAgLTE1Myw3ICsxNTMsNyBAQCBzdGF0aWMgU3RyaW5nIHRv RXJyb3JTdHJpbmcoY29uc3QgRXhjZXB0aW9uQ29kZSYgZWMpCiAKIHZvaWQgSW5zcGVjdG9yRE9N U3RvcmFnZUFnZW50OjpzZXRET01TdG9yYWdlSXRlbShFcnJvclN0cmluZyogZXJyb3JTdHJpbmcs IGNvbnN0IFJlZlB0cjxJbnNwZWN0b3JPYmplY3Q+JiBzdG9yYWdlSWQsIGNvbnN0IFN0cmluZyYg a2V5LCBjb25zdCBTdHJpbmcmIHZhbHVlKQogewotICAgIEZyYW1lKiBmcmFtZTsKKyAgICBGcmFt ZSogZnJhbWUgPSAwOwogICAgIFJlZlB0cjxTdG9yYWdlQXJlYT4gc3RvcmFnZUFyZWEgPSBmaW5k U3RvcmFnZUFyZWEoMCwgc3RvcmFnZUlkLCBmcmFtZSk7CiAgICAgaWYgKCFzdG9yYWdlQXJlYSkg ewogICAgICAgICAqZXJyb3JTdHJpbmcgPSAiU3RvcmFnZSBub3QgZm91bmQiOwpAQCAtMTY3LDcg KzE2Nyw3IEBAIHZvaWQgSW5zcGVjdG9yRE9NU3RvcmFnZUFnZW50OjpzZXRET01TdG9yYWdlSXRl bShFcnJvclN0cmluZyogZXJyb3JTdHJpbmcsIGNvbnN0CiAKIHZvaWQgSW5zcGVjdG9yRE9NU3Rv cmFnZUFnZW50OjpyZW1vdmVET01TdG9yYWdlSXRlbShFcnJvclN0cmluZyogZXJyb3JTdHJpbmcs IGNvbnN0IFJlZlB0cjxJbnNwZWN0b3JPYmplY3Q+JiBzdG9yYWdlSWQsIGNvbnN0IFN0cmluZyYg a2V5KQogewotICAgIEZyYW1lKiBmcmFtZTsKKyAgICBGcmFtZSogZnJhbWUgPSAwOwogICAgIFJl ZlB0cjxTdG9yYWdlQXJlYT4gc3RvcmFnZUFyZWEgPSBmaW5kU3RvcmFnZUFyZWEoMCwgc3RvcmFn ZUlkLCBmcmFtZSk7CiAgICAgaWYgKCFzdG9yYWdlQXJlYSkgewogICAgICAgICAqZXJyb3JTdHJp bmcgPSAiU3RvcmFnZSBub3QgZm91bmQiOwo= 202475 2013-05-21 15:49:51 -0700 2013-05-22 06:44:51 -0700 Patch bug-116340-20130521194713.patch text/plain 2304 hugo.lima U3VidmVyc2lvbiBSZXZpc2lvbjogMTUwNDc1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggYTVlYmQyNjJkMDg5MmRi ZmE5NzQ2YWRiODNlZjcyYjI2MTg5NjRlOC4uZDg3NTkwNjEyMDk3MGVlMDRiMzE0NTUwNWQwMGJh NjdlOWRjNDkxNSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDEzLTA1LTE3ICBIdWdv IFBhcmVudGUgTGltYSAgPGh1Z28ubGltYUBvcGVuYm9zc2Eub3JnPgorCisgICAgICAgIFdlYkNv cmUgZmFpbHMgdG8gY29tcGlsZSB3aXRoIC1XZXJyb3I9bWF5YmUtdW5pbml0aWFsaXplZCBvbiBH Q0MgNC44LjAKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTExNjM0MAorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg IE5vIG5ldyB0ZXN0cywganVzdCBhIHNpbXBsZSBidWlsZCBmaXguCisKKyAgICAgICAgKiBpbnNw ZWN0b3IvSW5zcGVjdG9yRE9NU3RvcmFnZUFnZW50LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Oklu c3BlY3RvckRPTVN0b3JhZ2VBZ2VudDo6c2V0RE9NU3RvcmFnZUl0ZW0pOiBJbml0IGZyYW1lIHZh cmlhYmxlLgorICAgICAgICAoV2ViQ29yZTo6SW5zcGVjdG9yRE9NU3RvcmFnZUFnZW50OjpyZW1v dmVET01TdG9yYWdlSXRlbSk6IEluaXQgZnJhbWUgdmFyaWFibGUuCisKIDIwMTMtMDUtMjEgIENv bW1pdCBRdWV1ZSAgPGNvbW1pdC1xdWV1ZUB3ZWJraXQub3JnPgogCiAgICAgICAgIFVucmV2aWV3 ZWQsIHJvbGxpbmcgb3V0IHIxNTA0MjUuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9pbnNw ZWN0b3IvSW5zcGVjdG9yRE9NU3RvcmFnZUFnZW50LmNwcCBiL1NvdXJjZS9XZWJDb3JlL2luc3Bl Y3Rvci9JbnNwZWN0b3JET01TdG9yYWdlQWdlbnQuY3BwCmluZGV4IDU4YWFkNDlhNDY0ZjEyN2M3 MzMwYTY4YWQ4YWQ4ZjhkMzU3YTkyMTguLjk4ZTMzNWIyMjFlMWFjNzQ4OTZhYmI4YWRkNmMwZTc5 MDFkMWQ3NjQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2luc3BlY3Rvci9JbnNwZWN0b3JE T01TdG9yYWdlQWdlbnQuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL2luc3BlY3Rvci9JbnNwZWN0 b3JET01TdG9yYWdlQWdlbnQuY3BwCkBAIC0xOTgsMjEgKzE5OCwyMSBAQCBQYXNzUmVmUHRyPFN0 b3JhZ2VBcmVhPiBJbnNwZWN0b3JET01TdG9yYWdlQWdlbnQ6OmZpbmRTdG9yYWdlQXJlYShFcnJv clN0cmluZyogZQogICAgIGlmICghc3VjY2VzcykgewogICAgICAgICBpZiAoZXJyb3JTdHJpbmcp CiAgICAgICAgICAgICAqZXJyb3JTdHJpbmcgPSAiSW52YWxpZCBzdG9yYWdlSWQgZm9ybWF0IjsK KyAgICAgICAgdGFyZ2V0RnJhbWUgPSAwOwogICAgICAgICByZXR1cm4gMDsKICAgICB9CiAKLSAg ICBGcmFtZSogZnJhbWUgPSBtX3BhZ2VBZ2VudC0+ZmluZEZyYW1lV2l0aFNlY3VyaXR5T3JpZ2lu KHNlY3VyaXR5T3JpZ2luKTsKLSAgICBpZiAoIWZyYW1lKSB7CisgICAgdGFyZ2V0RnJhbWUgPSBt X3BhZ2VBZ2VudC0+ZmluZEZyYW1lV2l0aFNlY3VyaXR5T3JpZ2luKHNlY3VyaXR5T3JpZ2luKTsK KyAgICBpZiAoIXRhcmdldEZyYW1lKSB7CiAgICAgICAgIGlmIChlcnJvclN0cmluZykKICAgICAg ICAgICAgICplcnJvclN0cmluZyA9ICJGcmFtZSBub3QgZm91bmQgZm9yIHRoZSBnaXZlbiBzZWN1 cml0eSBvcmlnaW4iOwogICAgICAgICByZXR1cm4gMDsKICAgICB9Ci0gICAgdGFyZ2V0RnJhbWUg PSBmcmFtZTsKIAogICAgIFBhZ2UqIHBhZ2UgPSBtX3BhZ2VBZ2VudC0+cGFnZSgpOwogICAgIGlm IChpc0xvY2FsU3RvcmFnZSkKLSAgICAgICAgcmV0dXJuIHBhZ2UtPmdyb3VwKCkubG9jYWxTdG9y YWdlKCktPnN0b3JhZ2VBcmVhKGZyYW1lLT5kb2N1bWVudCgpLT5zZWN1cml0eU9yaWdpbigpKTsK LSAgICByZXR1cm4gcGFnZS0+c2Vzc2lvblN0b3JhZ2UoKS0+c3RvcmFnZUFyZWEoZnJhbWUtPmRv Y3VtZW50KCktPnNlY3VyaXR5T3JpZ2luKCkpOworICAgICAgICByZXR1cm4gcGFnZS0+Z3JvdXAo KS5sb2NhbFN0b3JhZ2UoKS0+c3RvcmFnZUFyZWEodGFyZ2V0RnJhbWUtPmRvY3VtZW50KCktPnNl Y3VyaXR5T3JpZ2luKCkpOworICAgIHJldHVybiBwYWdlLT5zZXNzaW9uU3RvcmFnZSgpLT5zdG9y YWdlQXJlYSh0YXJnZXRGcmFtZS0+ZG9jdW1lbnQoKS0+c2VjdXJpdHlPcmlnaW4oKSk7CiB9CiAK IH0gLy8gbmFtZXNwYWNlIFdlYkNvcmUK