authorgravatar for ragul542rv@gmail.comRagul R <ragul542rv@gmail.com> 2026-08-25 12:15:08+02:00
committergravatar for squeek502@hotmail.comRyan Liptak <squeek502@hotmail.com> 2026-08-25 12:15:08+02:00
log58e556fb7ed7bc50481ac0e6fa066d1aeb10bdda
tree90cafa230eb286ad766d218d5eaab76ec4843764
parent841dd0eb874cf0febf9582031f86703f5cde00cb

Fix: unnecessary vtable.drain calls in writeSplatHeaderLimit (#36611)

Closes: https://codeberg.org/ziglang/zig/issues/36583 vtable.drain is only called when the limited data would not fit into the buffer. Fixing this unnecessary call also resolved the `writeSplatHeaderLimit` panic I also added an assert resembling the other functions. Reviewed-on: https://codeberg.org/ziglang/zig/pulls/36611 Reviewed-by: Ryan Liptak <squeek502@noreply.codeberg.org>

1 files changed, 71 insertions(+), 21 deletions(-)

lib/std/Io/Writer.zig+71-21
...@@ -221,35 +221,42 @@ pub fn writeSplatHeaderLimit(...@@ -221,35 +221,42 @@ pub fn writeSplatHeaderLimit(
221 limit: Limit,221 limit: Limit,
222) Error!usize {222) Error!usize {
223 var remaining = @backingInt(limit);223 var remaining = @backingInt(limit);
224 assert(data.len > 0);
224 {225 {
225 const copy_len = @min(header.len, w.buffer.len - w.end, remaining);226 const copy_len = @min(header.len, remaining);
226 if (header.len - copy_len != 0) return writeSplatHeaderLimitFinish(w, header, data, splat, remaining);227 if (w.buffer.len - w.end < copy_len) return try writeSplatHeaderLimitFinish(w, header, data, splat, remaining);
227 @memcpy(w.buffer[w.end..][0..copy_len], header[0..copy_len]);228 @memcpy(w.buffer[w.end..][0..copy_len], header[0..copy_len]);
228 w.end += copy_len;229 w.end += copy_len;
229 remaining -= copy_len;230 remaining -= copy_len;
230 }231 }
231 for (data[0 .. data.len - 1], 0..) |buf, i| {
232 const copy_len = @min(buf.len, w.buffer.len - w.end, remaining);
233 if (buf.len - copy_len != 0) return @backingInt(limit) - remaining +
234 try writeSplatHeaderLimitFinish(w, &.{}, data[i..], splat, remaining);
235 @memcpy(w.buffer[w.end..][0..copy_len], buf[0..copy_len]);
236 w.end += copy_len;
237 remaining -= copy_len;
238 }
239 const pattern = data[data.len - 1];
240 const splat_n = pattern.len * splat;
241 if (splat_n > @min(w.buffer.len - w.end, remaining)) {
242 const buffered_n = @backingInt(limit) - remaining;
243 const written = try writeSplatHeaderLimitFinish(w, &.{}, data[data.len - 1 ..][0..1], splat, remaining);
244 return buffered_n + written;
245 }
246232
247 for (0..splat) |_| {233 remaining_zero: {
248 @memcpy(w.buffer[w.end..][0..pattern.len], pattern);234 if (remaining == 0) break :remaining_zero;
249 w.end += pattern.len;235 for (data[0 .. data.len - 1], 0..) |bytes, i| {
236 const copy_len = @min(bytes.len, remaining);
237 if (w.buffer.len - w.end < copy_len) {
238 const n = try writeSplatHeaderLimitFinish(w, &.{}, data[i..], splat, remaining);
239 return @backingInt(limit) - remaining + n;
240 }
241 @memcpy(w.buffer[w.end..][0..copy_len], bytes[0..copy_len]);
242 w.end += copy_len;
243 remaining -= copy_len;
244 }
245
246 if (remaining == 0) break :remaining_zero;
247 const pattern = data[data.len - 1];
248 for (0..splat) |_| {
249 const copy_len = @min(pattern.len, remaining);
250 if (w.buffer.len - w.end < copy_len) {
251 const n = try writeSplatHeaderLimitFinish(w, &.{}, data[data.len - 1 ..][0..1], splat, remaining);
252 return @backingInt(limit) - remaining + n;
253 }
254 @memcpy(w.buffer[w.end..][0..copy_len], pattern[0..copy_len]);
255 w.end += copy_len;
256 remaining -= copy_len;
257 }
250 }258 }
251259
252 remaining -= splat_n;
253 return @backingInt(limit) - remaining;260 return @backingInt(limit) - remaining;
254}261}
255262
...@@ -293,6 +300,49 @@ fn writeSplatHeaderLimitFinish(...@@ -293,6 +300,49 @@ fn writeSplatHeaderLimitFinish(
293 return w.vtable.drain(w, (&vecs)[0..i], 1);300 return w.vtable.drain(w, (&vecs)[0..i], 1);
294}301}
295302
303const FixedSplatHeaderTestCase = struct {
304 buf_len: usize = 100,
305 header: []const u8,
306 data: []const []const u8,
307 splat: u8,
308 limit: u8,
309 expected_res: union(enum) { written: usize, write_failed },
310 expected_buf_content: []const u8,
311};
312
313fn testFixedWriteSplatHeaderLimit(comptime test_case: FixedSplatHeaderTestCase) !void {
314 var buf: [test_case.buf_len]u8 = @splat(0);
315 var w: std.Io.Writer = .fixed(&buf);
316 const n_or_error = w.writeSplatHeaderLimit(test_case.header, test_case.data, test_case.splat, .limited(test_case.limit));
317 switch (test_case.expected_res) {
318 .written => |expected_len| {
319 const n = try n_or_error;
320 try std.testing.expectEqual(expected_len, n);
321 },
322 .write_failed => {
323 try std.testing.expectError(error.WriteFailed, n_or_error);
324 },
325 }
326 try std.testing.expectEqualStrings(test_case.expected_buf_content, w.buffered());
327}
328
329test "fixed writer writeSplatHeaderLimit" {
330 // buffer is large
331 try testFixedWriteSplatHeaderLimit(.{ .header = "header is longer", .data = &.{""}, .splat = 1, .limit = 6, .expected_res = .{ .written = 6 }, .expected_buf_content = "header" });
332 try testFixedWriteSplatHeaderLimit(.{ .header = "head", .data = &.{"123456"}, .splat = 1, .limit = 5, .expected_res = .{ .written = 5 }, .expected_buf_content = "head1" });
333 try testFixedWriteSplatHeaderLimit(.{ .header = "head", .data = &.{"123"}, .splat = 1, .limit = 10, .expected_res = .{ .written = 7 }, .expected_buf_content = "head123" });
334 try testFixedWriteSplatHeaderLimit(.{ .header = "head", .data = &.{ "1", "abcdefg" }, .splat = 1, .limit = 6, .expected_res = .{ .written = 6 }, .expected_buf_content = "head1a" });
335 try testFixedWriteSplatHeaderLimit(.{ .header = "head", .data = &.{ "123", "abc" }, .splat = 2, .limit = 6, .expected_res = .{ .written = 6 }, .expected_buf_content = "head12" });
336 try testFixedWriteSplatHeaderLimit(.{ .header = "head", .data = &.{ "123", "abc" }, .splat = 2, .limit = 11, .expected_res = .{ .written = 11 }, .expected_buf_content = "head123abca" });
337 try testFixedWriteSplatHeaderLimit(.{ .header = "head", .data = &.{ "123", "a" }, .splat = 2, .limit = 10, .expected_res = .{ .written = 9 }, .expected_buf_content = "head123aa" });
338 try testFixedWriteSplatHeaderLimit(.{ .header = "head", .data = &.{ "123", "abc" }, .splat = 2, .limit = 100, .expected_res = .{ .written = 13 }, .expected_buf_content = "head123abcabc" });
339
340 // buffer is small
341 try testFixedWriteSplatHeaderLimit(.{ .header = "header is longer", .data = &.{""}, .splat = 1, .limit = 6, .expected_res = .write_failed, .expected_buf_content = "head", .buf_len = 4 });
342 try testFixedWriteSplatHeaderLimit(.{ .header = "head", .data = &.{"123456"}, .splat = 1, .limit = 8, .expected_res = .write_failed, .expected_buf_content = "head1", .buf_len = 5 });
343 try testFixedWriteSplatHeaderLimit(.{ .header = "head", .data = &.{ "123", "ab" }, .splat = 2, .limit = 100, .expected_res = .write_failed, .expected_buf_content = "head123aba", .buf_len = 10 });
344}
345
296test "writeSplatHeader splatting avoids buffer aliasing temptation" {346test "writeSplatHeader splatting avoids buffer aliasing temptation" {
297 const initial_buf = try testing.allocator.alloc(u8, 8);347 const initial_buf = try testing.allocator.alloc(u8, 8);
298 var aw: Allocating = .initOwnedSlice(testing.allocator, initial_buf);348 var aw: Allocating = .initOwnedSlice(testing.allocator, initial_buf);