From 6943cefebf94631ff02d6e28436c07f4030924c6 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 14 Jun 2018 16:15:32 -0400 Subject: [PATCH] std.os.path.dirname: return null instead of empty slice for when there is no directory component. Makes it harder to write bugs. closes #1017 --- src-self-hosted/introspect.zig | 2 +- src-self-hosted/main.zig | 2 +- std/build.zig | 16 +++++----- std/os/index.zig | 12 ++++---- std/os/path.zig | 54 +++++++++++++++++++++------------- 5 files changed, 51 insertions(+), 35 deletions(-) diff --git a/src-self-hosted/introspect.zig b/src-self-hosted/introspect.zig index 56b56c0c78..74084b48c6 100644 --- a/src-self-hosted/introspect.zig +++ b/src-self-hosted/introspect.zig @@ -27,7 +27,7 @@ pub fn findZigLibDir(allocator: *mem.Allocator) ![]u8 { var cur_path: []const u8 = self_exe_path; while (true) { - const test_dir = os.path.dirname(cur_path); + const test_dir = os.path.dirname(cur_path) orelse "."; if (mem.eql(u8, test_dir, cur_path)) { break; diff --git a/src-self-hosted/main.zig b/src-self-hosted/main.zig index 1c91ab9cbe..ffe23d2ffe 100644 --- a/src-self-hosted/main.zig +++ b/src-self-hosted/main.zig @@ -249,7 +249,7 @@ fn cmdBuild(allocator: *Allocator, args: []const []const u8) !void { defer build_args.deinit(); const build_file_basename = os.path.basename(build_file_abs); - const build_file_dirname = os.path.dirname(build_file_abs); + const build_file_dirname = os.path.dirname(build_file_abs) orelse "."; var full_cache_dir: []u8 = undefined; if (flags.single("cache-dir")) |cache_dir| { diff --git a/std/build.zig b/std/build.zig index 5733aec17d..16ce426bcb 100644 --- a/std/build.zig +++ b/std/build.zig @@ -617,7 +617,7 @@ pub const Builder = struct { warn("cp {} {}\n", source_path, dest_path); } - const dirname = os.path.dirname(dest_path); + const dirname = os.path.dirname(dest_path) orelse "."; const abs_source_path = self.pathFromRoot(source_path); os.makePath(self.allocator, dirname) catch |err| { warn("Unable to create path {}: {}\n", dirname, @errorName(err)); @@ -1395,8 +1395,9 @@ pub const LibExeObjStep = struct { cc_args.append(abs_source_file) catch unreachable; const cache_o_src = os.path.join(builder.allocator, builder.cache_root, source_file) catch unreachable; - const cache_o_dir = os.path.dirname(cache_o_src); - try builder.makePath(cache_o_dir); + if (os.path.dirname(cache_o_src)) |cache_o_dir| { + try builder.makePath(cache_o_dir); + } const cache_o_file = builder.fmt("{}{}", cache_o_src, self.target.oFileExt()); cc_args.append("-o") catch unreachable; cc_args.append(builder.pathFromRoot(cache_o_file)) catch unreachable; @@ -1509,8 +1510,9 @@ pub const LibExeObjStep = struct { cc_args.append(abs_source_file) catch unreachable; const cache_o_src = os.path.join(builder.allocator, builder.cache_root, source_file) catch unreachable; - const cache_o_dir = os.path.dirname(cache_o_src); - try builder.makePath(cache_o_dir); + if (os.path.dirname(cache_o_src)) |cache_o_dir| { + try builder.makePath(cache_o_dir); + } const cache_o_file = builder.fmt("{}{}", cache_o_src, self.target.oFileExt()); cc_args.append("-o") catch unreachable; cc_args.append(builder.pathFromRoot(cache_o_file)) catch unreachable; @@ -1855,7 +1857,7 @@ pub const WriteFileStep = struct { fn make(step: *Step) !void { const self = @fieldParentPtr(WriteFileStep, "step", step); const full_path = self.builder.pathFromRoot(self.file_path); - const full_path_dir = os.path.dirname(full_path); + const full_path_dir = os.path.dirname(full_path) orelse "."; os.makePath(self.builder.allocator, full_path_dir) catch |err| { warn("unable to make path {}: {}\n", full_path_dir, @errorName(err)); return err; @@ -1945,7 +1947,7 @@ pub const Step = struct { }; fn doAtomicSymLinks(allocator: *Allocator, output_path: []const u8, filename_major_only: []const u8, filename_name_only: []const u8) !void { - const out_dir = os.path.dirname(output_path); + const out_dir = os.path.dirname(output_path) orelse "."; const out_basename = os.path.basename(output_path); // sym link for libfoo.so.1 to libfoo.so.1.2.3 const major_only_path = os.path.join(allocator, out_dir, filename_major_only) catch unreachable; diff --git a/std/os/index.zig b/std/os/index.zig index 612301d25d..46f5e76d98 100644 --- a/std/os/index.zig +++ b/std/os/index.zig @@ -714,7 +714,7 @@ pub fn atomicSymLink(allocator: *Allocator, existing_path: []const u8, new_path: else => return err, // TODO zig should know this set does not include PathAlreadyExists } - const dirname = os.path.dirname(new_path); + const dirname = os.path.dirname(new_path) orelse "."; var rand_buf: [12]u8 = undefined; const tmp_path = try allocator.alloc(u8, dirname.len + 1 + base64.Base64Encoder.calcSize(rand_buf.len)); @@ -860,14 +860,14 @@ pub const AtomicFile = struct { var rand_buf: [12]u8 = undefined; - const dirname_component_len = if (dirname.len == 0) 0 else dirname.len + 1; + const dirname_component_len = if (dirname) |d| d.len + 1 else 0; const tmp_path = try allocator.alloc(u8, dirname_component_len + base64.Base64Encoder.calcSize(rand_buf.len)); errdefer allocator.free(tmp_path); - if (dirname.len != 0) { - mem.copy(u8, tmp_path[0..], dirname); - tmp_path[dirname.len] = os.path.sep; + if (dirname) |dir| { + mem.copy(u8, tmp_path[0..], dir); + tmp_path[dir.len] = os.path.sep; } while (true) { @@ -1965,7 +1965,7 @@ pub fn selfExeDirPath(allocator: *mem.Allocator) ![]u8 { // the executable was in when it was run. const full_exe_path = try readLink(allocator, "/proc/self/exe"); errdefer allocator.free(full_exe_path); - const dir = path.dirname(full_exe_path); + const dir = path.dirname(full_exe_path) orelse "."; return allocator.shrink(u8, full_exe_path, dir.len); }, Os.windows, Os.macosx, Os.ios => { diff --git a/std/os/path.zig b/std/os/path.zig index a3ad23b1a9..d3ab0c519f 100644 --- a/std/os/path.zig +++ b/std/os/path.zig @@ -648,8 +648,8 @@ fn testResolvePosix(paths: []const []const u8) []u8 { } /// If the path is a file in the current directory (no directory component) -/// then the returned slice has .len = 0. -pub fn dirname(path: []const u8) []const u8 { +/// then returns null +pub fn dirname(path: []const u8) ?[]const u8 { if (is_windows) { return dirnameWindows(path); } else { @@ -657,9 +657,9 @@ pub fn dirname(path: []const u8) []const u8 { } } -pub fn dirnameWindows(path: []const u8) []const u8 { +pub fn dirnameWindows(path: []const u8) ?[]const u8 { if (path.len == 0) - return path[0..0]; + return null; const root_slice = diskDesignatorWindows(path); if (path.len == root_slice.len) @@ -671,13 +671,13 @@ pub fn dirnameWindows(path: []const u8) []const u8 { while ((path[end_index] == '/' or path[end_index] == '\\') and end_index > root_slice.len) { if (end_index == 0) - return path[0..0]; + return null; end_index -= 1; } while (path[end_index] != '/' and path[end_index] != '\\' and end_index > root_slice.len) { if (end_index == 0) - return path[0..0]; + return null; end_index -= 1; } @@ -685,12 +685,15 @@ pub fn dirnameWindows(path: []const u8) []const u8 { end_index += 1; } + if (end_index == 0) + return null; + return path[0..end_index]; } -pub fn dirnamePosix(path: []const u8) []const u8 { +pub fn dirnamePosix(path: []const u8) ?[]const u8 { if (path.len == 0) - return path[0..0]; + return null; var end_index: usize = path.len - 1; while (path[end_index] == '/') { @@ -701,13 +704,16 @@ pub fn dirnamePosix(path: []const u8) []const u8 { while (path[end_index] != '/') { if (end_index == 0) - return path[0..0]; + return null; end_index -= 1; } if (end_index == 0 and path[end_index] == '/') return path[0..1]; + if (end_index == 0) + return null; + return path[0..end_index]; } @@ -717,10 +723,10 @@ test "os.path.dirnamePosix" { testDirnamePosix("/a", "/"); testDirnamePosix("/", "/"); testDirnamePosix("////", "/"); - testDirnamePosix("", ""); - testDirnamePosix("a", ""); - testDirnamePosix("a/", ""); - testDirnamePosix("a//", ""); + testDirnamePosix("", null); + testDirnamePosix("a", null); + testDirnamePosix("a/", null); + testDirnamePosix("a//", null); } test "os.path.dirnameWindows" { @@ -742,7 +748,7 @@ test "os.path.dirnameWindows" { testDirnameWindows("c:foo\\bar", "c:foo"); testDirnameWindows("c:foo\\bar\\", "c:foo"); testDirnameWindows("c:foo\\bar\\baz", "c:foo\\bar"); - testDirnameWindows("file:stream", ""); + testDirnameWindows("file:stream", null); testDirnameWindows("dir\\file:stream", "dir"); testDirnameWindows("\\\\unc\\share", "\\\\unc\\share"); testDirnameWindows("\\\\unc\\share\\foo", "\\\\unc\\share\\"); @@ -753,18 +759,26 @@ test "os.path.dirnameWindows" { testDirnameWindows("/a/b/", "/a"); testDirnameWindows("/a/b", "/a"); testDirnameWindows("/a", "/"); - testDirnameWindows("", ""); + testDirnameWindows("", null); testDirnameWindows("/", "/"); testDirnameWindows("////", "/"); - testDirnameWindows("foo", ""); + testDirnameWindows("foo", null); } -fn testDirnamePosix(input: []const u8, expected_output: []const u8) void { - assert(mem.eql(u8, dirnamePosix(input), expected_output)); +fn testDirnamePosix(input: []const u8, expected_output: ?[]const u8) void { + if (dirnamePosix(input)) |output| { + assert(mem.eql(u8, output, expected_output.?)); + } else { + assert(expected_output == null); + } } -fn testDirnameWindows(input: []const u8, expected_output: []const u8) void { - assert(mem.eql(u8, dirnameWindows(input), expected_output)); +fn testDirnameWindows(input: []const u8, expected_output: ?[]const u8) void { + if (dirnameWindows(input)) |output| { + assert(mem.eql(u8, output, expected_output.?)); + } else { + assert(expected_output == null); + } } pub fn basename(path: []const u8) []const u8 {