190865 2018-10-24 01:01:29 -0700 [WPE][GTK] Cleanups to the certificate encoder 2018-10-24 07:09:56 -0700 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 csaavedra csaavedra mcatanzaro webkit-bug-importer zan oldest_to_newest 1471787 0 csaavedra 2018-10-24 01:01:29 -0700 [WPE][GTK] Remove redundant variable from certificate encoder 1471788 1 353027 csaavedra 2018-10-24 01:01:46 -0700 Created attachment 353027 Patch 1471790 2 353027 zan 2018-10-24 01:26:05 -0700 Comment on attachment 353027 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353027&action=review > Source/WebKit/Shared/soup/WebCoreArgumentCodersSoup.cpp:61 > GByteArray* certificateData = 0; > Vector<GByteArray*> certificatesDataList; Move `certificateData` into the `do { }` scope, limiting its use to that part of the code. `certificatesDataList` should IMO be a Vector<GRefPtr<GByteArray>>, immediately adopting the `certificateData` pointer and avoiding the out-of-the-blue adoptGRef() int the loop below. > Source/WebKit/Shared/soup/WebCoreArgumentCodersSoup.cpp:80 > + for (int i = certificatesDataList.size(); i > 0; i--) { Use size_t for `i`. In the post-loop segment, use the prefixed decrement: `--i`. 1471791 3 353028 csaavedra 2018-10-24 01:51:40 -0700 Created attachment 353028 Patch 1471792 4 353028 zan 2018-10-24 01:56:15 -0700 Comment on attachment 353028 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353028&action=review > Source/WebKit/Shared/soup/WebCoreArgumentCodersSoup.cpp:63 > + GByteArray* certificateData = 0; > g_object_get(G_OBJECT(certificate), "certificate", &certificateData, NULL); `nullptr` should be used in both lines. > Source/WebKit/Shared/soup/WebCoreArgumentCodersSoup.cpp:80 > + GRefPtr<GByteArray> certificate = certificatesDataList[i - 1]; This takes an extra reference to the GByteArray pointer stored in the Vector. Use auto& to bind a C++ reference to the existing GRefPtr instead: `auto& certificate = ...` 1471795 5 csaavedra 2018-10-24 02:10:42 -0700 Committed r237377: <https://trac.webkit.org/changeset/237377> 1471796 6 webkit-bug-importer 2018-10-24 02:11:27 -0700 <rdar://problem/45514258> 1471816 7 353028 mcatanzaro 2018-10-24 07:06:08 -0700 Comment on attachment 353028 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=353028&action=review > Source/WebKit/Shared/soup/WebCoreArgumentCodersSoup.cpp:68 > certificatesDataList.append(certificateData); I think you need adoptGRef() here to avoid leaking it, right? >> Source/WebKit/Shared/soup/WebCoreArgumentCodersSoup.cpp:80 >> + GRefPtr<GByteArray> certificate = certificatesDataList[i - 1]; > > This takes an extra reference to the GByteArray pointer stored in the Vector. Use auto& to bind a C++ reference to the existing GRefPtr instead: > `auto& certificate = ...` It's less confusing if you bypass the GRefPtr (because you don't need to hold a ref here) and just go straight to the GByteArray: GByteArray* certificate = certificatesDataList[i - 1].get(); 1471819 8 mcatanzaro 2018-10-24 07:09:56 -0700 Oh I missed that you already committed (cq? was still set, try not to forget about that). Anyway, my second comment isn't important, but please check the first one to make sure you're not leaking. 353027 2018-10-24 01:01:46 -0700 2018-10-24 01:51:36 -0700 Patch bug-190865-20181024110145.patch text/plain 2260 csaavedra U3VidmVyc2lvbiBSZXZpc2lvbjogMjM3Mzc1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDQ2NGNmOTg1ODM5MWE2YWZm NTUyNDQ1ODU2NzBjMGRmYWZlNGJhZjcuLmUzNTQwODgzOTFlZTU4MzcwZWFmZjE0MTE4Y2ExYjBj ZDI5MzBiZGMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTgtMTAtMjQgIENsYXVkaW8g U2FhdmVkcmEgIDxjc2FhdmVkcmFAaWdhbGlhLmNvbT4KKworICAgICAgICBbV1BFXVtHVEtdIFJl bW92ZSByZWR1bmRhbnQgdmFyaWFibGUgZnJvbSBjZXJ0aWZpY2F0ZSBlbmNvZGVyCisgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xOTA4NjUKKworICAgICAg ICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFNoYXJlZC9zb3VwL1dl YkNvcmVBcmd1bWVudENvZGVyc1NvdXAuY3BwOgorICAgICAgICAoSVBDOjpBcmd1bWVudENvZGVy PENlcnRpZmljYXRlSW5mbz46OmVuY29kZSk6IHJlbW92ZQorICAgICAgICByZWR1bmRhbnQgbGVu Z3RoIHZhcmlhYmxlIGFuZCB1c2UgdmVjdG9yJ3Mgc2l6ZSBpbnN0ZWFkLgorCiAyMDE4LTEwLTIz ICBSeWFuIEhhZGRhZCAgPHJ5YW5oYWRkYWRAYXBwbGUuY29tPgogCiAgICAgICAgIFVucmV2aWV3 ZWQsIHJvbGxpbmcgb3V0IHIyMzcyNjEuCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L1NoYXJl ZC9zb3VwL1dlYkNvcmVBcmd1bWVudENvZGVyc1NvdXAuY3BwIGIvU291cmNlL1dlYktpdC9TaGFy ZWQvc291cC9XZWJDb3JlQXJndW1lbnRDb2RlcnNTb3VwLmNwcAppbmRleCBlMTEwOTIwM2JkYWM4 NGRkYzU2MzhlYzliYzk1YmQyYTEyYTc2NDYxLi5kNzk0ZTFkZmY4Y2MzZjc1MDA5Nzk1ZWJlOWRj ZGVjN2UzYzMxNzQ2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViS2l0L1NoYXJlZC9zb3VwL1dlYkNv cmVBcmd1bWVudENvZGVyc1NvdXAuY3BwCisrKyBiL1NvdXJjZS9XZWJLaXQvU2hhcmVkL3NvdXAv V2ViQ29yZUFyZ3VtZW50Q29kZXJzU291cC5jcHAKQEAgLTU2LDcgKzU2LDYgQEAgdm9pZCBBcmd1 bWVudENvZGVyPENlcnRpZmljYXRlSW5mbz46OmVuY29kZShFbmNvZGVyJiBlbmNvZGVyLCBjb25z dCBDZXJ0aWZpY2F0ZUkKICAgICAgICAgcmV0dXJuOwogICAgIH0KIAotICAgIHVpbnQzMl90IGNo YWluTGVuZ3RoID0gMDsKICAgICBHVGxzQ2VydGlmaWNhdGUqIGNlcnRpZmljYXRlID0gY2VydGlm aWNhdGVJbmZvLmNlcnRpZmljYXRlKCk7CiAgICAgR0J5dGVBcnJheSogY2VydGlmaWNhdGVEYXRh ID0gMDsKICAgICBWZWN0b3I8R0J5dGVBcnJheSo+IGNlcnRpZmljYXRlc0RhdGFMaXN0OwpAQCAt NjgsMTggKzY3LDE3IEBAIHZvaWQgQXJndW1lbnRDb2RlcjxDZXJ0aWZpY2F0ZUluZm8+OjplbmNv ZGUoRW5jb2RlciYgZW5jb2RlciwgY29uc3QgQ2VydGlmaWNhdGVJCiAgICAgICAgICAgICBicmVh azsKIAogICAgICAgICBjZXJ0aWZpY2F0ZXNEYXRhTGlzdC5hcHBlbmQoY2VydGlmaWNhdGVEYXRh KTsKLSAgICAgICAgY2hhaW5MZW5ndGgrKzsKIAogICAgICAgICBjZXJ0aWZpY2F0ZSA9IGdfdGxz X2NlcnRpZmljYXRlX2dldF9pc3N1ZXIoY2VydGlmaWNhdGUpOwogICAgIH0gd2hpbGUgKGNlcnRp ZmljYXRlKTsKIAotICAgIGVuY29kZXIgPDwgY2hhaW5MZW5ndGg7CisgICAgZW5jb2RlciA8PCBz dGF0aWNfY2FzdDx1aW50MzJfdD4oY2VydGlmaWNhdGVzRGF0YUxpc3Quc2l6ZSgpKTsKIAotICAg IGlmICghY2hhaW5MZW5ndGgpCisgICAgaWYgKGNlcnRpZmljYXRlc0RhdGFMaXN0LmlzRW1wdHko KSkKICAgICAgICAgcmV0dXJuOwogCiAgICAgLy8gRW5jb2RlIHN0YXJ0aW5nIGZyb20gdGhlIHJv b3QgY2VydGlmaWNhdGUuCi0gICAgZm9yICh1aW50MzJfdCBpID0gY2hhaW5MZW5ndGg7IGkgPiAw OyBpLS0pIHsKKyAgICBmb3IgKGludCBpID0gY2VydGlmaWNhdGVzRGF0YUxpc3Quc2l6ZSgpOyBp ID4gMDsgaS0tKSB7CiAgICAgICAgIEdSZWZQdHI8R0J5dGVBcnJheT4gY2VydGlmaWNhdGUgPSBh ZG9wdEdSZWYoY2VydGlmaWNhdGVzRGF0YUxpc3RbaSAtIDFdKTsKICAgICAgICAgZW5jb2Rlci5l bmNvZGVWYXJpYWJsZUxlbmd0aEJ5dGVBcnJheShJUEM6OkRhdGFSZWZlcmVuY2UoY2VydGlmaWNh dGUtPmRhdGEsIGNlcnRpZmljYXRlLT5sZW4pKTsKICAgICB9Cg== 353028 2018-10-24 01:51:40 -0700 2018-10-24 07:06:08 -0700 Patch bug-190865-20181024115138.patch text/plain 2377 csaavedra U3VidmVyc2lvbiBSZXZpc2lvbjogMjM3Mzc1CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViS2l0L0No YW5nZUxvZyBiL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCmluZGV4IDQ2NGNmOTg1ODM5MWE2YWZm NTUyNDQ1ODU2NzBjMGRmYWZlNGJhZjcuLjFiZjFjNWJlOWZlYzE5NjVkZmRhNmQzNjIwMjJkMjA1 ZmZkMDg1ZGYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJLaXQvQ2hhbmdlTG9nCisrKyBiL1NvdXJj ZS9XZWJLaXQvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMgQEAKKzIwMTgtMTAtMjQgIENsYXVkaW8g U2FhdmVkcmEgIDxjc2FhdmVkcmFAaWdhbGlhLmNvbT4KKworICAgICAgICBbV1BFXVtHVEtdIENs ZWFudXBzIHRvIHRoZSBjZXJ0aWZpY2F0ZSBlbmNvZGVyCisgICAgICAgIGh0dHBzOi8vYnVncy53 ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xOTA4NjUKKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIFNoYXJlZC9zb3VwL1dlYkNvcmVBcmd1bWVudENv ZGVyc1NvdXAuY3BwOgorICAgICAgICAoSVBDOjpBcmd1bWVudENvZGVyPENlcnRpZmljYXRlSW5m bz46OmVuY29kZSk6CisKIDIwMTgtMTAtMjMgIFJ5YW4gSGFkZGFkICA8cnlhbmhhZGRhZEBhcHBs ZS5jb20+CiAKICAgICAgICAgVW5yZXZpZXdlZCwgcm9sbGluZyBvdXQgcjIzNzI2MS4KZGlmZiAt LWdpdCBhL1NvdXJjZS9XZWJLaXQvU2hhcmVkL3NvdXAvV2ViQ29yZUFyZ3VtZW50Q29kZXJzU291 cC5jcHAgYi9Tb3VyY2UvV2ViS2l0L1NoYXJlZC9zb3VwL1dlYkNvcmVBcmd1bWVudENvZGVyc1Nv dXAuY3BwCmluZGV4IGUxMTA5MjAzYmRhYzg0ZGRjNTYzOGVjOWJjOTViZDJhMTJhNzY0NjEuLjRk MThkMzU4NDg3NDViOTJmYWViNzI3ZTZlNDFlNTJlNjA3YWVjZWEgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9XZWJLaXQvU2hhcmVkL3NvdXAvV2ViQ29yZUFyZ3VtZW50Q29kZXJzU291cC5jcHAKKysrIGIv U291cmNlL1dlYktpdC9TaGFyZWQvc291cC9XZWJDb3JlQXJndW1lbnRDb2RlcnNTb3VwLmNwcApA QCAtNTYsMzEgKzU2LDI4IEBAIHZvaWQgQXJndW1lbnRDb2RlcjxDZXJ0aWZpY2F0ZUluZm8+Ojpl bmNvZGUoRW5jb2RlciYgZW5jb2RlciwgY29uc3QgQ2VydGlmaWNhdGVJCiAgICAgICAgIHJldHVy bjsKICAgICB9CiAKLSAgICB1aW50MzJfdCBjaGFpbkxlbmd0aCA9IDA7CiAgICAgR1Rsc0NlcnRp ZmljYXRlKiBjZXJ0aWZpY2F0ZSA9IGNlcnRpZmljYXRlSW5mby5jZXJ0aWZpY2F0ZSgpOwotICAg IEdCeXRlQXJyYXkqIGNlcnRpZmljYXRlRGF0YSA9IDA7Ci0gICAgVmVjdG9yPEdCeXRlQXJyYXkq PiBjZXJ0aWZpY2F0ZXNEYXRhTGlzdDsKLQorICAgIFZlY3RvcjxHUmVmUHRyPEdCeXRlQXJyYXk+ PiBjZXJ0aWZpY2F0ZXNEYXRhTGlzdDsKICAgICBkbyB7CisgICAgICAgIEdCeXRlQXJyYXkqIGNl cnRpZmljYXRlRGF0YSA9IDA7CiAgICAgICAgIGdfb2JqZWN0X2dldChHX09CSkVDVChjZXJ0aWZp Y2F0ZSksICJjZXJ0aWZpY2F0ZSIsICZjZXJ0aWZpY2F0ZURhdGEsIE5VTEwpOwogCiAgICAgICAg IGlmICghY2VydGlmaWNhdGVEYXRhKQogICAgICAgICAgICAgYnJlYWs7CiAKICAgICAgICAgY2Vy dGlmaWNhdGVzRGF0YUxpc3QuYXBwZW5kKGNlcnRpZmljYXRlRGF0YSk7Ci0gICAgICAgIGNoYWlu TGVuZ3RoKys7CiAKICAgICAgICAgY2VydGlmaWNhdGUgPSBnX3Rsc19jZXJ0aWZpY2F0ZV9nZXRf aXNzdWVyKGNlcnRpZmljYXRlKTsKICAgICB9IHdoaWxlIChjZXJ0aWZpY2F0ZSk7CiAKLSAgICBl bmNvZGVyIDw8IGNoYWluTGVuZ3RoOworICAgIGVuY29kZXIgPDwgc3RhdGljX2Nhc3Q8dWludDMy X3Q+KGNlcnRpZmljYXRlc0RhdGFMaXN0LnNpemUoKSk7CiAKLSAgICBpZiAoIWNoYWluTGVuZ3Ro KQorICAgIGlmIChjZXJ0aWZpY2F0ZXNEYXRhTGlzdC5pc0VtcHR5KCkpCiAgICAgICAgIHJldHVy bjsKIAogICAgIC8vIEVuY29kZSBzdGFydGluZyBmcm9tIHRoZSByb290IGNlcnRpZmljYXRlLgot ICAgIGZvciAodWludDMyX3QgaSA9IGNoYWluTGVuZ3RoOyBpID4gMDsgaS0tKSB7Ci0gICAgICAg IEdSZWZQdHI8R0J5dGVBcnJheT4gY2VydGlmaWNhdGUgPSBhZG9wdEdSZWYoY2VydGlmaWNhdGVz RGF0YUxpc3RbaSAtIDFdKTsKKyAgICBmb3IgKHNpemVfdCBpID0gY2VydGlmaWNhdGVzRGF0YUxp c3Quc2l6ZSgpOyBpID4gMDsgLS1pKSB7CisgICAgICAgIEdSZWZQdHI8R0J5dGVBcnJheT4gY2Vy dGlmaWNhdGUgPSBjZXJ0aWZpY2F0ZXNEYXRhTGlzdFtpIC0gMV07CiAgICAgICAgIGVuY29kZXIu ZW5jb2RlVmFyaWFibGVMZW5ndGhCeXRlQXJyYXkoSVBDOjpEYXRhUmVmZXJlbmNlKGNlcnRpZmlj YXRlLT5kYXRhLCBjZXJ0aWZpY2F0ZS0+bGVuKSk7CiAgICAgfQogCg==