Skip to content

Commit

Permalink
Add fs.path.ComponentIterator and use it in Dir.makePath
Browse files Browse the repository at this point in the history
Before this commit, there were three issues with the makePath implementation:

1. The component iteration did not 'collapse' consecutive path separators; instead, it would treat `a/b//c` as `a/b//c`, `a/b/`, `a/b`, and `a`.
2. Trailing path separators led to redundant `makeDir` calls, e.g. with the path `a/b/` (if `a` doesn't exist), it would try to create `a/b/`, then try `a/b`, then try `a`, then try `a/b`, and finally try `a/b/` again.
3. The iteration did not treat the root of a path specially, so on Windows it could attempt to make a directory with a path like `X:` for something like `X:\a\b\c` if the `X:\` drive doesn't exist. This didn't lead to any problems that I could find, but there's no reason to try to make a drive letter as a directory (or any other root path).

This commit fixes all three issues by introducing a ComponentIterator that is root-aware and handles both sequential path separators and trailing path separators and uses it in `Dir.makePath`. This reduces the number of `makeDir` calls for paths where (1) the root of the path doesn't exist, (2) there are consecutive path separators, or (3) there are trailing path separators

As an example, here are the makeDir calls that would have been made before this commit when calling `makePath` for a relative path like `a/b//c//` (where the full path needs to be created):

a/b//c//
a/b//c/
a/b//c
a/b/
a/b
a
a/b
a/b/
a/b//c
a/b//c/
a/b//c//

And after this commit:

a/b//c
a/b
a
a/b
a/b//c
  • Loading branch information
squeek502 authored and andrewrk committed Jul 27, 2023
1 parent 7e1af51 commit 49053cb
Show file tree
Hide file tree
Showing 3 changed files with 604 additions and 24 deletions.
24 changes: 7 additions & 17 deletions lib/std/fs.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1462,32 +1462,22 @@ pub const Dir = struct {
/// This function is not atomic, and if it returns an error, the file system may
/// have been modified regardless.
pub fn makePath(self: Dir, sub_path: []const u8) !void {
var end_index: usize = sub_path.len;
var it = try path.componentIterator(sub_path);
var component = it.last() orelse return;
while (true) {
self.makeDir(sub_path[0..end_index]) catch |err| switch (err) {
self.makeDir(component.path) catch |err| switch (err) {
error.PathAlreadyExists => {
// TODO stat the file and return an error if it's not a directory
// this is important because otherwise a dangling symlink
// could cause an infinite loop
if (end_index == sub_path.len) return;
},
error.FileNotFound => {
// march end_index backward until next path component
while (true) {
if (end_index == 0) return err;
end_index -= 1;
if (path.isSep(sub_path[end_index])) break;
}
error.FileNotFound => |e| {
component = it.previous() orelse return e;
continue;
},
else => return err,
else => |e| return e,

This comment has been minimized.

Copy link
@mlugg

mlugg Jul 28, 2023

Member

Note that this change (correctly) changes this function's error set. It previously included error.PathAlreadyExists despite that error being impossible.

};
if (end_index == sub_path.len) return;
// march end_index forward until next path component
while (true) {
end_index += 1;
if (end_index == sub_path.len or path.isSep(sub_path[end_index])) break;
}
component = it.next() orelse return;
}
}

Expand Down
Loading

0 comments on commit 49053cb

Please sign in to comment.