Bug 237540

Summary: Add early return for empty path in makeAllDirectories
Product: WebKit Reporter: Sihui Liu <sihui_liu>
Component: New BugsAssignee: 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 Flags
Patch
none
Patch
none
Patch
ews-feeder: commit-queue-
Patch none

Sihui Liu
Reported 2022-03-07 10:53:17 PST
...
Attachments
Patch (1.30 KB, patch)
2022-03-07 10:58 PST, Sihui Liu 		no flags
DetailsFormatted DiffDiff
Patch (2.54 KB, patch)
2022-03-07 11:19 PST, Sihui Liu 		no flags
DetailsFormatted DiffDiff
Patch (2.62 KB, patch)
2022-03-07 12:59 PST, Sihui Liu 		ews-feeder: commit-queue-
DetailsFormatted DiffDiff
Patch (2.64 KB, patch)
2022-03-07 14:00 PST, Sihui Liu 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Sihui Liu
Comment 1 2022-03-07 10:58:12 PST
Created attachment 454002 [details] Patch
Sihui Liu
Comment 2 2022-03-07 10:59:15 PST
rdar://89901536
Chris Dumez
Comment 3 2022-03-07 11:08:40 PST
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?
Sihui Liu
Comment 4 2022-03-07 11:19:35 PST
Created attachment 454005 [details] Patch
Sihui Liu
Comment 5 2022-03-07 11:21:41 PST
(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
Chris Dumez
Comment 6 2022-03-07 12:31:34 PST
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?
Sihui Liu
Comment 7 2022-03-07 12:55:26 PST
(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
Chris Dumez
Comment 8 2022-03-07 12:56:17 PST
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.
Sihui Liu
Comment 9 2022-03-07 12:59:09 PST
Created attachment 454018 [details] Patch
Sihui Liu
Comment 10 2022-03-07 14:00:04 PST
Created attachment 454030 [details] Patch
EWS
Comment 11 2022-03-08 00:07:21 PST
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].
Note You need to log in before you can comment on or make changes to this bug.