Commit 7d0c461b77

Andrew Kelley <andrew@ziglang.org>
2020-11-14 01:36:49
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
1 parent 2d28082
Changed files (1)
lib
std
fs
path.zig
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);
 }