From bf03ae7acc2e8478958e46b2e418fcf29aa99128 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 13 Nov 2020 17:36:49 -0700 Subject: [PATCH] std.fs.path.dirname: return null when input path is root This intentionally diverges from the unix dirname command, as well as Python and Node.js standard libraries, which all have this edge case return the input path, unmodified. This is a footgun, and nobody should have ever done it this way. Even the man page contradicts the behavior. It says: "strip last component from file name". Now consider, if you remove the last item from an array of length 1, then you have now an array of length 0. After you strip the last component, there should be no components remaining. Clearly, returning the input parameter unmodified in this case does not match the documented behavior. This is my justification for taking a stand on this API design. closes #6746 closes #6727 closes #6584 closes #6592 closes #6602 --- lib/std/fs/path.zig | 38 ++++++++++++++++++++++---------------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/lib/std/fs/path.zig b/lib/std/fs/path.zig index 5e418e8e5c..1bf2211372 100644 --- a/lib/std/fs/path.zig +++ b/lib/std/fs/path.zig @@ -749,8 +749,12 @@ fn testResolvePosix(paths: []const []const u8, expected: []const u8) !void { return testing.expect(mem.eql(u8, actual, expected)); } +/// Strip the last component from a file path. +/// /// If the path is a file in the current directory (no directory component) -/// then returns null +/// then returns null. +/// +/// If the path is the root directory, returns null. pub fn dirname(path: []const u8) ?[]const u8 { if (builtin.os.tag == .windows) { return dirnameWindows(path); @@ -765,19 +769,19 @@ pub fn dirnameWindows(path: []const u8) ?[]const u8 { const root_slice = diskDesignatorWindows(path); if (path.len == root_slice.len) - return path; + return null; const have_root_slash = path.len > root_slice.len and (path[root_slice.len] == '/' or path[root_slice.len] == '\\'); var end_index: usize = path.len - 1; - while ((path[end_index] == '/' or path[end_index] == '\\') and end_index > root_slice.len) { + while (path[end_index] == '/' or path[end_index] == '\\') { if (end_index == 0) return null; end_index -= 1; } - while (path[end_index] != '/' and path[end_index] != '\\' and end_index > root_slice.len) { + while (path[end_index] != '/' and path[end_index] != '\\') { if (end_index == 0) return null; end_index -= 1; @@ -800,7 +804,7 @@ pub fn dirnamePosix(path: []const u8) ?[]const u8 { var end_index: usize = path.len - 1; while (path[end_index] == '/') { if (end_index == 0) - return path[0..1]; + return null; end_index -= 1; } @@ -810,7 +814,7 @@ pub fn dirnamePosix(path: []const u8) ?[]const u8 { end_index -= 1; } - if (end_index == 0 and path[end_index] == '/') + if (end_index == 0 and path[0] == '/') return path[0..1]; if (end_index == 0) @@ -823,8 +827,10 @@ test "dirnamePosix" { testDirnamePosix("/a/b/c", "/a/b"); testDirnamePosix("/a/b/c///", "/a/b"); testDirnamePosix("/a", "/"); - testDirnamePosix("/", "/"); - testDirnamePosix("////", "/"); + testDirnamePosix("/", null); + testDirnamePosix("//", null); + testDirnamePosix("///", null); + testDirnamePosix("////", null); testDirnamePosix("", null); testDirnamePosix("a", null); testDirnamePosix("a/", null); @@ -832,27 +838,27 @@ test "dirnamePosix" { } test "dirnameWindows" { - testDirnameWindows("c:\\", "c:\\"); + testDirnameWindows("c:\\", null); testDirnameWindows("c:\\foo", "c:\\"); testDirnameWindows("c:\\foo\\", "c:\\"); testDirnameWindows("c:\\foo\\bar", "c:\\foo"); testDirnameWindows("c:\\foo\\bar\\", "c:\\foo"); testDirnameWindows("c:\\foo\\bar\\baz", "c:\\foo\\bar"); - testDirnameWindows("\\", "\\"); + testDirnameWindows("\\", null); testDirnameWindows("\\foo", "\\"); testDirnameWindows("\\foo\\", "\\"); testDirnameWindows("\\foo\\bar", "\\foo"); testDirnameWindows("\\foo\\bar\\", "\\foo"); testDirnameWindows("\\foo\\bar\\baz", "\\foo\\bar"); - testDirnameWindows("c:", "c:"); - testDirnameWindows("c:foo", "c:"); - testDirnameWindows("c:foo\\", "c:"); + testDirnameWindows("c:", null); + testDirnameWindows("c:foo", null); + testDirnameWindows("c:foo\\", null); testDirnameWindows("c:foo\\bar", "c:foo"); testDirnameWindows("c:foo\\bar\\", "c:foo"); testDirnameWindows("c:foo\\bar\\baz", "c:foo\\bar"); testDirnameWindows("file:stream", null); testDirnameWindows("dir\\file:stream", "dir"); - testDirnameWindows("\\\\unc\\share", "\\\\unc\\share"); + testDirnameWindows("\\\\unc\\share", null); testDirnameWindows("\\\\unc\\share\\foo", "\\\\unc\\share\\"); testDirnameWindows("\\\\unc\\share\\foo\\", "\\\\unc\\share\\"); testDirnameWindows("\\\\unc\\share\\foo\\bar", "\\\\unc\\share\\foo"); @@ -862,8 +868,8 @@ test "dirnameWindows" { testDirnameWindows("/a/b", "/a"); testDirnameWindows("/a", "/"); testDirnameWindows("", null); - testDirnameWindows("/", "/"); - testDirnameWindows("////", "/"); + testDirnameWindows("/", null); + testDirnameWindows("////", null); testDirnameWindows("foo", null); }