Skip to content

Stop using std.Buffer in places that don't make sense #4405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 8 commits into from
1 change: 0 additions & 1 deletion build.zig
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ const BufMap = std.BufMap;
const warn = std.debug.warn;
const mem = std.mem;
const ArrayList = std.ArrayList;
const Buffer = std.Buffer;
const io = std.io;
const fs = std.fs;
const InstallDirectoryOptions = std.build.InstallDirectoryOptions;
Expand Down
15 changes: 15 additions & 0 deletions lib/std/array_list.zig
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,21 @@ pub fn AlignedArrayList(comptime T: type, comptime alignment: ?u29) type {
if (self.len == 0) return null;
return self.pop();
}

pub fn eql(self: Self, m: []const T) bool {
return mem.eql(T, self.toSliceConst(), m);
}

pub fn startsWith(self: Self, m: []const T) bool {
if (self.len < m.len) return false;
return mem.eql(T, self.items[0..m.len], m);
}

pub fn endsWith(self: Self, m: []const T) bool {
if (self.len < m.len) return false;
const start = self.len - m.len;
return mem.eql(T, self.items[start..self.len], m);
}
};
}

Expand Down
2 changes: 1 addition & 1 deletion lib/std/buffer.zig
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ pub const Buffer = struct {

pub fn startsWith(self: Buffer, m: []const u8) bool {
if (self.len() < m.len) return false;
return mem.eql(u8, self.list.items[0..m.len], m);
return self.list.startsWith(m);
}

pub fn endsWith(self: Buffer, m: []const u8) bool {
Expand Down
8 changes: 3 additions & 5 deletions lib/std/build.zig
Original file line number Diff line number Diff line change
Expand Up @@ -926,11 +926,9 @@ pub const Builder = struct {

try child.spawn();

var stdout = std.Buffer.initNull(self.allocator);
defer std.Buffer.deinit(&stdout);

var stdout_file_in_stream = child.stdout.?.inStream();
try stdout_file_in_stream.stream.readAllBuffer(&stdout, max_output_size);
const stdout = try stdout_file_in_stream.stream.readAllAlloc(self.allocator, max_output_size);
errdefer self.allocator.free(stdout);

const term = try child.wait();
switch (term) {
Expand All @@ -939,7 +937,7 @@ pub const Builder = struct {
out_code.* = @truncate(u8, code);
return error.ExitCodeFailure;
}
return stdout.toOwnedSlice();
return stdout;
},
.Signal, .Stopped, .Unknown => |code| {
out_code.* = @truncate(u8, code);
Expand Down
36 changes: 21 additions & 15 deletions lib/std/build/run.zig
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const mem = std.mem;
const process = std.process;
const ArrayList = std.ArrayList;
const BufMap = std.BufMap;
const Buffer = std.Buffer;
const warn = std.debug.warn;

const max_stdout_size = 1 * 1024 * 1024; // 1 MiB
Expand Down Expand Up @@ -169,26 +168,33 @@ pub const RunStep = struct {
return err;
};

var stdout = Buffer.initNull(self.builder.allocator);
var stderr = Buffer.initNull(self.builder.allocator);

// TODO need to poll to read these streams to prevent a deadlock (or rely on evented I/O).

var stdout: []const u8 = undefined;
switch (self.stdout_action) {
.expect_exact, .expect_matches => {
var stdout_file_in_stream = child.stdout.?.inStream();
stdout_file_in_stream.stream.readAllBuffer(&stdout, max_stdout_size) catch unreachable;
stdout = stdout_file_in_stream.stream.readAllAlloc(self.builder.allocator, max_stdout_size) catch unreachable;
},
.inherit, .ignore => {},
}
defer switch (self.stdout_action) {
.expect_exact, .expect_matches => self.builder.allocator.free(stdout),
.inherit, .ignore => {},
};

switch (self.stdout_action) {
var stderr: []const u8 = undefined;
switch (self.stderr_action) {
.expect_exact, .expect_matches => {
var stderr_file_in_stream = child.stderr.?.inStream();
stderr_file_in_stream.stream.readAllBuffer(&stderr, max_stdout_size) catch unreachable;
stderr = stderr_file_in_stream.stream.readAllAlloc(self.builder.allocator, max_stdout_size) catch unreachable;
},
.inherit, .ignore => {},
}
defer switch (self.stderr_action) {
.expect_exact, .expect_matches => self.builder.allocator.free(stderr),
.inherit, .ignore => {},
};

const term = child.wait() catch |err| {
warn("Unable to spawn {}: {}\n", .{ argv[0], @errorName(err) });
Expand Down Expand Up @@ -216,29 +222,29 @@ pub const RunStep = struct {
switch (self.stderr_action) {
.inherit, .ignore => {},
.expect_exact => |expected_bytes| {
if (!mem.eql(u8, expected_bytes, stderr.toSliceConst())) {
if (!mem.eql(u8, expected_bytes, stderr)) {
warn(
\\
\\========= Expected this stderr: =========
\\{}
\\========= But found: ====================
\\{}
\\
, .{ expected_bytes, stderr.toSliceConst() });
, .{ expected_bytes, stderr });
printCmd(cwd, argv);
return error.TestFailed;
}
},
.expect_matches => |matches| for (matches) |match| {
if (mem.indexOf(u8, stderr.toSliceConst(), match) == null) {
if (mem.indexOf(u8, stderr, match) == null) {
warn(
\\
\\========= Expected to find in stderr: =========
\\{}
\\========= But stderr does not contain it: =====
\\{}
\\
, .{ match, stderr.toSliceConst() });
, .{ match, stderr });
printCmd(cwd, argv);
return error.TestFailed;
}
Expand All @@ -248,29 +254,29 @@ pub const RunStep = struct {
switch (self.stdout_action) {
.inherit, .ignore => {},
.expect_exact => |expected_bytes| {
if (!mem.eql(u8, expected_bytes, stdout.toSliceConst())) {
if (!mem.eql(u8, expected_bytes, stdout)) {
warn(
\\
\\========= Expected this stdout: =========
\\{}
\\========= But found: ====================
\\{}
\\
, .{ expected_bytes, stdout.toSliceConst() });
, .{ expected_bytes, stdout });
printCmd(cwd, argv);
return error.TestFailed;
}
},
.expect_matches => |matches| for (matches) |match| {
if (mem.indexOf(u8, stdout.toSliceConst(), match) == null) {
if (mem.indexOf(u8, stdout, match) == null) {
warn(
\\
\\========= Expected to find in stdout: =========
\\{}
\\========= But stdout does not contain it: =====
\\{}
\\
, .{ match, stdout.toSliceConst() });
, .{ match, stdout });
printCmd(cwd, argv);
return error.TestFailed;
}
Expand Down
16 changes: 7 additions & 9 deletions lib/std/child_process.zig
Original file line number Diff line number Diff line change
Expand Up @@ -217,21 +217,19 @@ pub const ChildProcess = struct {

try child.spawn();

var stdout = Buffer.initNull(args.allocator);
var stderr = Buffer.initNull(args.allocator);
defer Buffer.deinit(&stdout);
defer Buffer.deinit(&stderr);

var stdout_file_in_stream = child.stdout.?.inStream();
var stderr_file_in_stream = child.stderr.?.inStream();

try stdout_file_in_stream.stream.readAllBuffer(&stdout, args.max_output_bytes);
try stderr_file_in_stream.stream.readAllBuffer(&stderr, args.max_output_bytes);
// TODO need to poll to read these streams to prevent a deadlock (or rely on evented I/O).
const stdout = try stdout_file_in_stream.stream.readAllAlloc(args.allocator, args.max_output_bytes);
errdefer args.allocator.free(stdout);
const stderr = try stderr_file_in_stream.stream.readAllAlloc(args.allocator, args.max_output_bytes);
errdefer args.allocator.free(stderr);

return ExecResult{
.term = try child.wait(),
.stdout = stdout.toOwnedSlice(),
.stderr = stderr.toOwnedSlice(),
.stdout = stdout,
.stderr = stderr,
};
}

Expand Down
22 changes: 11 additions & 11 deletions lib/std/fmt.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1633,10 +1633,10 @@ test "hexToBytes" {
test "formatIntValue with comptime_int" {
const value: comptime_int = 123456789123456789;

var buf = try std.Buffer.init(std.testing.allocator, "");
var buf = std.ArrayList(u8).init(std.testing.allocator);
defer buf.deinit();
try formatIntValue(value, "", FormatOptions{}, &buf, @TypeOf(std.Buffer.append).ReturnType.ErrorSet, std.Buffer.append);
std.testing.expect(mem.eql(u8, buf.toSlice(), "123456789123456789"));
try formatIntValue(value, "", FormatOptions{}, &buf, @TypeOf(std.ArrayList(u8).appendSlice).ReturnType.ErrorSet, std.ArrayList(u8).appendSlice);
std.testing.expect(mem.eql(u8, buf.toSliceConst(), "123456789123456789"));
}

test "formatType max_depth" {
Expand Down Expand Up @@ -1688,24 +1688,24 @@ test "formatType max_depth" {
inst.a = &inst;
inst.tu.ptr = &inst.tu;

var buf0 = try std.Buffer.init(std.testing.allocator, "");
var buf0 = std.ArrayList(u8).init(std.testing.allocator);
defer buf0.deinit();
try formatType(inst, "", FormatOptions{}, &buf0, @TypeOf(std.Buffer.append).ReturnType.ErrorSet, std.Buffer.append, 0);
try formatType(inst, "", FormatOptions{}, &buf0, @TypeOf(std.ArrayList(u8).appendSlice).ReturnType.ErrorSet, std.ArrayList(u8).appendSlice, 0);
std.testing.expect(mem.eql(u8, buf0.toSlice(), "S{ ... }"));

var buf1 = try std.Buffer.init(std.testing.allocator, "");
var buf1 = std.ArrayList(u8).init(std.testing.allocator);
defer buf1.deinit();
try formatType(inst, "", FormatOptions{}, &buf1, @TypeOf(std.Buffer.append).ReturnType.ErrorSet, std.Buffer.append, 1);
try formatType(inst, "", FormatOptions{}, &buf1, @TypeOf(std.ArrayList(u8).appendSlice).ReturnType.ErrorSet, std.ArrayList(u8).appendSlice, 1);
std.testing.expect(mem.eql(u8, buf1.toSlice(), "S{ .a = S{ ... }, .tu = TU{ ... }, .e = E.Two, .vec = (10.200,2.220) }"));

var buf2 = try std.Buffer.init(std.testing.allocator, "");
var buf2 = std.ArrayList(u8).init(std.testing.allocator);
defer buf2.deinit();
try formatType(inst, "", FormatOptions{}, &buf2, @TypeOf(std.Buffer.append).ReturnType.ErrorSet, std.Buffer.append, 2);
try formatType(inst, "", FormatOptions{}, &buf2, @TypeOf(std.ArrayList(u8).appendSlice).ReturnType.ErrorSet, std.ArrayList(u8).appendSlice, 2);
std.testing.expect(mem.eql(u8, buf2.toSlice(), "S{ .a = S{ .a = S{ ... }, .tu = TU{ ... }, .e = E.Two, .vec = (10.200,2.220) }, .tu = TU{ .ptr = TU{ ... } }, .e = E.Two, .vec = (10.200,2.220) }"));

var buf3 = try std.Buffer.init(std.testing.allocator, "");
var buf3 = std.ArrayList(u8).init(std.testing.allocator);
defer buf3.deinit();
try formatType(inst, "", FormatOptions{}, &buf3, @TypeOf(std.Buffer.append).ReturnType.ErrorSet, std.Buffer.append, 3);
try formatType(inst, "", FormatOptions{}, &buf3, @TypeOf(std.ArrayList(u8).appendSlice).ReturnType.ErrorSet, std.ArrayList(u8).appendSlice, 3);
std.testing.expect(mem.eql(u8, buf3.toSlice(), "S{ .a = S{ .a = S{ .a = S{ ... }, .tu = TU{ ... }, .e = E.Two, .vec = (10.200,2.220) }, .tu = TU{ .ptr = TU{ ... } }, .e = E.Two, .vec = (10.200,2.220) }, .tu = TU{ .ptr = TU{ .ptr = TU{ ... } } }, .e = E.Two, .vec = (10.200,2.220) }"));
}

Expand Down
51 changes: 34 additions & 17 deletions lib/std/io/in_stream.zig
Original file line number Diff line number Diff line change
Expand Up @@ -65,22 +65,31 @@ pub fn InStream(comptime ReadError: type) type {
/// If `buffer.len()` would exceed `max_size`, `error.StreamTooLong` is returned and
/// the contents read from the stream are lost.
pub fn readAllBuffer(self: *Self, buffer: *Buffer, max_size: usize) !void {
try buffer.resize(0);
try buffer.list.ensureCapacity(1);
buffer.list.len = 0;
errdefer buffer.resize(0) catch unreachable; // make sure we leave buffer in a valid state on error
try self.readAllArrayList(&buffer.list, max_size);
try buffer.list.append(0);
}

/// Appends to the ArrayList contents by reading from the stream until end of stream is found.
/// If the ArrayList length would exceed `max_size`, `error.StreamTooLong` is returned and the contents
/// read from the stream so far are lost.
pub fn readAllArrayList(self: *Self, array_list: *std.ArrayList(u8), max_size: usize) !void {
var actual_buf_len: usize = 0;
while (true) {
const dest_slice = buffer.toSlice()[actual_buf_len..];
const dest_slice = array_list.toSlice()[actual_buf_len..];
const bytes_read = try self.readFull(dest_slice);
actual_buf_len += bytes_read;

if (bytes_read != dest_slice.len) {
buffer.shrink(actual_buf_len);
array_list.shrink(actual_buf_len);
return;
}

const new_buf_size = math.min(max_size, actual_buf_len + mem.page_size);
if (new_buf_size == actual_buf_len) return error.StreamTooLong;
try buffer.resize(new_buf_size);
try array_list.resize(new_buf_size);
}
}

Expand All @@ -89,32 +98,41 @@ pub fn InStream(comptime ReadError: type) type {
/// Caller owns returned memory.
/// If this function returns an error, the contents from the stream read so far are lost.
pub fn readAllAlloc(self: *Self, allocator: *mem.Allocator, max_size: usize) ![]u8 {
var buf = Buffer.initNull(allocator);
defer buf.deinit();

try self.readAllBuffer(&buf, max_size);
return buf.toOwnedSlice();
var array_list = std.ArrayList(u8).init(allocator);
defer array_list.deinit();
try self.readAllArrayList(&array_list, max_size);
return array_list.toOwnedSlice();
}

/// Replaces `buffer` contents by reading from the stream until `delimiter` is found.
/// Does not include the delimiter in the result.
/// If `buffer.len()` would exceed `max_size`, `error.StreamTooLong` is returned and the contents
/// read from the stream so far are lost.
pub fn readUntilDelimiterBuffer(self: *Self, buffer: *Buffer, delimiter: u8, max_size: usize) !void {
try buffer.resize(0);
try buffer.list.ensureCapacity(1);
buffer.list.len = 0;
errdefer buffer.resize(0) catch unreachable; // make sure we leave buffer in a valid state on error
try self.readUntilDelimiterArrayList(&buffer.list, delimiter, max_size);
try buffer.list.append(0);
}

/// Appends to the ArrayList contents by reading from the stream until `delimiter` is found.
/// Does not include the delimiter in the result.
/// If the ArrayList length would exceed `max_size`, `error.StreamTooLong` is returned and the contents
/// read from the stream so far are lost.
pub fn readUntilDelimiterArrayList(self: *Self, array_list: *std.ArrayList(u8), delimiter: u8, max_size: usize) !void {
while (true) {
var byte: u8 = try self.readByte();

if (byte == delimiter) {
return;
}

if (buffer.len() == max_size) {
if (array_list.len == max_size) {
return error.StreamTooLong;
}

try buffer.appendByte(byte);
try array_list.append(byte);
}
}

Expand All @@ -123,11 +141,10 @@ pub fn InStream(comptime ReadError: type) type {
/// Caller owns returned memory.
/// If this function returns an error, the contents from the stream read so far are lost.
pub fn readUntilDelimiterAlloc(self: *Self, allocator: *mem.Allocator, delimiter: u8, max_size: usize) ![]u8 {
var buf = Buffer.initNull(allocator);
defer buf.deinit();

try self.readUntilDelimiterBuffer(&buf, delimiter, max_size);
return buf.toOwnedSlice();
var array_list = std.ArrayList(u8).init(allocator);
defer array_list.deinit();
try self.readUntilDelimiterArrayList(&array_list, delimiter, max_size);
return array_list.toOwnedSlice();
}

/// Reads from the stream until specified byte is found. If the buffer is not
Expand Down
Loading