From c3ef85aa7654af7f6f45ae9d6497d9fff42e9aff Mon Sep 17 00:00:00 2001 From: ominitay <37453713+ominitay@users.noreply.github.com> Date: Fri, 24 Dec 2021 01:21:35 +0000 Subject: [PATCH 1/2] Add test for using `fs.Dir.Iterator` twice --- lib/std/fs/test.zig | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index 1ab6608327..519dc98187 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -180,6 +180,42 @@ test "Dir.Iterator" { try testing.expect(contains(&entries, Dir.Entry{ .name = "some_dir", .kind = Dir.Entry.Kind.Directory })); } +// TODO Only passes on Windows, see https://github.com/ziglang/zig/issues/10317 +test "Dir.Iterator twice" { + if (builtin.os.tag != .windows) return error.SkipZigTest; + + var tmp_dir = tmpDir(.{ .iterate = true }); + defer tmp_dir.cleanup(); + + // First, create a couple of entries to iterate over. + const file = try tmp_dir.dir.createFile("some_file", .{}); + file.close(); + + try tmp_dir.dir.makeDir("some_dir"); + + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + const allocator = arena.allocator(); + + var i: u8 = 0; + while (i < 2) : (i += 1) { + var entries = std.ArrayList(Dir.Entry).init(allocator); + + // Create iterator. + var iter = tmp_dir.dir.iterate(); + while (try iter.next()) |entry| { + // We cannot just store `entry` as on Windows, we're re-using the name buffer + // which means we'll actually share the `name` pointer between entries! + const name = try allocator.dupe(u8, entry.name); + try entries.append(Dir.Entry{ .name = name, .kind = entry.kind }); + } + + try testing.expect(entries.items.len == 2); // note that the Iterator skips '.' and '..' + try testing.expect(contains(&entries, Dir.Entry{ .name = "some_file", .kind = Dir.Entry.Kind.File })); + try testing.expect(contains(&entries, Dir.Entry{ .name = "some_dir", .kind = Dir.Entry.Kind.Directory })); + } +} + fn entryEql(lhs: Dir.Entry, rhs: Dir.Entry) bool { return mem.eql(u8, lhs.name, rhs.name) and lhs.kind == rhs.kind; } From dc11fe4047450167f6b5b2b0e786881b86b3eb27 Mon Sep 17 00:00:00 2001 From: ominitay <37453713+ominitay@users.noreply.github.com> Date: Thu, 27 Jan 2022 09:59:30 +0000 Subject: [PATCH 2/2] std: Fix using fs.Dir.Iterator twice This fixes the use of multiple `Iterator`s in a row on a directory. Previously, on many platforms, using an `Iterator` on an already-iterated directory would give no entries. Fixing this involved seeking to the beginning of the directory on the first call of `next()`. --- lib/std/fs.zig | 33 +++++++++++++++++++++++++++++---- lib/std/fs/test.zig | 3 --- 2 files changed, 29 insertions(+), 7 deletions(-) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index a09f9f2ed2..27a76a6d5f 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -300,6 +300,7 @@ pub const Dir = struct { buf: [8192]u8, // TODO align(@alignOf(os.system.dirent)), index: usize, end_index: usize, + first_iter: bool, const Self = @This(); @@ -319,6 +320,10 @@ pub const Dir = struct { fn nextDarwin(self: *Self) !?Entry { start_over: while (true) { if (self.index >= self.end_index) { + if (self.first_iter) { + std.os.lseek_SET(self.dir.fd, 0) catch unreachable; // EBADF here likely means that the Dir was not opened with iteration permissions + self.first_iter = false; + } const rc = os.system.__getdirentries64( self.dir.fd, &self.buf, @@ -369,6 +374,10 @@ pub const Dir = struct { fn nextSolaris(self: *Self) !?Entry { start_over: while (true) { if (self.index >= self.end_index) { + if (self.first_iter) { + std.os.lseek_SET(self.dir.fd, 0) catch unreachable; // EBADF here likely means that the Dir was not opened with iteration permissions + self.first_iter = false; + } const rc = os.system.getdents(self.dir.fd, &self.buf, self.buf.len); switch (os.errno(rc)) { .SUCCESS => {}, @@ -423,6 +432,10 @@ pub const Dir = struct { fn nextBsd(self: *Self) !?Entry { start_over: while (true) { if (self.index >= self.end_index) { + if (self.first_iter) { + std.os.lseek_SET(self.dir.fd, 0) catch unreachable; // EBADF here likely means that the Dir was not opened with iteration permissions + self.first_iter = false; + } const rc = if (builtin.os.tag == .netbsd) os.system.__getdents30(self.dir.fd, &self.buf, self.buf.len) else @@ -479,6 +492,7 @@ pub const Dir = struct { buf: [8192]u8, // TODO align(@alignOf(os.dirent64)), index: usize, end_index: usize, + first_iter: bool, const Self = @This(); @@ -491,6 +505,10 @@ pub const Dir = struct { // TODO: find a better max const HAIKU_MAX_COUNT = 10000; if (self.index >= self.end_index) { + if (self.first_iter) { + std.os.lseek_SET(self.dir.fd, 0) catch unreachable; // EBADF here likely means that the Dir was not opened with iteration permissions + self.first_iter = false; + } const rc = os.system._kern_read_dir( self.dir.fd, &self.buf, @@ -563,6 +581,7 @@ pub const Dir = struct { buf: [8192]u8 align(if (builtin.os.tag != .linux) 1 else @alignOf(linux.dirent64)), index: usize, end_index: usize, + first_iter: bool, const Self = @This(); const linux = os.linux; @@ -574,6 +593,10 @@ pub const Dir = struct { pub fn next(self: *Self) Error!?Entry { start_over: while (true) { if (self.index >= self.end_index) { + if (self.first_iter) { + std.os.lseek_SET(self.dir.fd, 0) catch unreachable; // EBADF here likely means that the Dir was not opened with iteration permissions + self.first_iter = false; + } const rc = linux.getdents64(self.dir.fd, &self.buf, self.buf.len); switch (linux.getErrno(rc)) { .SUCCESS => {}, @@ -620,7 +643,7 @@ pub const Dir = struct { buf: [8192]u8 align(@alignOf(os.windows.FILE_BOTH_DIR_INFORMATION)), index: usize, end_index: usize, - first: bool, + first_iter: bool, name_data: [256]u8, const Self = @This(); @@ -645,9 +668,9 @@ pub const Dir = struct { .FileBothDirectoryInformation, w.FALSE, null, - if (self.first) @as(w.BOOLEAN, w.TRUE) else @as(w.BOOLEAN, w.FALSE), + if (self.first_iter) @as(w.BOOLEAN, w.TRUE) else @as(w.BOOLEAN, w.FALSE), ); - self.first = false; + self.first_iter = false; if (io.Information == 0) return null; self.index = 0; self.end_index = io.Information; @@ -769,18 +792,20 @@ pub const Dir = struct { .index = 0, .end_index = 0, .buf = undefined, + .first_iter = true, }, .linux, .haiku => return Iterator{ .dir = self, .index = 0, .end_index = 0, .buf = undefined, + .first_iter = true, }, .windows => return Iterator{ .dir = self, .index = 0, .end_index = 0, - .first = true, + .first_iter = true, .buf = undefined, .name_data = undefined, }, diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index 519dc98187..2e4d457179 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -180,10 +180,7 @@ test "Dir.Iterator" { try testing.expect(contains(&entries, Dir.Entry{ .name = "some_dir", .kind = Dir.Entry.Kind.Directory })); } -// TODO Only passes on Windows, see https://github.com/ziglang/zig/issues/10317 test "Dir.Iterator twice" { - if (builtin.os.tag != .windows) return error.SkipZigTest; - var tmp_dir = tmpDir(.{ .iterate = true }); defer tmp_dir.cleanup();