Skip to content

Commit 219acaa

Browse files
sno2andrewrk
authored andcommitted
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 commit 219acaa

File tree

3 files changed

+57
-38
lines changed

3 files changed

+57
-38
lines changed

lib/std/Build/Step/Compile.zig

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1862,19 +1862,18 @@ pub fn doAtomicSymLinks(
18621862
filename_name_only: []const u8,
18631863
) !void {
18641864
const b = step.owner;
1865-
const arena = b.allocator;
18661865
const out_dir = fs.path.dirname(output_path) orelse ".";
18671866
const out_basename = fs.path.basename(output_path);
18681867
// sym link for libfoo.so.1 to libfoo.so.1.2.3
18691868
const major_only_path = b.pathJoin(&.{ out_dir, filename_major_only });
1870-
fs.atomicSymLink(arena, out_basename, major_only_path) catch |err| {
1869+
fs.cwd().atomicSymLink(out_basename, major_only_path, .{}) catch |err| {
18711870
return step.fail("unable to symlink {s} -> {s}: {s}", .{
18721871
major_only_path, out_basename, @errorName(err),
18731872
});
18741873
};
18751874
// sym link for libfoo.so to libfoo.so.1
18761875
const name_only_path = b.pathJoin(&.{ out_dir, filename_name_only });
1877-
fs.atomicSymLink(arena, filename_major_only, name_only_path) catch |err| {
1876+
fs.cwd().atomicSymLink(filename_major_only, name_only_path, .{}) catch |err| {
18781877
return step.fail("Unable to symlink {s} -> {s}: {s}", .{
18791878
name_only_path, filename_major_only, @errorName(err),
18801879
});

lib/std/fs.zig

Lines changed: 3 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -101,37 +101,9 @@ pub const base64_encoder = base64.Base64Encoder.init(base64_alphabet, null);
101101
/// Base64 decoder, replacing the standard `+/` with `-_` so that it can be used in a file name on any filesystem.
102102
pub const base64_decoder = base64.Base64Decoder.init(base64_alphabet, null);
103103

104-
/// TODO remove the allocator requirement from this API
105-
/// TODO move to Dir
106-
/// On Windows, both paths should be encoded as [WTF-8](https://simonsapin.github.io/wtf-8/).
107-
/// On WASI, both paths should be encoded as valid UTF-8.
108-
/// On other platforms, both paths are an opaque sequence of bytes with no particular encoding.
109-
pub fn atomicSymLink(allocator: Allocator, existing_path: []const u8, new_path: []const u8) !void {
110-
if (cwd().symLink(existing_path, new_path, .{})) {
111-
return;
112-
} else |err| switch (err) {
113-
error.PathAlreadyExists => {},
114-
else => return err, // TODO zig should know this set does not include PathAlreadyExists
115-
}
116-
117-
const dirname = path.dirname(new_path) orelse ".";
118-
119-
var rand_buf: [AtomicFile.random_bytes_len]u8 = undefined;
120-
const tmp_path = try allocator.alloc(u8, dirname.len + 1 + base64_encoder.calcSize(rand_buf.len));
121-
defer allocator.free(tmp_path);
122-
@memcpy(tmp_path[0..dirname.len], dirname);
123-
tmp_path[dirname.len] = path.sep;
124-
while (true) {
125-
crypto.random.bytes(rand_buf[0..]);
126-
_ = base64_encoder.encode(tmp_path[dirname.len + 1 ..], &rand_buf);
127-
128-
if (cwd().symLink(existing_path, tmp_path, .{})) {
129-
return cwd().rename(tmp_path, new_path);
130-
} else |err| switch (err) {
131-
error.PathAlreadyExists => continue,
132-
else => return err, // TODO zig should know this set does not include PathAlreadyExists
133-
}
134-
}
104+
/// Deprecated. Use `cwd().atomicSymLink()` instead.
105+
pub fn atomicSymLink(_: Allocator, existing_path: []const u8, new_path: []const u8) !void {
106+
try cwd().atomicSymLink(existing_path, new_path, .{});
135107
}
136108

137109
/// Same as `Dir.updateFile`, except asserts that both `source_path` and `dest_path`

lib/std/fs/Dir.zig

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1758,10 +1758,11 @@ pub fn renameW(self: Dir, old_sub_path_w: []const u16, new_sub_path_w: []const u
17581758
return posix.renameatW(self.fd, old_sub_path_w, self.fd, new_sub_path_w);
17591759
}
17601760

1761-
/// Use with `Dir.symLink` and `symLinkAbsolute` to specify whether the symlink
1762-
/// will point to a file or a directory. This value is ignored on all hosts
1763-
/// except Windows where creating symlinks to different resource types, requires
1764-
/// different flags. By default, `symLinkAbsolute` is assumed to point to a file.
1761+
/// Use with `Dir.symLink`, `Dir.atomicSymLink`, and `symLinkAbsolute` to
1762+
/// specify whether the symlink will point to a file or a directory. This value
1763+
/// is ignored on all hosts except Windows where creating symlinks to different
1764+
/// resource types, requires different flags. By default, `symLinkAbsolute` is
1765+
/// assumed to point to a file.
17651766
pub const SymLinkFlags = struct {
17661767
is_directory: bool = false,
17671768
};
@@ -1847,6 +1848,50 @@ pub fn symLinkW(
18471848
return windows.CreateSymbolicLink(self.fd, sym_link_path_w, target_path_w, flags.is_directory);
18481849
}
18491850

1851+
/// Same as `symLink`, except tries to create the symbolic link until it
1852+
/// succeeds or encounters an error other than `error.PathAlreadyExists`.
1853+
/// On Windows, both paths should be encoded as [WTF-8](https://simonsapin.github.io/wtf-8/).
1854+
/// On WASI, both paths should be encoded as valid UTF-8.
1855+
/// On other platforms, both paths are an opaque sequence of bytes with no particular encoding.
1856+
pub fn atomicSymLink(
1857+
dir: Dir,
1858+
target_path: []const u8,
1859+
sym_link_path: []const u8,
1860+
flags: SymLinkFlags,
1861+
) !void {
1862+
if (dir.symLink(target_path, sym_link_path, flags)) {
1863+
return;
1864+
} else |err| switch (err) {
1865+
error.PathAlreadyExists => {},
1866+
else => |e| return e,
1867+
}
1868+
1869+
const dirname = path.dirname(sym_link_path) orelse ".";
1870+
1871+
var rand_buf: [AtomicFile.random_bytes_len]u8 = undefined;
1872+
1873+
const temp_path_len = dirname.len + 1 + base64_encoder.calcSize(rand_buf.len);
1874+
var temp_path_buf: [fs.max_path_bytes]u8 = undefined;
1875+
1876+
if (temp_path_len > temp_path_buf.len) return error.NameTooLong;
1877+
@memcpy(temp_path_buf[0..dirname.len], dirname);
1878+
temp_path_buf[dirname.len] = path.sep;
1879+
1880+
const temp_path = temp_path_buf[0..temp_path_len];
1881+
1882+
while (true) {
1883+
crypto.random.bytes(rand_buf[0..]);
1884+
_ = base64_encoder.encode(temp_path[dirname.len + 1 ..], rand_buf[0..]);
1885+
1886+
if (dir.symLink(target_path, temp_path, flags)) {
1887+
return dir.rename(temp_path, sym_link_path);
1888+
} else |err| switch (err) {
1889+
error.PathAlreadyExists => continue,
1890+
else => |e| return e,
1891+
}
1892+
}
1893+
}
1894+
18501895
pub const ReadLinkError = posix.ReadLinkError;
18511896

18521897
/// Read value of a symbolic link.
@@ -2695,8 +2740,11 @@ const builtin = @import("builtin");
26952740
const std = @import("../std.zig");
26962741
const File = std.fs.File;
26972742
const AtomicFile = std.fs.AtomicFile;
2743+
const base64_encoder = fs.base64_encoder;
2744+
const crypto = std.crypto;
26982745
const posix = std.posix;
26992746
const mem = std.mem;
2747+
const path = fs.path;
27002748
const fs = std.fs;
27012749
const Allocator = std.mem.Allocator;
27022750
const assert = std.debug.assert;

0 commit comments

Comments
 (0)