Commit 219acaa1d6

Carter Snook <cartersnook04@gmail.com>
2024-06-13 03:39:18
std.fs.Dir: Refactor atomicSymLink from std.fs
Deprecates std.fs.atomicSymLink and removes the allocator requirement from the new std.fs.Dir.atomicSymLink. Replaces the two usages of this within std. I did not include the TODOs from the original code that were based off of `switch (err) { ..., else => return err }` not having correct inference that cases handled in `...` are impossible in the error union return type because these are not specified in many places but I can add them back if wanted. Thank you @squeek502 for help with fixing buffer overflows!
1 parent 4a77c7f
Changed files (3)
lib
std
lib/std/Build/Step/Compile.zig
@@ -1862,19 +1862,18 @@ pub fn doAtomicSymLinks(
     filename_name_only: []const u8,
 ) !void {
     const b = step.owner;
-    const arena = b.allocator;
     const out_dir = fs.path.dirname(output_path) orelse ".";
     const out_basename = fs.path.basename(output_path);
     // sym link for libfoo.so.1 to libfoo.so.1.2.3
     const major_only_path = b.pathJoin(&.{ out_dir, filename_major_only });
-    fs.atomicSymLink(arena, out_basename, major_only_path) catch |err| {
+    fs.cwd().atomicSymLink(out_basename, major_only_path, .{}) catch |err| {
         return step.fail("unable to symlink {s} -> {s}: {s}", .{
             major_only_path, out_basename, @errorName(err),
         });
     };
     // sym link for libfoo.so to libfoo.so.1
     const name_only_path = b.pathJoin(&.{ out_dir, filename_name_only });
-    fs.atomicSymLink(arena, filename_major_only, name_only_path) catch |err| {
+    fs.cwd().atomicSymLink(filename_major_only, name_only_path, .{}) catch |err| {
         return step.fail("Unable to symlink {s} -> {s}: {s}", .{
             name_only_path, filename_major_only, @errorName(err),
         });
lib/std/fs/Dir.zig
@@ -1758,10 +1758,11 @@ pub fn renameW(self: Dir, old_sub_path_w: []const u16, new_sub_path_w: []const u
     return posix.renameatW(self.fd, old_sub_path_w, self.fd, new_sub_path_w);
 }
 
-/// Use with `Dir.symLink` and `symLinkAbsolute` to specify whether the symlink
-/// will point to a file or a directory. This value is ignored on all hosts
-/// except Windows where creating symlinks to different resource types, requires
-/// different flags. By default, `symLinkAbsolute` is assumed to point to a file.
+/// Use with `Dir.symLink`, `Dir.atomicSymLink`, and `symLinkAbsolute` to
+/// specify whether the symlink will point to a file or a directory. This value
+/// is ignored on all hosts except Windows where creating symlinks to different
+/// resource types, requires different flags. By default, `symLinkAbsolute` is
+/// assumed to point to a file.
 pub const SymLinkFlags = struct {
     is_directory: bool = false,
 };
@@ -1847,6 +1848,50 @@ pub fn symLinkW(
     return windows.CreateSymbolicLink(self.fd, sym_link_path_w, target_path_w, flags.is_directory);
 }
 
+/// Same as `symLink`, except tries to create the symbolic link until it
+/// succeeds or encounters an error other than `error.PathAlreadyExists`.
+/// On Windows, both paths should be encoded as [WTF-8](https://simonsapin.github.io/wtf-8/).
+/// On WASI, both paths should be encoded as valid UTF-8.
+/// On other platforms, both paths are an opaque sequence of bytes with no particular encoding.
+pub fn atomicSymLink(
+    dir: Dir,
+    target_path: []const u8,
+    sym_link_path: []const u8,
+    flags: SymLinkFlags,
+) !void {
+    if (dir.symLink(target_path, sym_link_path, flags)) {
+        return;
+    } else |err| switch (err) {
+        error.PathAlreadyExists => {},
+        else => |e| return e,
+    }
+
+    const dirname = path.dirname(sym_link_path) orelse ".";
+
+    var rand_buf: [AtomicFile.random_bytes_len]u8 = undefined;
+
+    const temp_path_len = dirname.len + 1 + base64_encoder.calcSize(rand_buf.len);
+    var temp_path_buf: [fs.max_path_bytes]u8 = undefined;
+
+    if (temp_path_len > temp_path_buf.len) return error.NameTooLong;
+    @memcpy(temp_path_buf[0..dirname.len], dirname);
+    temp_path_buf[dirname.len] = path.sep;
+
+    const temp_path = temp_path_buf[0..temp_path_len];
+
+    while (true) {
+        crypto.random.bytes(rand_buf[0..]);
+        _ = base64_encoder.encode(temp_path[dirname.len + 1 ..], rand_buf[0..]);
+
+        if (dir.symLink(target_path, temp_path, flags)) {
+            return dir.rename(temp_path, sym_link_path);
+        } else |err| switch (err) {
+            error.PathAlreadyExists => continue,
+            else => |e| return e,
+        }
+    }
+}
+
 pub const ReadLinkError = posix.ReadLinkError;
 
 /// Read value of a symbolic link.
@@ -2695,8 +2740,11 @@ const builtin = @import("builtin");
 const std = @import("../std.zig");
 const File = std.fs.File;
 const AtomicFile = std.fs.AtomicFile;
+const base64_encoder = fs.base64_encoder;
+const crypto = std.crypto;
 const posix = std.posix;
 const mem = std.mem;
+const path = fs.path;
 const fs = std.fs;
 const Allocator = std.mem.Allocator;
 const assert = std.debug.assert;
lib/std/fs.zig
@@ -101,37 +101,9 @@ pub const base64_encoder = base64.Base64Encoder.init(base64_alphabet, null);
 /// Base64 decoder, replacing the standard `+/` with `-_` so that it can be used in a file name on any filesystem.
 pub const base64_decoder = base64.Base64Decoder.init(base64_alphabet, null);
 
-/// TODO remove the allocator requirement from this API
-/// TODO move to Dir
-/// On Windows, both paths should be encoded as [WTF-8](https://simonsapin.github.io/wtf-8/).
-/// On WASI, both paths should be encoded as valid UTF-8.
-/// On other platforms, both paths are an opaque sequence of bytes with no particular encoding.
-pub fn atomicSymLink(allocator: Allocator, existing_path: []const u8, new_path: []const u8) !void {
-    if (cwd().symLink(existing_path, new_path, .{})) {
-        return;
-    } else |err| switch (err) {
-        error.PathAlreadyExists => {},
-        else => return err, // TODO zig should know this set does not include PathAlreadyExists
-    }
-
-    const dirname = path.dirname(new_path) orelse ".";
-
-    var rand_buf: [AtomicFile.random_bytes_len]u8 = undefined;
-    const tmp_path = try allocator.alloc(u8, dirname.len + 1 + base64_encoder.calcSize(rand_buf.len));
-    defer allocator.free(tmp_path);
-    @memcpy(tmp_path[0..dirname.len], dirname);
-    tmp_path[dirname.len] = path.sep;
-    while (true) {
-        crypto.random.bytes(rand_buf[0..]);
-        _ = base64_encoder.encode(tmp_path[dirname.len + 1 ..], &rand_buf);
-
-        if (cwd().symLink(existing_path, tmp_path, .{})) {
-            return cwd().rename(tmp_path, new_path);
-        } else |err| switch (err) {
-            error.PathAlreadyExists => continue,
-            else => return err, // TODO zig should know this set does not include PathAlreadyExists
-        }
-    }
+/// Deprecated. Use `cwd().atomicSymLink()` instead.
+pub fn atomicSymLink(_: Allocator, existing_path: []const u8, new_path: []const u8) !void {
+    try cwd().atomicSymLink(existing_path, new_path, .{});
 }
 
 /// Same as `Dir.updateFile`, except asserts that both `source_path` and `dest_path`