42058 2010-07-12 01:00:42 -0700 [BREWMP] Don't pass PassOwnPtr in makeAllDirectories 2010-09-26 10:25:41 -0700 1 1 1 Unclassified WebKit Platform 528+ (Nightly build) Other Other RESOLVED FIXED P2 Normal --- 33564 0 skyul webkit-unassigned beergun commit-queue joybro201 xhiloh oldest_to_newest 249709 0 skyul 2010-07-12 01:00:42 -0700 PassOwnPtr's release is renamed to leakPtr. 249710 1 61194 skyul 2010-07-12 01:02:15 -0700 Created attachment 61194 Patch 250020 2 61194 darin 2010-07-12 13:44:03 -0700 Comment on attachment 61194 Patch But why do you want to leak a pointer here? If the makeAllDirectories function needs to take a file manager object it should take a PassOwnPtr. 250021 3 61194 darin 2010-07-12 13:44:46 -0700 Comment on attachment 61194 Patch Wait, no, this patch is wrong. 250022 4 61194 darin 2010-07-12 13:47:52 -0700 Comment on attachment 61194 Patch > @@ -144,7 +144,7 @@ static bool makeAllDirectories(PassOwnPt > } > > if (lastDivPos > 0) { > - if (!makeAllDirectories(fileMgr.release(), path.substring(0, lastDivPos))) > + if (!makeAllDirectories(fileMgr.leakPtr(), path.substring(0, lastDivPos))) > return false; This is buggy and needs a change in design. If this function fails, you'll be left with 0 in fileMgr and not be able to complete the function. Since makeAllDirectories takes a pointer to an object that it destroys, it can't call itself, and then go on to use that object. We'll need to refactor this code to find the mistake. You could get this to compile by just passing fileMgr here. That would change the behavior back to what it was before, but there is a bug here because fileMgr will be 0 once it's passed to another PassOwnPtr. > - return makeAllDirectories(fileMgr.release(), canonicalPath(path)); > + return makeAllDirectories(fileMgr.leakPtr(), canonicalPath(path)); This code was correct before. fileMgr here is an OwnPtr, and calling release on it gives you a PassOwnPtr, which is exactly what you need to call makeAllDirectories. 270511 5 65717 skyul 2010-08-27 08:40:02 -0700 Created attachment 65717 Patch Change makeAllDirectories to take a raw FileMgr pointer. I am not sure if this is the best way to avoid the problem. Any suggestion? 272584 6 65717 abarth 2010-08-31 19:41:10 -0700 Comment on attachment 65717 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=65717&action=prettypatch > WebCore/platform/brew/FileSystemBrew.cpp:134 > -static bool makeAllDirectories(PassOwnPtr<IFileMgr> fileMgr, const String& path) > +static bool makeAllDirectories(IFileMgr* rawFileMgr, const String& path) rawFileMgr isn't the right name here, for sure. Perhaps fileManager? I'd have to look at the larger context to see what the memory ownership is like here. 272841 7 66220 skyul 2010-09-01 09:27:20 -0700 Created attachment 66220 Patch Change rawFileMgr to fileManager as Adam suggested. 285121 8 66220 eric 2010-09-26 10:12:01 -0700 Comment on attachment 66220 Patch LGTM. 285127 9 66220 commit-queue 2010-09-26 10:25:35 -0700 Comment on attachment 66220 Patch Clearing flags on attachment: 66220 Committed r68342: <http://trac.webkit.org/changeset/68342> 285128 10 commit-queue 2010-09-26 10:25:41 -0700 All reviewed patches have been landed. Closing bug. 61194 2010-07-12 01:02:15 -0700 2010-08-27 08:40:02 -0700 Patch leakPtr.patch text/plain 1475 skyul SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA2MzA2MykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTUgQEAKKzIwMTAtMDctMTIgIEt3YW5nIFl1bCBTZW8gIDxza3l1bEBjb21wYW55 MTAwLm5ldD4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBbQlJFV01QXSBCdWlsZCBmaXg6IFVzZSBQYXNzT3duUHRyJ3MgbGVha1B0cgorICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9NDIwNTgKKworICAgICAgICBQ YXNzT3duUHRyJ3MgcmVsZWFzZSBpcyByZW5hbWVkIHRvIGxlYWtQdHIuCisKKyAgICAgICAgKiBw bGF0Zm9ybS9icmV3L0ZpbGVTeXN0ZW1CcmV3LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6Om1ha2VB bGxEaXJlY3Rvcmllcyk6CisKIDIwMTAtMDctMTEgIE1hY2llaiBTdGFjaG93aWFrICA8bWpzQGFw cGxlLmNvbT4KIAogICAgICAgICBSZXZpZXdlZCBieSBEYW4gQmVybnN0ZWluLgpJbmRleDogV2Vi Q29yZS9wbGF0Zm9ybS9icmV3L0ZpbGVTeXN0ZW1CcmV3LmNwcAo9PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJD b3JlL3BsYXRmb3JtL2JyZXcvRmlsZVN5c3RlbUJyZXcuY3BwCShyZXZpc2lvbiA2MzA2MikKKysr IFdlYkNvcmUvcGxhdGZvcm0vYnJldy9GaWxlU3lzdGVtQnJldy5jcHAJKHdvcmtpbmcgY29weSkK QEAgLTE0NCw3ICsxNDQsNyBAQCBzdGF0aWMgYm9vbCBtYWtlQWxsRGlyZWN0b3JpZXMoUGFzc093 blB0CiAgICAgfQogCiAgICAgaWYgKGxhc3REaXZQb3MgPiAwKSB7Ci0gICAgICAgIGlmICghbWFr ZUFsbERpcmVjdG9yaWVzKGZpbGVNZ3IucmVsZWFzZSgpLCBwYXRoLnN1YnN0cmluZygwLCBsYXN0 RGl2UG9zKSkpCisgICAgICAgIGlmICghbWFrZUFsbERpcmVjdG9yaWVzKGZpbGVNZ3IubGVha1B0 cigpLCBwYXRoLnN1YnN0cmluZygwLCBsYXN0RGl2UG9zKSkpCiAgICAgICAgICAgICByZXR1cm4g ZmFsc2U7CiAgICAgfQogCkBAIC0xNjUsNyArMTY1LDcgQEAgYm9vbCBtYWtlQWxsRGlyZWN0b3Jp ZXMoY29uc3QgU3RyaW5nJiBwYQogewogICAgIE93blB0cjxJRmlsZU1ncj4gZmlsZU1nciA9IGNy ZWF0ZUluc3RhbmNlPElGaWxlTWdyPihBRUVDTFNJRF9GSUxFTUdSKTsKIAotICAgIHJldHVybiBt YWtlQWxsRGlyZWN0b3JpZXMoZmlsZU1nci5yZWxlYXNlKCksIGNhbm9uaWNhbFBhdGgocGF0aCkp OworICAgIHJldHVybiBtYWtlQWxsRGlyZWN0b3JpZXMoZmlsZU1nci5sZWFrUHRyKCksIGNhbm9u aWNhbFBhdGgocGF0aCkpOwogfQogCiBTdHJpbmcgaG9tZURpcmVjdG9yeVBhdGgoKQo= 65717 2010-08-27 08:40:02 -0700 2010-09-01 09:27:20 -0700 Patch FileSystemBrew.patch text/plain 2460 skyul SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA2NjIyMykKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMTAtMDgtMjcgIEt3YW5nIFl1bCBTZW8gIDxza3l1bEBjb21wYW55 MTAwLm5ldD4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBbQlJFV01QXSBEb24ndCBwYXNzIFBhc3NPd25QdHIgaW4gbWFrZUFsbERpcmVjdG9yaWVzCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD00MjA1OAorCisg ICAgICAgIENoYW5nZSBtYWtlQWxsRGlyZWN0b3JpZXMgdG8gdGFrZSBhIHJhdyBGaWxlTWdyIHBv aW50ZXIKKyAgICAgICAgYmVjYXVzZSBpdCBpcyBub3Qgc2FmZSB0byBwYXNzIFBhc3NPd25QdHIg cmVjdXJzaXZlbHkuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9icmV3L0ZpbGVTeXN0ZW1CcmV3LmNw cDoKKyAgICAgICAgKFdlYkNvcmU6Om1ha2VBbGxEaXJlY3Rvcmllcyk6CisKIDIwMTAtMDgtMjcg IFBhdHJpY2sgR2Fuc3RlcmVyICA8cGFyb2dhQHBhcm9nYS5jb20+CiAKICAgICAgICAgUmV2aWV3 ZWQgYnkgTmlrb2xhcyBaaW1tZXJtYW5uLgpJbmRleDogV2ViQ29yZS9wbGF0Zm9ybS9icmV3L0Zp bGVTeXN0ZW1CcmV3LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL2JyZXcvRmls ZVN5c3RlbUJyZXcuY3BwCShyZXZpc2lvbiA2NjIxMSkKKysrIFdlYkNvcmUvcGxhdGZvcm0vYnJl dy9GaWxlU3lzdGVtQnJldy5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTEzMSw3ICsxMzEsNyBAQCBz dGF0aWMgU3RyaW5nIGNhbm9uaWNhbFBhdGgoY29uc3QgU3RyaW5nCiAgICAgcmV0dXJuIGNhbm9u UGF0aDsKIH0KIAotc3RhdGljIGJvb2wgbWFrZUFsbERpcmVjdG9yaWVzKFBhc3NPd25QdHI8SUZp bGVNZ3I+IGZpbGVNZ3IsIGNvbnN0IFN0cmluZyYgcGF0aCkKK3N0YXRpYyBib29sIG1ha2VBbGxE aXJlY3RvcmllcyhJRmlsZU1nciogcmF3RmlsZU1nciwgY29uc3QgU3RyaW5nJiBwYXRoKQogewog ICAgIGlmIChwYXRoID09IGNhbm9uaWNhbFBhdGgoQUVFRlNfSE9NRV9ESVIpKQogICAgICAgICBy ZXR1cm4gdHJ1ZTsKQEAgLTE0NCw3ICsxNDQsNyBAQCBzdGF0aWMgYm9vbCBtYWtlQWxsRGlyZWN0 b3JpZXMoUGFzc093blB0CiAgICAgfQogCiAgICAgaWYgKGxhc3REaXZQb3MgPiAwKSB7Ci0gICAg ICAgIGlmICghbWFrZUFsbERpcmVjdG9yaWVzKGZpbGVNZ3IucmVsZWFzZSgpLCBwYXRoLnN1YnN0 cmluZygwLCBsYXN0RGl2UG9zKSkpCisgICAgICAgIGlmICghbWFrZUFsbERpcmVjdG9yaWVzKHJh d0ZpbGVNZ3IsIHBhdGguc3Vic3RyaW5nKDAsIGxhc3REaXZQb3MpKSkKICAgICAgICAgICAgIHJl dHVybiBmYWxzZTsKICAgICB9CiAKQEAgLTE1MiwxMCArMTUyLDEwIEBAIHN0YXRpYyBib29sIG1h a2VBbGxEaXJlY3RvcmllcyhQYXNzT3duUHQKIAogICAgIC8vIElGSUxFTUdSX01rRGlyIHJldHVy biBTVUNDRVNTIHdoZW4gdGhlIGZpbGUgaXMgc3VjY2Vzc2Z1bGx5IGNyZWF0ZWQgb3IgaWYgZmls ZSBhbHJlYWR5IGV4aXN0cy4KICAgICAvLyBTbyB3ZSBuZWVkIHRvIGNoZWNrIGZpbGVpbmZvLmF0 dHJpYi4KLSAgICBJRklMRU1HUl9Na0RpcihmaWxlTWdyLmdldCgpLCBmb2xkZXIudXRmOCgpLmRh dGEoKSk7CisgICAgSUZJTEVNR1JfTWtEaXIocmF3RmlsZU1nciwgZm9sZGVyLnV0ZjgoKS5kYXRh KCkpOwogCiAgICAgRmlsZUluZm8gZmlsZUluZm87Ci0gICAgaWYgKElGSUxFTUdSX0dldEluZm8o ZmlsZU1nci5nZXQoKSwgZm9sZGVyLnV0ZjgoKS5kYXRhKCksICZmaWxlSW5mbykgIT0gU1VDQ0VT UykKKyAgICBpZiAoSUZJTEVNR1JfR2V0SW5mbyhyYXdGaWxlTWdyLCBmb2xkZXIudXRmOCgpLmRh dGEoKSwgJmZpbGVJbmZvKSAhPSBTVUNDRVNTKQogICAgICAgICByZXR1cm4gZmFsc2U7CiAKICAg ICByZXR1cm4gZmlsZUluZm8uYXR0cmliICYgX0ZBX0RJUjsKQEAgLTE2NSw3ICsxNjUsNyBAQCBi b29sIG1ha2VBbGxEaXJlY3Rvcmllcyhjb25zdCBTdHJpbmcmIHBhCiB7CiAgICAgT3duUHRyPElG aWxlTWdyPiBmaWxlTWdyID0gY3JlYXRlSW5zdGFuY2U8SUZpbGVNZ3I+KEFFRUNMU0lEX0ZJTEVN R1IpOwogCi0gICAgcmV0dXJuIG1ha2VBbGxEaXJlY3RvcmllcyhmaWxlTWdyLnJlbGVhc2UoKSwg Y2Fub25pY2FsUGF0aChwYXRoKSk7CisgICAgcmV0dXJuIG1ha2VBbGxEaXJlY3RvcmllcyhmaWxl TWdyLmdldCgpLCBjYW5vbmljYWxQYXRoKHBhdGgpKTsKIH0KIAogU3RyaW5nIGhvbWVEaXJlY3Rv cnlQYXRoKCkK 66220 2010-09-01 09:27:20 -0700 2010-09-26 10:25:35 -0700 Patch FileSystemBrew.patch text/plain 2461 skyul SW5kZXg6IFdlYkNvcmUvQ2hhbmdlTG9nCj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT0KLS0tIFdlYkNvcmUvQ2hhbmdlTG9n CShyZXZpc2lvbiA2NjYwOCkKKysrIFdlYkNvcmUvQ2hhbmdlTG9nCSh3b3JraW5nIGNvcHkpCkBA IC0xLDMgKzEsMTYgQEAKKzIwMTAtMDktMDEgIEt3YW5nIFl1bCBTZW8gIDxza3l1bEBjb21wYW55 MTAwLm5ldD4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICBbQlJFV01QXSBEb24ndCBwYXNzIFBhc3NPd25QdHIgaW4gbWFrZUFsbERpcmVjdG9yaWVzCisg ICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD00MjA1OAorCisg ICAgICAgIENoYW5nZSBtYWtlQWxsRGlyZWN0b3JpZXMgdG8gdGFrZSBhIHJhdyBGaWxlTWdyIHBv aW50ZXIKKyAgICAgICAgYmVjYXVzZSBpdCBpcyBub3Qgc2FmZSB0byBwYXNzIFBhc3NPd25QdHIg cmVjdXJzaXZlbHkuCisKKyAgICAgICAgKiBwbGF0Zm9ybS9icmV3L0ZpbGVTeXN0ZW1CcmV3LmNw cDoKKyAgICAgICAgKFdlYkNvcmU6Om1ha2VBbGxEaXJlY3Rvcmllcyk6CisKIDIwMTAtMDktMDEg IEFsZWphbmRybyBHLiBDYXN0cm8gIDxhbGV4QGlnYWxpYS5jb20+CiAKICAgICAgICAgUmV2aWV3 ZWQgYnkgTWFydGluIFJvYmluc29uLgpJbmRleDogV2ViQ29yZS9wbGF0Zm9ybS9icmV3L0ZpbGVT eXN0ZW1CcmV3LmNwcAo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBXZWJDb3JlL3BsYXRmb3JtL2JyZXcvRmlsZVN5 c3RlbUJyZXcuY3BwCShyZXZpc2lvbiA2NjUyNykKKysrIFdlYkNvcmUvcGxhdGZvcm0vYnJldy9G aWxlU3lzdGVtQnJldy5jcHAJKHdvcmtpbmcgY29weSkKQEAgLTEzMSw3ICsxMzEsNyBAQCBzdGF0 aWMgU3RyaW5nIGNhbm9uaWNhbFBhdGgoY29uc3QgU3RyaW5nCiAgICAgcmV0dXJuIGNhbm9uUGF0 aDsKIH0KIAotc3RhdGljIGJvb2wgbWFrZUFsbERpcmVjdG9yaWVzKFBhc3NPd25QdHI8SUZpbGVN Z3I+IGZpbGVNZ3IsIGNvbnN0IFN0cmluZyYgcGF0aCkKK3N0YXRpYyBib29sIG1ha2VBbGxEaXJl Y3RvcmllcyhJRmlsZU1nciogZmlsZU1hbmFnZXIsIGNvbnN0IFN0cmluZyYgcGF0aCkKIHsKICAg ICBpZiAocGF0aCA9PSBjYW5vbmljYWxQYXRoKEFFRUZTX0hPTUVfRElSKSkKICAgICAgICAgcmV0 dXJuIHRydWU7CkBAIC0xNDQsNyArMTQ0LDcgQEAgc3RhdGljIGJvb2wgbWFrZUFsbERpcmVjdG9y aWVzKFBhc3NPd25QdAogICAgIH0KIAogICAgIGlmIChsYXN0RGl2UG9zID4gMCkgewotICAgICAg ICBpZiAoIW1ha2VBbGxEaXJlY3RvcmllcyhmaWxlTWdyLnJlbGVhc2UoKSwgcGF0aC5zdWJzdHJp bmcoMCwgbGFzdERpdlBvcykpKQorICAgICAgICBpZiAoIW1ha2VBbGxEaXJlY3RvcmllcyhmaWxl TWFuYWdlciwgcGF0aC5zdWJzdHJpbmcoMCwgbGFzdERpdlBvcykpKQogICAgICAgICAgICAgcmV0 dXJuIGZhbHNlOwogICAgIH0KIApAQCAtMTUyLDEwICsxNTIsMTAgQEAgc3RhdGljIGJvb2wgbWFr ZUFsbERpcmVjdG9yaWVzKFBhc3NPd25QdAogCiAgICAgLy8gSUZJTEVNR1JfTWtEaXIgcmV0dXJu IFNVQ0NFU1Mgd2hlbiB0aGUgZmlsZSBpcyBzdWNjZXNzZnVsbHkgY3JlYXRlZCBvciBpZiBmaWxl IGFscmVhZHkgZXhpc3RzLgogICAgIC8vIFNvIHdlIG5lZWQgdG8gY2hlY2sgZmlsZWluZm8uYXR0 cmliLgotICAgIElGSUxFTUdSX01rRGlyKGZpbGVNZ3IuZ2V0KCksIGZvbGRlci51dGY4KCkuZGF0 YSgpKTsKKyAgICBJRklMRU1HUl9Na0RpcihmaWxlTWFuYWdlciwgZm9sZGVyLnV0ZjgoKS5kYXRh KCkpOwogCiAgICAgRmlsZUluZm8gZmlsZUluZm87Ci0gICAgaWYgKElGSUxFTUdSX0dldEluZm8o ZmlsZU1nci5nZXQoKSwgZm9sZGVyLnV0ZjgoKS5kYXRhKCksICZmaWxlSW5mbykgIT0gU1VDQ0VT UykKKyAgICBpZiAoSUZJTEVNR1JfR2V0SW5mbyhmaWxlTWFuYWdlciwgZm9sZGVyLnV0ZjgoKS5k YXRhKCksICZmaWxlSW5mbykgIT0gU1VDQ0VTUykKICAgICAgICAgcmV0dXJuIGZhbHNlOwogCiAg ICAgcmV0dXJuIGZpbGVJbmZvLmF0dHJpYiAmIF9GQV9ESVI7CkBAIC0xNjUsNyArMTY1LDcgQEAg Ym9vbCBtYWtlQWxsRGlyZWN0b3JpZXMoY29uc3QgU3RyaW5nJiBwYQogewogICAgIE93blB0cjxJ RmlsZU1ncj4gZmlsZU1nciA9IGNyZWF0ZUluc3RhbmNlPElGaWxlTWdyPihBRUVDTFNJRF9GSUxF TUdSKTsKIAotICAgIHJldHVybiBtYWtlQWxsRGlyZWN0b3JpZXMoZmlsZU1nci5yZWxlYXNlKCks IGNhbm9uaWNhbFBhdGgocGF0aCkpOworICAgIHJldHVybiBtYWtlQWxsRGlyZWN0b3JpZXMoZmls ZU1nci5nZXQoKSwgY2Fub25pY2FsUGF0aChwYXRoKSk7CiB9CiAKIFN0cmluZyBob21lRGlyZWN0 b3J5UGF0aCgpCg==