From 7b417c6caf13e07d05bf798cae2f8beeaee4fe6c Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 27 May 2025 20:41:36 -0700 Subject: [PATCH] std: improve the discarding writer by making the vtable use File.Reader instead of File and Offset --- lib/std/Build/Step/Run.zig | 2 +- lib/std/compress/flate/Compress.zig | 25 +------ lib/std/compress/zstd/Decompress.zig | 25 +------ lib/std/crypto/phc_encoding.zig | 6 +- lib/std/crypto/scrypt.zig | 6 +- lib/std/fmt.zig | 12 ++- lib/std/fs/File.zig | 2 +- lib/std/io/BufferedReader.zig | 46 +----------- lib/std/io/Writer.zig | 108 ++++++++++++++------------- lib/std/io/Writer/Null.zig | 70 ----------------- lib/std/zon/stringify.zig | 6 +- 11 files changed, 82 insertions(+), 226 deletions(-) delete mode 100644 lib/std/io/Writer/Null.zig diff --git a/lib/std/Build/Step/Run.zig b/lib/std/Build/Step/Run.zig index 7371820a8f..ef954f77e8 100644 --- a/lib/std/Build/Step/Run.zig +++ b/lib/std/Build/Step/Run.zig @@ -78,7 +78,7 @@ max_stdio_size: usize, /// If stderr or stdout exceeds this amount, the child process is killed and /// the step fails. -stdio_limit: std.io.Reader.Limit, +stdio_limit: std.io.Limit, captured_stdout: ?*Output, captured_stderr: ?*Output, diff --git a/lib/std/compress/flate/Compress.zig b/lib/std/compress/flate/Compress.zig index bc54128f05..634d247ee7 100644 --- a/lib/std/compress/flate/Compress.zig +++ b/lib/std/compress/flate/Compress.zig @@ -73,11 +73,7 @@ pub fn readable(c: *Compress, buffer: []u8) std.io.BufferedReader { return .{ .unbuffered_reader = .{ .context = c, - .vtable = .{ - .read = read, - .readVec = readVec, - .discard = discard, - }, + .vtable = .{ .read = read }, }, .buffer = buffer, }; @@ -863,25 +859,6 @@ fn read( } } -fn readVec(context: ?*anyopaque, data: []const []u8) std.io.Reader.Error!usize { - var bw: std.io.BufferedWriter = undefined; - bw.initVec(data); - return read(context, &bw, .countVec(data)) catch |err| switch (err) { - error.WriteFailed => unreachable, // Prevented by the limit. - else => |e| return e, - }; -} - -fn discard(context: ?*anyopaque, limit: std.io.Reader.Limit) std.io.Reader.Error!usize { - var trash_buffer: [64]u8 = undefined; - var null_writer: std.io.Writer.Null = undefined; - var bw = null_writer.writer().buffered(&trash_buffer); - return read(context, &bw, limit) catch |err| switch (err) { - error.WriteFailed => unreachable, - else => |e| return e, - }; -} - test "generate a Huffman code from an array of frequencies" { var freqs: [19]u16 = [_]u16{ 8, // 0 diff --git a/lib/std/compress/zstd/Decompress.zig b/lib/std/compress/zstd/Decompress.zig index b477029dbc..92e076daad 100644 --- a/lib/std/compress/zstd/Decompress.zig +++ b/lib/std/compress/zstd/Decompress.zig @@ -73,11 +73,7 @@ pub fn init(input: *BufferedReader, options: Options) Decompress { pub fn reader(self: *Decompress) Reader { return .{ .context = self, - .vtable = &.{ - .read = read, - .readVec = readVec, - .discard = discard, - }, + .vtable = &.{ .read = read }, }; } @@ -255,25 +251,6 @@ fn readInFrame(d: *Decompress, bw: *BufferedWriter, limit: Reader.Limit, state: return bytes_written; } -fn discard(context: ?*anyopaque, limit: Reader.Limit) Reader.Error!usize { - var trash_buffer: [64]u8 = undefined; - var null_writer: std.io.Writer.Null = undefined; - var bw = null_writer.writer().buffered(&trash_buffer); - return read(context, &bw, limit) catch |err| switch (err) { - error.WriteFailed => unreachable, - else => |e| return e, - }; -} - -fn readVec(context: ?*anyopaque, data: []const []u8) Reader.Error!usize { - var bw: BufferedWriter = undefined; - bw.initVec(data); - return read(context, &bw, .countVec(data)) catch |err| switch (err) { - error.WriteFailed => unreachable, - else => |e| return e, - }; -} - pub const Frame = struct { hasher_opt: ?std.hash.XxHash64, window_size: usize, diff --git a/lib/std/crypto/phc_encoding.zig b/lib/std/crypto/phc_encoding.zig index c23702747a..690680991f 100644 --- a/lib/std/crypto/phc_encoding.zig +++ b/lib/std/crypto/phc_encoding.zig @@ -196,9 +196,11 @@ pub fn serialize(params: anytype, str: []u8) Error![]const u8 { /// Compute the number of bytes required to serialize `params` pub fn calcSize(params: anytype) usize { - var null_writer: std.io.Writer.Null = .{}; var trash: [128]u8 = undefined; - var bw = null_writer.writable(&trash); + var bw: std.io.BufferedWriter = .{ + .unbuffered_writer = .discarding, + .buffer = &trash, + }; serializeTo(params, &bw) catch unreachable; return bw.count; } diff --git a/lib/std/crypto/scrypt.zig b/lib/std/crypto/scrypt.zig index c2b5976d70..b47a979c24 100644 --- a/lib/std/crypto/scrypt.zig +++ b/lib/std/crypto/scrypt.zig @@ -312,9 +312,11 @@ const crypt_format = struct { /// Compute the number of bytes required to serialize `params` pub fn calcSize(params: anytype) usize { - var null_writer: std.io.Writer.Null = .{}; var trash: [64]u8 = undefined; - var bw = null_writer.writer().buffered(&trash); + var bw: std.io.BufferedWriter = .{ + .unbuffered_writer = .discarding, + .buffer = &trash, + }; serializeTo(params, &bw) catch |err| switch (err) { error.WriteFailed => unreachable, }; diff --git a/lib/std/fmt.zig b/lib/std/fmt.zig index 30abafc876..b9f1daa131 100644 --- a/lib/std/fmt.zig +++ b/lib/std/fmt.zig @@ -845,10 +845,14 @@ pub fn bufPrintZ(buf: []u8, comptime fmt: []const u8, args: anytype) BufPrintErr /// Count the characters needed for format. pub fn count(comptime fmt: []const u8, args: anytype) usize { - var trash_buffer: [std.atomic.cache_line]u8 = undefined; - var null_writer: std.io.Writer.Null = undefined; - var bw = null_writer.writer().buffered(&trash_buffer); - bw.print(fmt, args) catch unreachable; + var trash_buffer: [64]u8 = undefined; + var bw: std.io.BufferedWriter = .{ + .unbuffered_writer = .discarding, + .buffer = &trash_buffer, + }; + bw.print(fmt, args) catch |err| switch (err) { + error.WriteFailed => unreachable, + }; return bw.count; } diff --git a/lib/std/fs/File.zig b/lib/std/fs/File.zig index 4f530656e2..14ad87ae79 100644 --- a/lib/std/fs/File.zig +++ b/lib/std/fs/File.zig @@ -1361,7 +1361,7 @@ pub const Writer = struct { context: ?*anyopaque, in_file: std.fs.File, in_offset: std.io.Writer.Offset, - in_limit: std.io.Writer.Limit, + in_limit: std.io.Limit, headers_and_trailers: []const []const u8, headers_len: usize, ) std.io.Writer.FileError!usize { diff --git a/lib/std/io/BufferedReader.zig b/lib/std/io/BufferedReader.zig index 474256ca3a..8d43901004 100644 --- a/lib/std/io/BufferedReader.zig +++ b/lib/std/io/BufferedReader.zig @@ -203,13 +203,7 @@ fn defaultDiscard(br: *BufferedReader, limit: Limit) Reader.Error!usize { assert(br.seek == 0); assert(br.end == 0); var bw: BufferedWriter = .{ - .unbuffered_writer = .{ - .context = undefined, - .vtable = &.{ - .writeSplat = defaultDiscardWriteSplat, - .writeFile = defaultDiscardWriteFile, - }, - }, + .unbuffered_writer = .discarding, .buffer = br.buffer, }; const n = br.read(&bw, limit) catch |err| switch (err) { @@ -227,44 +221,6 @@ fn defaultDiscard(br: *BufferedReader, limit: Limit) Reader.Error!usize { return n; } -fn defaultDiscardWriteSplat(context: ?*anyopaque, data: []const []const u8, splat: usize) Writer.Error!usize { - _ = context; - const headers = data[0 .. data.len - 1]; - const pattern = data[headers.len..]; - var written: usize = pattern.len * splat; - for (headers) |bytes| written += bytes.len; - return written; -} - -fn defaultDiscardWriteFile( - context: ?*anyopaque, - file_reader: *std.fs.File.Reader, - limit: Limit, - headers_and_trailers: []const []const u8, - headers_len: usize, -) Writer.FileError!usize { - _ = context; - if (file_reader.getSize()) |size| { - const remaining = size - file_reader.pos; - const seek_amt = limit.minInt(remaining); - // Error is observable on `file_reader` instance, and is safe to ignore - // depending on the caller's needs. Caller can make that decision. - file_reader.seekForward(seek_amt) catch {}; - var n: usize = seek_amt; - for (headers_and_trailers[0..headers_len]) |bytes| n += bytes.len; - if (seek_amt == remaining) { - // Since we made it all the way through the file, the trailers are - // also included. - for (headers_and_trailers[headers_len..]) |bytes| n += bytes.len; - } - return n; - } else |_| { - // Error is observable on `file_reader` instance, and it is better to - // treat the file as a pipe. - return error.Unimplemented; - } -} - /// Returns the next `len` bytes from `unbuffered_reader`, filling the buffer as /// necessary. /// diff --git a/lib/std/io/Writer.zig b/lib/std/io/Writer.zig index 56b6d02f57..82aa3d1ab5 100644 --- a/lib/std/io/Writer.zig +++ b/lib/std/io/Writer.zig @@ -2,8 +2,7 @@ const std = @import("../std.zig"); const assert = std.debug.assert; const Writer = @This(); const Limit = std.io.Limit; - -pub const Null = @import("Writer/Null.zig"); +const File = std.fs.File; context: ?*anyopaque, vtable: *const VTable, @@ -37,15 +36,7 @@ pub const VTable = struct { /// efficient implementation. writeFile: *const fn ( ctx: ?*anyopaque, - file: std.fs.File, - /// If this is `Offset.none`, `file` will be streamed, affecting the - /// seek position. Otherwise, it will be read positionally without - /// affecting the seek position. `error.Unseekable` is only possible - /// when reading positionally. - /// - /// An offset past the end of the file is treated the same as an offset - /// equal to the end of the file. - offset: Offset, + file_reader: *File.Reader, /// Maximum amount of bytes to read from the file. Implementations may /// assume that the file size does not exceed this amount. /// @@ -63,7 +54,9 @@ pub const Error = error{ WriteFailed, }; -pub const FileError = std.fs.File.PReadError || error{ +pub const FileError = error{ + /// Detailed diagnostics are found on the `File.Reader` struct. + ReadFailed, /// See the `Writer` implementation for detailed diagnostics. WriteFailed, /// Indicates the caller should do its own file reading; the callee cannot @@ -71,30 +64,6 @@ pub const FileError = std.fs.File.PReadError || error{ Unimplemented, }; -pub const Offset = enum(u64) { - zero = 0, - /// Indicates to read the file as a stream. - none = std.math.maxInt(u64), - _, - - pub fn init(integer: u64) Offset { - const result: Offset = @enumFromInt(integer); - assert(result != .none); - return result; - } - - pub fn toInt(o: Offset) ?u64 { - return if (o == .none) null else @intFromEnum(o); - } - - pub fn advance(o: Offset, amount: u64) Offset { - return switch (o) { - .none => .none, - else => .init(@intFromEnum(o) + amount), - }; - } -}; - pub fn writeVec(w: Writer, data: []const []const u8) Error!usize { assert(data.len > 0); return w.vtable.writeSplat(w.context, data, 1); @@ -107,13 +76,12 @@ pub fn writeSplat(w: Writer, data: []const []const u8, splat: usize) Error!usize pub fn writeFile( w: Writer, - file: std.fs.File, - offset: Offset, + file_reader: *File.Reader, limit: Limit, headers_and_trailers: []const []const u8, headers_len: usize, ) FileError!usize { - return w.vtable.writeFile(w.context, file, offset, limit, headers_and_trailers, headers_len); + return w.vtable.writeFile(w.context, file_reader, limit, headers_and_trailers, headers_len); } pub fn buffered(w: Writer, buffer: []u8) std.io.BufferedWriter { @@ -136,15 +104,13 @@ pub fn failingWriteSplat(context: ?*anyopaque, data: []const []const u8, splat: pub fn failingWriteFile( context: ?*anyopaque, - file: std.fs.File, - offset: Offset, + file_reader: *File.Reader, limit: Limit, headers_and_trailers: []const []const u8, headers_len: usize, ) FileError!usize { _ = context; - _ = file; - _ = offset; + _ = file_reader; _ = limit; _ = headers_and_trailers; _ = headers_len; @@ -159,19 +125,63 @@ pub const failing: Writer = .{ }, }; +pub fn discardingWriteSplat(context: ?*anyopaque, data: []const []const u8, splat: usize) Error!usize { + _ = context; + const headers = data[0 .. data.len - 1]; + const pattern = data[headers.len..]; + var written: usize = pattern.len * splat; + for (headers) |bytes| written += bytes.len; + return written; +} + +pub fn discardingWriteFile( + context: ?*anyopaque, + file_reader: *std.fs.File.Reader, + limit: Limit, + headers_and_trailers: []const []const u8, + headers_len: usize, +) Writer.FileError!usize { + _ = context; + if (file_reader.getSize()) |size| { + const remaining = size - file_reader.pos; + const seek_amt = limit.minInt(remaining); + // Error is observable on `file_reader` instance, and is safe to ignore + // depending on the caller's needs. Caller can make that decision. + file_reader.seekForward(seek_amt) catch {}; + var n: usize = seek_amt; + for (headers_and_trailers[0..headers_len]) |bytes| n += bytes.len; + if (seek_amt == remaining) { + // Since we made it all the way through the file, the trailers are + // also included. + for (headers_and_trailers[headers_len..]) |bytes| n += bytes.len; + } + return n; + } else |_| { + // Error is observable on `file_reader` instance, and it is better to + // treat the file as a pipe. + return error.Unimplemented; + } +} + +pub const discarding: Writer = .{ + .context = undefined, + .vtable = &.{ + .writeSplat = discardingWriteSplat, + .writeFile = discardingWriteFile, + }, +}; + /// For use when the `Writer` implementation can cannot offer a more efficient /// implementation than a basic read/write loop on the file. pub fn unimplementedWriteFile( context: ?*anyopaque, - file: std.fs.File, - offset: Offset, + file_reader: *File.Reader, limit: Limit, headers_and_trailers: []const []const u8, headers_len: usize, ) FileError!usize { _ = context; - _ = file; - _ = offset; + _ = file_reader; _ = limit; _ = headers_and_trailers; _ = headers_len; @@ -255,7 +265,3 @@ pub fn Hashed(comptime Hasher: type) type { } }; } - -test { - _ = Null; -} diff --git a/lib/std/io/Writer/Null.zig b/lib/std/io/Writer/Null.zig deleted file mode 100644 index 22616bdfd7..0000000000 --- a/lib/std/io/Writer/Null.zig +++ /dev/null @@ -1,70 +0,0 @@ -//! A `Writer` that discards all data. - -const std = @import("../../std.zig"); -const Writer = std.io.Writer; - -const NullWriter = @This(); - -err: ?Error = null, - -pub const Error = std.fs.File.StatError; - -pub fn writer(nw: *NullWriter) Writer { - return .{ - .context = nw, - .vtable = &.{ - .writeSplat = writeSplat, - .writeFile = writeFile, - }, - }; -} - -pub fn writable(nw: *NullWriter, buffer: []u8) std.io.BufferedWriter { - return writer(nw).buffered(buffer); -} - -fn writeSplat(context: ?*anyopaque, data: []const []const u8, splat: usize) Writer.Error!usize { - _ = context; - const headers = data[0 .. data.len - 1]; - const pattern = data[headers.len..]; - var written: usize = pattern.len * splat; - for (headers) |bytes| written += bytes.len; - return written; -} - -fn writeFile( - context: ?*anyopaque, - file: std.fs.File, - offset: Writer.Offset, - limit: Writer.Limit, - headers_and_trailers: []const []const u8, - headers_len: usize, -) Writer.FileError!usize { - const nw: *NullWriter = @alignCast(@ptrCast(context)); - var n: usize = 0; - if (offset == .none) { - @panic("TODO seek the file forwards"); - } - const limit_int = limit.toInt() orelse { - const headers = headers_and_trailers[0..headers_len]; - for (headers) |bytes| n += bytes.len; - if (offset.toInt()) |off| { - const stat = file.stat() catch |err| { - nw.err = err; - return error.WriteFailed; - }; - n += stat.size - off; - for (headers_and_trailers[headers_len..]) |bytes| n += bytes.len; - return n; - } - @panic("TODO stream from file until eof, counting"); - }; - for (headers_and_trailers) |bytes| n += bytes.len; - return limit_int + n; -} - -test "writing a small string" { - var nw: NullWriter = undefined; - var bw = nw.writer().unbuffered(); - try bw.writeAll("yay"); -} diff --git a/lib/std/zon/stringify.zig b/lib/std/zon/stringify.zig index 434a33eee7..d0d9a8e74f 100644 --- a/lib/std/zon/stringify.zig +++ b/lib/std/zon/stringify.zig @@ -1040,8 +1040,10 @@ pub const Serializer = struct { }; test Serializer { - var null_writer: std.io.Writer.Null = undefined; - var bw = null_writer.writer().unbuffered(); + var bw: std.io.BufferedWriter = .{ + .unbuffered_writer = .discarding, + .buffer = &.{}, + }; var s: Serializer = .{ .writer = &bw }; var vec2 = try s.beginStruct(.{}); try vec2.field("x", 1.5, .{});