237477 2022-03-04 11:23:11 -0800 Add null check for path in makeAllDirectories 2022-03-04 22:04:18 -0800 1 1 1 Unclassified WebKit New Bugs WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 sihui_liu sihui_liu benjamin cdumez cmarcelo darin ews-watchlist ggaren webkit-bug-importer oldest_to_newest 1848355 0 sihui_liu 2022-03-04 11:23:11 -0800 ... 1848359 1 453859 sihui_liu 2022-03-04 11:30:39 -0800 Created attachment 453859 Patch 1848411 2 453859 cdumez 2022-03-04 14:30:06 -0800 Comment on attachment 453859 Patch Your test is failing on api-gtk. 1848433 3 453859 darin 2022-03-04 15:10:10 -0800 Comment on attachment 453859 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=453859&action=review > Source/WTF/wtf/posix/FileSystemPOSIX.cpp:280 > + LOG_ERROR("File failed to access. Error message: %s", safeStrerror(errno).data()); I think "file failed to access" is a vague message to put in a log, and not how I would describe a failure of the access function. If we are going to log this, then the message should make some mention of the fact that were were attempting to create directories, perhaps. Also wondering why we decided to log about the failure of this access call but not about the failures of the other access and mkdir calls below. Generally we want these file system abstraction functions like makeAllDirectories to not log directly to the console, so callers can make calls they know will or might fail without unconditionally cluttering logs. I understand that this conflicts with the desire to know what’s happening when diagnosing a bug. I’m not sure there is an easy answer. Generally speaking it would not be good if all file system calls logged what happened directly to the console, and certainly that’s not how the underlying POSIX functions behave. 1848445 4 453859 sihui_liu 2022-03-04 16:32:18 -0800 Comment on attachment 453859 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=453859&action=review >> Source/WTF/wtf/posix/FileSystemPOSIX.cpp:280 >> + LOG_ERROR("File failed to access. Error message: %s", safeStrerror(errno).data()); > > I think "file failed to access" is a vague message to put in a log, and not how I would describe a failure of the access function. If we are going to log this, then the message should make some mention of the fact that were were attempting to create directories, perhaps. Also wondering why we decided to log about the failure of this access call but not about the failures of the other access and mkdir calls below. > > Generally we want these file system abstraction functions like makeAllDirectories to not log directly to the console, so callers can make calls they know will or might fail without unconditionally cluttering logs. I understand that this conflicts with the desire to know what’s happening when diagnosing a bug. I’m not sure there is an easy answer. Generally speaking it would not be good if all file system calls logged what happened directly to the console, and certainly that’s not how the underlying POSIX functions behave. I added the log here because I realized that !access() does not necessarily mean file does not exist; it could be something more serious (like the fullPath is null case). (I planned to add early return on errno != ENOENT, instead of error log, but I am not sure if that would exclude some valid cases...) I think it's fair that we don't want to indroduce unnecessary logs for file system calls. Since there's no known issue here so far, I will remove this line. 1848448 5 453881 sihui_liu 2022-03-04 16:46:42 -0800 Created attachment 453881 Patch 1848505 6 ews-feeder 2022-03-04 22:03:23 -0800 Committed r290862 (248093@main): <https://commits.webkit.org/248093@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 453881. 1848506 7 webkit-bug-importer 2022-03-04 22:04:18 -0800 <rdar://problem/89848974> 453859 2022-03-04 11:30:39 -0800 2022-03-04 16:46:39 -0800 Patch bug-237477-20220304113039.patch text/plain 2947 sihui_liu U3VidmVyc2lvbiBSZXZpc2lvbjogMjkwNzg4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IDRkZmIwNDkwNGFjOGIyZWEwYTllYmM3 ZDVkZjUyMjIyYTgyMjhjNTAuLmJiNTI3ZjQ1MjVhNmM5NzQ2MDZkYzMwODExNDg2MTJmYzlmODdj MTMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTMgQEAKKzIwMjItMDMtMDQgIFNpaHVpIExpdSAgPHNpaHVpX2xp dUBhcHBsZS5jb20+CisKKyAgICAgICAgQWRkIG51bGwgY2hlY2sgZm9yIHBhdGggaW4gbWFrZUFs bERpcmVjdG9yaWVzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0yMzc0NzcKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAg ICAgICAqIHd0Zi9wb3NpeC9GaWxlU3lzdGVtUE9TSVguY3BwOgorICAgICAgICAoV1RGOjpGaWxl U3lzdGVtSW1wbDo6bWFrZUFsbERpcmVjdG9yaWVzKToKKwogMjAyMi0wMy0wMiAgQWxleCBDaHJp c3RlbnNlbiAgPGFjaHJpc3RlbnNlbkB3ZWJraXQub3JnPgogCiAgICAgICAgIEFkZCBTUEkgX1dL RGF0YVRhc2sKZGlmZiAtLWdpdCBhL1NvdXJjZS9XVEYvd3RmL3Bvc2l4L0ZpbGVTeXN0ZW1QT1NJ WC5jcHAgYi9Tb3VyY2UvV1RGL3d0Zi9wb3NpeC9GaWxlU3lzdGVtUE9TSVguY3BwCmluZGV4IGYx MjEyNTM4NzYzNDUzNTQxOTI0M2Q4MWVjNmM0ZmYwYjFiZjAzYjIuLjU1Y2Y1YmRmZTYwZjM1YTFj ZTRmMWQ4MDYxNGY1YzBlZmY2NjI0MjggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3RmL3Bvc2l4 L0ZpbGVTeXN0ZW1QT1NJWC5jcHAKKysrIGIvU291cmNlL1dURi93dGYvcG9zaXgvRmlsZVN5c3Rl bVBPU0lYLmNwcApAQCAtMjcwLDkgKzI3MCwxNSBAQCBib29sIGRlbGV0ZUZpbGUoY29uc3QgU3Ry aW5nJiBwYXRoKQogYm9vbCBtYWtlQWxsRGlyZWN0b3JpZXMoY29uc3QgU3RyaW5nJiBwYXRoKQog ewogICAgIGF1dG8gZnVsbFBhdGggPSBmaWxlU3lzdGVtUmVwcmVzZW50YXRpb24ocGF0aCk7Cisg ICAgaWYgKGZ1bGxQYXRoLmlzTnVsbCgpKQorICAgICAgICByZXR1cm4gZmFsc2U7CisKICAgICBp ZiAoIWFjY2VzcyhmdWxsUGF0aC5kYXRhKCksIEZfT0spKQogICAgICAgICByZXR1cm4gdHJ1ZTsK IAorICAgIGlmIChlcnJubyAhPSBFTk9FTlQpCisgICAgICAgIExPR19FUlJPUigiRmlsZSBmYWls ZWQgdG8gYWNjZXNzLiBFcnJvciBtZXNzYWdlOiAlcyIsIHNhZmVTdHJlcnJvcihlcnJubykuZGF0 YSgpKTsKKyAgICAKICAgICBjaGFyKiBwID0gZnVsbFBhdGgubXV0YWJsZURhdGEoKSArIDE7CiAg ICAgaW50IGxlbmd0aCA9IGZ1bGxQYXRoLmxlbmd0aCgpOwogICAgIGlmIChwW2xlbmd0aCAtIDFd ID09ICcvJykKZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwpp bmRleCBmMGI1MGNmMzdjM2RjOWMxYzUxYzQxOTk1MDE1N2ZiMDQ3YWFjZjE5Li5iN2RlYzk2YzU1 ZjZmODhhM2VjOTc4YTIxZDE4NTUwMDhmM2UzZWU1IDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VM b2cKKysrIGIvVG9vbHMvQ2hhbmdlTG9nCkBAIC0xLDMgKzEsMTMgQEAKKzIwMjItMDMtMDQgIFNp aHVpIExpdSAgPHNpaHVpX2xpdUBhcHBsZS5jb20+CisKKyAgICAgICAgQWRkIG51bGwgY2hlY2sg Zm9yIHBhdGggaW4gbWFrZUFsbERpcmVjdG9yaWVzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD0yMzc0NzcKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JP RFkgKE9PUFMhKS4KKworICAgICAgICAqIFRlc3RXZWJLaXRBUEkvVGVzdHMvV1RGL0ZpbGVTeXN0 ZW0uY3BwOgorICAgICAgICAoVGVzdFdlYktpdEFQSTo6VEVTVF9GKToKKwogMjAyMi0wMy0wMyAg TXlsZXMgQy4gTWF4ZmllbGQgIDxtbWF4ZmllbGRAYXBwbGUuY29tPgogCiAgICAgICAgIFtTdHls ZV0gRm9yYmlkIHNwYWNlcyBiZXR3ZWVuIHR5cGUgbmFtZXMgYW5kIHByb3RvY29sIG5hbWVzIGlu IE9iamVjdGl2ZS1DCmRpZmYgLS1naXQgYS9Ub29scy9UZXN0V2ViS2l0QVBJL1Rlc3RzL1dURi9G aWxlU3lzdGVtLmNwcCBiL1Rvb2xzL1Rlc3RXZWJLaXRBUEkvVGVzdHMvV1RGL0ZpbGVTeXN0ZW0u Y3BwCmluZGV4IDg2ZTVhNGIzZjYzOGEzOGYwZjE4Y2ExMmQ2NmQ1MjkxMDAyZTMyODQuLjE3YzIy ZjA5MGRiNWE2MmVlMGE4Yzc4MWFhMTZkMDBjMzY4MzI0ZjUgMTAwNjQ0Ci0tLSBhL1Rvb2xzL1Rl c3RXZWJLaXRBUEkvVGVzdHMvV1RGL0ZpbGVTeXN0ZW0uY3BwCisrKyBiL1Rvb2xzL1Rlc3RXZWJL aXRBUEkvVGVzdHMvV1RGL0ZpbGVTeXN0ZW0uY3BwCkBAIC00ODUsNiArNDg1LDEwIEBAIFRFU1Rf RihGaWxlU3lzdGVtVGVzdCwgbWFrZUFsbERpcmVjdG9yaWVzKQogICAgIEVYUEVDVF9FUShGaWxl U3lzdGVtOjpmaWxlVHlwZShzdWJGb2xkZXJQYXRoKSwgRmlsZVN5c3RlbTo6RmlsZVR5cGU6OkRp cmVjdG9yeSk7CiAgICAgRVhQRUNUX1RSVUUoRmlsZVN5c3RlbTo6ZGVsZXRlTm9uRW1wdHlEaXJl Y3RvcnkodGVtcEVtcHR5Rm9sZGVyUGF0aCgpKSk7CiAgICAgRVhQRUNUX0ZBTFNFKEZpbGVTeXN0 ZW06OmZpbGVFeGlzdHMoc3ViRm9sZGVyUGF0aCkpOworICAgIFN0cmluZyBpbnZhbGlkRm9sZGVy UGF0aCA9IEZpbGVTeXN0ZW06OnBhdGhCeUFwcGVuZGluZ0NvbXBvbmVudCh0ZW1wRW1wdHlGb2xk ZXJQYXRoKCksICJpbnZhbGlkIik7CisgICAgaW52YWxpZEZvbGRlclBhdGguYXBwZW5kKCdcMCcp OworICAgIGludmFsaWRGb2xkZXJQYXRoLmFwcGVuZCgnYScpOworICAgIEVYUEVDVF9GQUxTRShG aWxlU3lzdGVtOjptYWtlQWxsRGlyZWN0b3JpZXMoaW52YWxpZEZvbGRlclBhdGgpKTsKIH0KIAog VEVTVF9GKEZpbGVTeXN0ZW1UZXN0LCB2b2x1bWVGcmVlU3BhY2UpCg== 453881 2022-03-04 16:46:42 -0800 2022-03-04 22:03:25 -0800 Patch bug-237477-20220304164641.patch text/plain 2632 sihui_liu U3VidmVyc2lvbiBSZXZpc2lvbjogMjkwNzg4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IDRkZmIwNDkwNGFjOGIyZWEwYTllYmM3 ZDVkZjUyMjIyYTgyMjhjNTAuLjE4NGZmMTU4M2EzMGUyYzFiNmM4OGQ2NzVkODVhZDkxYjBmMjE2 NjYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTMgQEAKKzIwMjItMDMtMDQgIFNpaHVpIExpdSAgPHNpaHVpX2xp dUBhcHBsZS5jb20+CisKKyAgICAgICAgQWRkIG51bGwgY2hlY2sgZm9yIHBhdGggaW4gbWFrZUFs bERpcmVjdG9yaWVzCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD0yMzc0NzcKKworICAgICAgICBSZXZpZXdlZCBieSBEYXJpbiBBZGxlci4KKworICAgICAg ICAqIHd0Zi9wb3NpeC9GaWxlU3lzdGVtUE9TSVguY3BwOgorICAgICAgICAoV1RGOjpGaWxlU3lz dGVtSW1wbDo6bWFrZUFsbERpcmVjdG9yaWVzKToKKwogMjAyMi0wMy0wMiAgQWxleCBDaHJpc3Rl bnNlbiAgPGFjaHJpc3RlbnNlbkB3ZWJraXQub3JnPgogCiAgICAgICAgIEFkZCBTUEkgX1dLRGF0 YVRhc2sKZGlmZiAtLWdpdCBhL1NvdXJjZS9XVEYvd3RmL3Bvc2l4L0ZpbGVTeXN0ZW1QT1NJWC5j cHAgYi9Tb3VyY2UvV1RGL3d0Zi9wb3NpeC9GaWxlU3lzdGVtUE9TSVguY3BwCmluZGV4IGYxMjEy NTM4NzYzNDUzNTQxOTI0M2Q4MWVjNmM0ZmYwYjFiZjAzYjIuLmFlMGE3MDZmNTY1MWQ1MWI4Mzli MTg5YWRkZjQ4NWYwNDQxNDdmZTkgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvd3RmL3Bvc2l4L0Zp bGVTeXN0ZW1QT1NJWC5jcHAKKysrIGIvU291cmNlL1dURi93dGYvcG9zaXgvRmlsZVN5c3RlbVBP U0lYLmNwcApAQCAtMjcwLDYgKzI3MCw5IEBAIGJvb2wgZGVsZXRlRmlsZShjb25zdCBTdHJpbmcm IHBhdGgpCiBib29sIG1ha2VBbGxEaXJlY3Rvcmllcyhjb25zdCBTdHJpbmcmIHBhdGgpCiB7CiAg ICAgYXV0byBmdWxsUGF0aCA9IGZpbGVTeXN0ZW1SZXByZXNlbnRhdGlvbihwYXRoKTsKKyAgICBp ZiAoZnVsbFBhdGguaXNOdWxsKCkpCisgICAgICAgIHJldHVybiBmYWxzZTsKKwogICAgIGlmICgh YWNjZXNzKGZ1bGxQYXRoLmRhdGEoKSwgRl9PSykpCiAgICAgICAgIHJldHVybiB0cnVlOwogCmRp ZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cgYi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggZjBiNTBj ZjM3YzNkYzljMWM1MWM0MTk5NTAxNTdmYjA0N2FhY2YxOS4uYzJjNDk3YjgyMzYyMTk1MDM1YmQx M2NkYjJmZjI1NDZlMzI5NTM2MyAxMDA2NDQKLS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rv b2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDEzIEBACisyMDIyLTAzLTA0ICBTaWh1aSBMaXUgIDxz aWh1aV9saXVAYXBwbGUuY29tPgorCisgICAgICAgIEFkZCBudWxsIGNoZWNrIGZvciBwYXRoIGlu IG1ha2VBbGxEaXJlY3RvcmllcworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9MjM3NDc3CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgRGFyaW4gQWRsZXIuCisK KyAgICAgICAgKiBUZXN0V2ViS2l0QVBJL1Rlc3RzL1dURi9GaWxlU3lzdGVtLmNwcDoKKyAgICAg ICAgKFRlc3RXZWJLaXRBUEk6OlRFU1RfRik6CisKIDIwMjItMDMtMDMgIE15bGVzIEMuIE1heGZp ZWxkICA8bW1heGZpZWxkQGFwcGxlLmNvbT4KIAogICAgICAgICBbU3R5bGVdIEZvcmJpZCBzcGFj ZXMgYmV0d2VlbiB0eXBlIG5hbWVzIGFuZCBwcm90b2NvbCBuYW1lcyBpbiBPYmplY3RpdmUtQwpk aWZmIC0tZ2l0IGEvVG9vbHMvVGVzdFdlYktpdEFQSS9UZXN0cy9XVEYvRmlsZVN5c3RlbS5jcHAg Yi9Ub29scy9UZXN0V2ViS2l0QVBJL1Rlc3RzL1dURi9GaWxlU3lzdGVtLmNwcAppbmRleCA4NmU1 YTRiM2Y2MzhhMzhmMGYxOGNhMTJkNjZkNTI5MTAwMmUzMjg0Li5lZWI4NTA1YzM2YTBlNzhkZWQx MmFkOTdjNTAxNzE4OTViMjE0MmZmIDEwMDY0NAotLS0gYS9Ub29scy9UZXN0V2ViS2l0QVBJL1Rl c3RzL1dURi9GaWxlU3lzdGVtLmNwcAorKysgYi9Ub29scy9UZXN0V2ViS2l0QVBJL1Rlc3RzL1dU Ri9GaWxlU3lzdGVtLmNwcApAQCAtNDg1LDYgKzQ4NSwxMCBAQCBURVNUX0YoRmlsZVN5c3RlbVRl c3QsIG1ha2VBbGxEaXJlY3RvcmllcykKICAgICBFWFBFQ1RfRVEoRmlsZVN5c3RlbTo6ZmlsZVR5 cGUoc3ViRm9sZGVyUGF0aCksIEZpbGVTeXN0ZW06OkZpbGVUeXBlOjpEaXJlY3RvcnkpOwogICAg IEVYUEVDVF9UUlVFKEZpbGVTeXN0ZW06OmRlbGV0ZU5vbkVtcHR5RGlyZWN0b3J5KHRlbXBFbXB0 eUZvbGRlclBhdGgoKSkpOwogICAgIEVYUEVDVF9GQUxTRShGaWxlU3lzdGVtOjpmaWxlRXhpc3Rz KHN1YkZvbGRlclBhdGgpKTsKKyAgICBTdHJpbmcgaW52YWxpZEZvbGRlclBhdGg7CisgICAgaW52 YWxpZEZvbGRlclBhdGguYXBwZW5kKCdcMCcpOworICAgIGludmFsaWRGb2xkZXJQYXRoLmFwcGVu ZCgnYScpOworICAgIEVYUEVDVF9GQUxTRShGaWxlU3lzdGVtOjptYWtlQWxsRGlyZWN0b3JpZXMo aW52YWxpZEZvbGRlclBhdGgpKTsKIH0KIAogVEVTVF9GKEZpbGVTeXN0ZW1UZXN0LCB2b2x1bWVG cmVlU3BhY2UpCg==