Summary: | Add early return for empty path in makeAllDirectories | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sihui Liu <sihui_liu> | ||||||||||
Component: | New Bugs | Assignee: | Sihui Liu <sihui_liu> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, ews-watchlist, simon.fraser, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Sihui Liu
2022-03-07 10:53:17 PST
Created attachment 454002 [details]
Patch
Comment on attachment 454002 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454002&action=review > Source/WTF/ChangeLog:8 > + * wtf/posix/FileSystemPOSIX.cpp: Why no API test? Created attachment 454005 [details]
Patch
(In reply to Chris Dumez from comment #3) > Comment on attachment 454002 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=454002&action=review > > > Source/WTF/ChangeLog:8 > > + * wtf/posix/FileSystemPOSIX.cpp: > > Why no API test? Added Comment on attachment 454005 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454005&action=review > Source/WTF/wtf/posix/FileSystemPOSIX.cpp:280 > + if (!length) Why aren't we replacing the `fullPath.isNull()` early return above with a `fullPath.isEmpty()` instead? Is there a reason we want to do the access() call in between? (In reply to Chris Dumez from comment #6) > Comment on attachment 454005 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=454005&action=review > > > Source/WTF/wtf/posix/FileSystemPOSIX.cpp:280 > > + if (!length) > > Why aren't we replacing the `fullPath.isNull()` early return above with a > `fullPath.isEmpty()` instead? Is there a reason we want to do the access() > call in between? We can replace it there; this is safety net before accessing fullPath's content, so I added it here Comment on attachment 454005 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=454005&action=review >>> Source/WTF/wtf/posix/FileSystemPOSIX.cpp:280 >>> + if (!length) >> >> Why aren't we replacing the `fullPath.isNull()` early return above with a `fullPath.isEmpty()` instead? Is there a reason we want to do the access() call in between? > > We can replace it there; this is safety net before accessing fullPath's content, so I added it here Ok, then I'd prefer we do the check earlier like I suggested. r=me with that change. Created attachment 454018 [details]
Patch
Created attachment 454030 [details]
Patch
Committed r290978 (248156@main): <https://commits.webkit.org/248156@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 454030 [details]. |