From 64bd134818b77ba56deb613112ec8cdb0c967703 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Tue, 30 Jun 2020 17:43:00 +0200 Subject: [PATCH 1/4] Add Dir.Iterator tests This commit adds some `std.fs.Dir.Iterator` tests. --- lib/std/fs/test.zig | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index 52f823a32f..ea08b8f064 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -3,10 +3,47 @@ const testing = std.testing; const builtin = std.builtin; const fs = std.fs; const mem = std.mem; +const wasi = std.os.wasi; +const Dir = std.fs.Dir; const File = std.fs.File; const tmpDir = testing.tmpDir; +test "Dir.Iterator" { + 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"); + + // Create iterator. + var iter = tmp_dir.dir.iterate(); + var entries = std.ArrayList(Dir.Entry).init(testing.allocator); + defer entries.deinit(); + + while (try iter.next()) |entry| { + try entries.append(entry); + } + + testing.expect(entries.items.len == 2); // note that the Iterator skips '.' and '..' + testing.expect(contains(&entries, Dir.Entry{ .name = "some_file", .kind = Dir.Entry.Kind.File })); + testing.expect(contains(&entries, Dir.Entry{ .name = "some_dir", .kind = Dir.Entry.Kind.Directory })); +} + +fn entry_eql(lhs: Dir.Entry, rhs: Dir.Entry) bool { + return mem.eql(u8, lhs.name, rhs.name) and lhs.kind == rhs.kind; +} + +fn contains(entries: *const std.ArrayList(Dir.Entry), el: Dir.Entry) bool { + for (entries.items) |entry| { + if (entry_eql(entry, el)) return true; + } + return false; +} + test "readAllAlloc" { var tmp_dir = tmpDir(.{}); defer tmp_dir.cleanup(); @@ -237,7 +274,7 @@ test "fs.copyFile" { try expectFileContents(tmp.dir, dest_file2, data); } -fn expectFileContents(dir: fs.Dir, file_path: []const u8, data: []const u8) !void { +fn expectFileContents(dir: Dir, file_path: []const u8, data: []const u8) !void { const contents = try dir.readFileAlloc(testing.allocator, file_path, 1000); defer testing.allocator.free(contents); From b5badd112288b528d75085d82d5f7f1b109e87dc Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 1 Jul 2020 14:15:58 +0200 Subject: [PATCH 2/4] Fix memory corruption in Dir.Iterator test --- lib/std/fs/test.zig | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index ea08b8f064..ddf4a28213 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -5,6 +5,7 @@ const fs = std.fs; const mem = std.mem; const wasi = std.os.wasi; +const ArenaAllocator = std.heap.ArenaAllocator; const Dir = std.fs.Dir; const File = std.fs.File; const tmpDir = testing.tmpDir; @@ -19,13 +20,18 @@ test "Dir.Iterator" { try tmp_dir.dir.makeDir("some_dir"); + var arena = ArenaAllocator.init(testing.allocator); + defer arena.deinit(); + + var entries = std.ArrayList(Dir.Entry).init(&arena.allocator); + // Create iterator. var iter = tmp_dir.dir.iterate(); - var entries = std.ArrayList(Dir.Entry).init(testing.allocator); - defer entries.deinit(); - while (try iter.next()) |entry| { - try entries.append(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 mem.dupe(&arena.allocator, u8, entry.name); + try entries.append(Dir.Entry{ .name = name, .kind = entry.kind }); } testing.expect(entries.items.len == 2); // note that the Iterator skips '.' and '..' From e7d02eae4d99fbae11e91ddd7601e5ac2a392292 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Thu, 2 Jul 2020 20:54:57 +0200 Subject: [PATCH 3/4] Update lib/std/fs/test.zig Co-authored-by: Joachim Schmidt --- lib/std/fs/test.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index ddf4a28213..a3cf2e8002 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -30,7 +30,7 @@ test "Dir.Iterator" { 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 mem.dupe(&arena.allocator, u8, entry.name); + const name = try arena.allocator.dupe(u8, entry.name); try entries.append(Dir.Entry{ .name = name, .kind = entry.kind }); } From 417c92895263250ca399bcf7a1a2abaee6067624 Mon Sep 17 00:00:00 2001 From: Jakub Konka Date: Wed, 8 Jul 2020 00:03:45 +0200 Subject: [PATCH 4/4] Add comment about memory invalidation in Iterator.next on Win --- lib/std/fs.zig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/std/fs.zig b/lib/std/fs.zig index e932253bdb..f8777f28e4 100644 --- a/lib/std/fs.zig +++ b/lib/std/fs.zig @@ -453,6 +453,8 @@ pub const Dir = struct { pub const Error = IteratorError; + /// Memory such as file names referenced in this returned entry becomes invalid + /// with subsequent calls to `next`, as well as when this `Dir` is deinitialized. pub fn next(self: *Self) Error!?Entry { start_over: while (true) { const w = os.windows;