Skip to content

Remove allocators from std.fs.{Dir.open, deleteTree} #3249

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion doc/docgen.zig
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ pub fn main() !void {
var toc = try genToc(allocator, &tokenizer);

try fs.makePath(allocator, tmp_dir_name);
defer fs.deleteTree(allocator, tmp_dir_name) catch {};
defer fs.deleteTree(tmp_dir_name) catch {};

try genHtml(allocator, &tokenizer, &toc, &buffered_out_stream.stream, zig_exe);
try buffered_out_stream.flush();
Expand Down
4 changes: 2 additions & 2 deletions lib/std/build.zig
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ pub const Builder = struct {
if (self.verbose) {
warn("rm {}\n", full_path);
}
fs.deleteTree(self.allocator, full_path) catch {};
fs.deleteTree(full_path) catch {};
}

// TODO remove empty directories
Expand Down Expand Up @@ -2655,7 +2655,7 @@ pub const RemoveDirStep = struct {
const self = @fieldParentPtr(RemoveDirStep, "step", step);

const full_path = self.builder.pathFromRoot(self.dir_path);
fs.deleteTree(self.builder.allocator, full_path) catch |err| {
fs.deleteTree(full_path) catch |err| {
warn("Unable to remove {}: {}\n", full_path, @errorName(err));
return err;
};
Expand Down
2 changes: 1 addition & 1 deletion lib/std/event/fs.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1312,7 +1312,7 @@ const test_tmp_dir = "std_event_fs_test";
//
// // TODO move this into event loop too
// try os.makePath(allocator, test_tmp_dir);
// defer os.deleteTree(allocator, test_tmp_dir) catch {};
// defer os.deleteTree(test_tmp_dir) catch {};
//
// var loop: Loop = undefined;
// try loop.initMultiThreaded(allocator);
Expand Down
61 changes: 18 additions & 43 deletions lib/std/fs.zig
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ pub const MAX_PATH_BYTES = switch (builtin.os) {
else => @compileError("Unsupported OS"),
};

pub const MAX_BUF_BYTES: usize = 8192;

// here we replace the standard +/ with -_ so that it can be used in a file name
const b64_fs_encoder = base64.Base64Encoder.init("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_", base64.standard_pad_char);

Expand Down Expand Up @@ -372,7 +374,7 @@ const DeleteTreeError = error{
/// this function recursively removes its entries and then tries again.
/// TODO determine if we can remove the allocator requirement
/// https://github.com/ziglang/zig/issues/2886
pub fn deleteTree(allocator: *Allocator, full_path: []const u8) DeleteTreeError!void {
pub fn deleteTree(full_path: []const u8) DeleteTreeError!void {
start_over: while (true) {
var got_access_denied = false;
// First, try deleting the item as a file. This way we don't follow sym links.
Expand All @@ -396,7 +398,7 @@ pub fn deleteTree(allocator: *Allocator, full_path: []const u8) DeleteTreeError!
=> return err,
}
{
var dir = Dir.open(allocator, full_path) catch |err| switch (err) {
var dir = Dir.open(full_path) catch |err| switch (err) {
error.NotDir => {
if (got_access_denied) {
return error.AccessDenied;
Expand Down Expand Up @@ -425,17 +427,14 @@ pub fn deleteTree(allocator: *Allocator, full_path: []const u8) DeleteTreeError!
};
defer dir.close();

var full_entry_buf = std.ArrayList(u8).init(allocator);
defer full_entry_buf.deinit();

while (try dir.next()) |entry| {
try full_entry_buf.resize(full_path.len + entry.name.len + 1);
const full_entry_path = full_entry_buf.toSlice();
var full_entry_buf: [MAX_BUF_BYTES]u8 = undefined;
const full_entry_path = full_entry_buf[0..];
mem.copy(u8, full_entry_path, full_path);
full_entry_path[full_path.len] = path.sep;
mem.copy(u8, full_entry_path[full_path.len + 1 ..], entry.name);

try deleteTree(allocator, full_entry_path);
try deleteTree(full_entry_path[0..full_path.len + entry.name.len + 1]);
}
}
return deleteDir(full_path);
Expand All @@ -446,19 +445,18 @@ pub fn deleteTree(allocator: *Allocator, full_path: []const u8) DeleteTreeError!
/// files, and into the one that reads files from an open directory handle.
pub const Dir = struct {
handle: Handle,
allocator: *Allocator,

pub const Handle = switch (builtin.os) {
.macosx, .ios, .freebsd, .netbsd => struct {
fd: i32,
seek: i64,
buf: []u8,
buf: [MAX_BUF_BYTES]u8,
index: usize,
end_index: usize,
},
.linux => struct {
fd: i32,
buf: []u8,
buf: [MAX_BUF_BYTES]u8,
index: usize,
end_index: usize,
},
Expand Down Expand Up @@ -513,9 +511,8 @@ pub const Dir = struct {
/// Call close when done.
/// TODO remove the allocator requirement from this API
/// https://github.com/ziglang/zig/issues/2885
pub fn open(allocator: *Allocator, dir_path: []const u8) OpenError!Dir {
pub fn open(dir_path: []const u8) OpenError!Dir {
return Dir{
.allocator = allocator,
.handle = switch (builtin.os) {
.windows => blk: {
var find_file_data: os.windows.WIN32_FIND_DATAW = undefined;
Expand Down Expand Up @@ -549,7 +546,6 @@ pub const Dir = struct {
if (os.windows.is_the_target) {
return os.windows.FindClose(self.handle.handle);
}
self.allocator.free(self.handle.buf);
os.close(self.handle.fd);
}

Expand Down Expand Up @@ -580,14 +576,10 @@ pub const Dir = struct {
fn nextDarwin(self: *Dir) !?Entry {
start_over: while (true) {
if (self.handle.index >= self.handle.end_index) {
if (self.handle.buf.len == 0) {
self.handle.buf = try self.allocator.alloc(u8, mem.page_size);
}

while (true) {
const rc = os.system.__getdirentries64(
self.handle.fd,
self.handle.buf.ptr,
self.handle.buf[0..].ptr,
self.handle.buf.len,
&self.handle.seek,
);
Expand All @@ -597,10 +589,7 @@ pub const Dir = struct {
os.EBADF => unreachable,
os.EFAULT => unreachable,
os.ENOTDIR => unreachable,
os.EINVAL => {
self.handle.buf = try self.allocator.realloc(self.handle.buf, self.handle.buf.len * 2);
continue;
},
os.EINVAL => unreachable,
else => |err| return os.unexpectedErrno(err),
}
}
Expand Down Expand Up @@ -667,21 +656,14 @@ pub const Dir = struct {
fn nextLinux(self: *Dir) !?Entry {
start_over: while (true) {
if (self.handle.index >= self.handle.end_index) {
if (self.handle.buf.len == 0) {
self.handle.buf = try self.allocator.alloc(u8, mem.page_size);
}

while (true) {
const rc = os.linux.getdents64(self.handle.fd, self.handle.buf.ptr, self.handle.buf.len);
const rc = os.linux.getdents64(self.handle.fd, self.handle.buf[0..].ptr, self.handle.buf.len);
switch (os.linux.getErrno(rc)) {
0 => {},
os.EBADF => unreachable,
os.EFAULT => unreachable,
os.ENOTDIR => unreachable,
os.EINVAL => {
self.handle.buf = try self.allocator.realloc(self.handle.buf, self.handle.buf.len * 2);
continue;
},
os.EINVAL => unreachable,
else => |err| return os.unexpectedErrno(err),
}
if (rc == 0) return null;
Expand Down Expand Up @@ -721,14 +703,10 @@ pub const Dir = struct {
fn nextBsd(self: *Dir) !?Entry {
start_over: while (true) {
if (self.handle.index >= self.handle.end_index) {
if (self.handle.buf.len == 0) {
self.handle.buf = try self.allocator.alloc(u8, mem.page_size);
}

while (true) {
const rc = os.system.getdirentries(
self.handle.fd,
self.handle.buf.ptr,
self.handle.buf[0..].ptr,
self.handle.buf.len,
&self.handle.seek,
);
Expand All @@ -737,10 +715,7 @@ pub const Dir = struct {
os.EBADF => unreachable,
os.EFAULT => unreachable,
os.ENOTDIR => unreachable,
os.EINVAL => {
self.handle.buf = try self.allocator.realloc(self.handle.buf, self.handle.buf.len * 2);
continue;
},
os.EINVAL => unreachable,
else => |err| return os.unexpectedErrno(err),
}
if (rc == 0) return null;
Expand Down Expand Up @@ -808,7 +783,7 @@ pub const Walker = struct {
try self.name_buffer.append(base.name);
if (base.kind == .Directory) {
// TODO https://github.com/ziglang/zig/issues/2888
var new_dir = try Dir.open(self.stack.allocator, self.name_buffer.toSliceConst());
var new_dir = try Dir.open(self.name_buffer.toSliceConst());
{
errdefer new_dir.close();
try self.stack.append(StackItem{
Expand Down Expand Up @@ -842,7 +817,7 @@ pub const Walker = struct {
pub fn walkPath(allocator: *Allocator, dir_path: []const u8) !Walker {
assert(!mem.endsWith(u8, dir_path, path.sep_str));

var dir_it = try Dir.open(allocator, dir_path);
var dir_it = try Dir.open(dir_path);
errdefer dir_it.close();

var name_buffer = try std.Buffer.init(allocator, dir_path);
Expand Down
6 changes: 3 additions & 3 deletions lib/std/os/test.zig
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ test "makePath, put some files in it, deleteTree" {
try fs.makePath(a, "os_test_tmp" ++ fs.path.sep_str ++ "b" ++ fs.path.sep_str ++ "c");
try io.writeFile("os_test_tmp" ++ fs.path.sep_str ++ "b" ++ fs.path.sep_str ++ "c" ++ fs.path.sep_str ++ "file.txt", "nonsense");
try io.writeFile("os_test_tmp" ++ fs.path.sep_str ++ "b" ++ fs.path.sep_str ++ "file2.txt", "blah");
try fs.deleteTree(a, "os_test_tmp");
if (fs.Dir.open(a, "os_test_tmp")) |dir| {
try fs.deleteTree("os_test_tmp");
if (fs.Dir.open("os_test_tmp")) |dir| {
@panic("expected error");
} else |err| {
expect(err == error.FileNotFound);
Expand All @@ -37,7 +37,7 @@ test "access file" {

try io.writeFile("os_test_tmp" ++ fs.path.sep_str ++ "file.txt", "");
try os.access("os_test_tmp" ++ fs.path.sep_str ++ "file.txt", os.F_OK);
try fs.deleteTree(a, "os_test_tmp");
try fs.deleteTree("os_test_tmp");
}

fn testThreadIdFn(thread_id: *Thread.Id) void {
Expand Down
2 changes: 1 addition & 1 deletion src-self-hosted/main.zig
Original file line number Diff line number Diff line change
Expand Up @@ -747,7 +747,7 @@ async fn fmtPath(fmt: *Fmt, file_path_ref: []const u8, check_mode: bool) FmtErro
)) catch |err| switch (err) {
error.IsDir, error.AccessDenied => {
// TODO make event based (and dir.next())
var dir = try fs.Dir.open(fmt.loop.allocator, file_path);
var dir = try fs.Dir.open(file_path);
defer dir.close();

var group = event.Group(FmtError!void).init(fmt.loop);
Expand Down
2 changes: 1 addition & 1 deletion src-self-hosted/stage1.zig
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ fn fmtPath(fmt: *Fmt, file_path_ref: []const u8, check_mode: bool) FmtError!void
const source_code = io.readFileAlloc(fmt.allocator, file_path) catch |err| switch (err) {
error.IsDir, error.AccessDenied => {
// TODO make event based (and dir.next())
var dir = try fs.Dir.open(fmt.allocator, file_path);
var dir = try fs.Dir.open(file_path);
defer dir.close();

while (try dir.next()) |entry| {
Expand Down
4 changes: 2 additions & 2 deletions src-self-hosted/test.zig
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,11 @@ pub const TestContext = struct {
errdefer allocator.free(self.zig_lib_dir);

try std.fs.makePath(allocator, tmp_dir_name);
errdefer std.fs.deleteTree(allocator, tmp_dir_name) catch {};
errdefer std.fs.deleteTree(tmp_dir_name) catch {};
}

fn deinit(self: *TestContext) void {
std.fs.deleteTree(allocator, tmp_dir_name) catch {};
std.fs.deleteTree(tmp_dir_name) catch {};
allocator.free(self.zig_lib_dir);
self.zig_compiler.deinit();
self.loop.deinit();
Expand Down
2 changes: 1 addition & 1 deletion test/cli.zig
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pub fn main() !void {
testMissingOutputPath,
};
for (test_fns) |testFn| {
try fs.deleteTree(a, dir_path);
try fs.deleteTree(dir_path);
try fs.makeDir(dir_path);
try testFn(zig_exe, dir_path);
}
Expand Down
2 changes: 1 addition & 1 deletion tools/process_headers.zig
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ pub fn main() !void {
try dir_stack.append(target_include_dir);

while (dir_stack.popOrNull()) |full_dir_name| {
var dir = std.fs.Dir.open(allocator, full_dir_name) catch |err| switch (err) {
var dir = std.fs.Dir.open(full_dir_name) catch |err| switch (err) {
error.FileNotFound => continue :search,
error.AccessDenied => continue :search,
else => return err,
Expand Down