authorgravatar for mercenary@noreply.codeberg.orgmercenary <mercenary@noreply.codeberg.org> 2026-01-30 20:19:19+01:00
committergravatar for andrew@ziglang.orgAndrew Kelley <andrew@ziglang.org> 2026-01-30 20:19:19+01:00
logfa988e88ed21485830a70276b5c7567efb122f80
treeecd73646095246276a7ab7cbdfe6aa799c911195
parentaa38f07c5173f9722ebfb933058a2a032c2badf3

zstd.Decompress: smarter rebase when discarding (#30891)

The call to `rebase` in `discardIndirect` and `discardDirect` was inappropriate. As `rebase` expects the `capacity` parameter to exclude the sliding window, this call was asking for ANOTHER `d.window_len` bytes. This was impossible to fulfill with a buffer smaller than 2*`d.window_len`, and caused [#25764](https://github.com/ziglang/zig/issues/25764). This PR adds a basic test to do a discard (which does trigger [#25764](https://github.com/ziglang/zig/issues/25764)), and rebases only as much as is required to make the discard succeed ([or no rebase at all](https://github.com/ziglang/zig/issues/25764#issuecomment-3484716253)). That means: ideally rebase to fit `limit`, or if the buffer is too small, as much as possible. I must say, `discardDirect` does not make much sense to me, but I replaced it anyway. `rebaseForDiscard` works fine with `d.reader.buffer.len == 0`. Let me know if anything should be changed. Reviewed-on: https://codeberg.org/ziglang/zig/pulls/30891 Reviewed-by: Andrew Kelley <andrew@ziglang.org> Co-authored-by: mercenary <mercenary@noreply.codeberg.org> Co-committed-by: mercenary <mercenary@noreply.codeberg.org>

3 files changed, 43 insertions(+), 2 deletions(-)

lib/std/Io.zig+8
...@@ -728,6 +728,14 @@ pub const Limit = enum(usize) {...@@ -728,6 +728,14 @@ pub const Limit = enum(usize) {
728 return @enumFromInt(@min(@intFromEnum(a), @intFromEnum(b)));728 return @enumFromInt(@min(@intFromEnum(a), @intFromEnum(b)));
729 }729 }
730730
731 pub fn max(a: Limit, b: Limit) Limit {
732 if (a == .unlimited or b == .unlimited) {
733 return .unlimited;
734 }
735
736 return @enumFromInt(@max(@intFromEnum(a), @intFromEnum(b)));
737 }
738
731 pub fn minInt(l: Limit, n: usize) usize {739 pub fn minInt(l: Limit, n: usize) usize {
732 return @min(n, @intFromEnum(l));740 return @min(n, @intFromEnum(l));
733 }741 }
lib/std/compress/zstd.zig+13
...@@ -88,6 +88,17 @@ fn testDecompress(gpa: std.mem.Allocator, compressed: []const u8) ![]u8 {...@@ -88,6 +88,17 @@ fn testDecompress(gpa: std.mem.Allocator, compressed: []const u8) ![]u8 {
88 return out.toOwnedSlice();88 return out.toOwnedSlice();
89}89}
9090
91/// Create a `Decompress` from `compressed` and immediately discard all output. Returns the number
92/// of discarded bytes.
93fn testDiscard(gpa: std.mem.Allocator, compressed: []const u8) !usize {
94 const buf: []u8 = try gpa.alloc(u8, default_window_len + block_size_max);
95 defer gpa.free(buf);
96
97 var in: std.Io.Reader = .fixed(compressed);
98 var zstd_stream: Decompress = .init(&in, buf, .{});
99 return try zstd_stream.reader.discardRemaining();
100}
101
91fn testExpectDecompress(uncompressed: []const u8, compressed: []const u8) !void {102fn testExpectDecompress(uncompressed: []const u8, compressed: []const u8) !void {
92 const gpa = std.testing.allocator;103 const gpa = std.testing.allocator;
93 const result = try testDecompress(gpa, compressed);104 const result = try testDecompress(gpa, compressed);
...@@ -117,6 +128,8 @@ test Decompress {...@@ -117,6 +128,8 @@ test Decompress {
117128
118 try testExpectDecompress(uncompressed, compressed3);129 try testExpectDecompress(uncompressed, compressed3);
119 try testExpectDecompress(uncompressed, compressed19);130 try testExpectDecompress(uncompressed, compressed19);
131 try std.testing.expectEqual(uncompressed.len, testDiscard(std.testing.allocator, compressed3));
132 try std.testing.expectEqual(uncompressed.len, testDiscard(std.testing.allocator, compressed19));
120}133}
121134
122test "partial magic number" {135test "partial magic number" {
lib/std/compress/zstd/Decompress.zig+22-2
...@@ -123,9 +123,13 @@ fn rebaseFallible(r: *Reader, capacity: usize) Reader.RebaseError!void {...@@ -123,9 +123,13 @@ fn rebaseFallible(r: *Reader, capacity: usize) Reader.RebaseError!void {
123 rebase(r, capacity);123 rebase(r, capacity);
124}124}
125125
126// Rebase the buffer, keeping at least the sliding window (`d.window_len` bytes) buffered
126fn rebase(r: *Reader, capacity: usize) void {127fn rebase(r: *Reader, capacity: usize) void {
127 const d: *Decompress = @alignCast(@fieldParentPtr("reader", r));128 const d: *Decompress = @alignCast(@fieldParentPtr("reader", r));
129 // `capacity` must fit in the buffer along with the required sliding window
128 assert(capacity <= r.buffer.len - d.window_len);130 assert(capacity <= r.buffer.len - d.window_len);
131 // According to the vtable contract, this function will only be called if the free space in the
132 // buffer cannot already fit `capacity` bytes
129 assert(r.end + capacity > r.buffer.len);133 assert(r.end + capacity > r.buffer.len);
130 const discard_n = @min(r.seek, r.end - d.window_len);134 const discard_n = @min(r.seek, r.end - d.window_len);
131 const keep = r.buffer[discard_n..r.end];135 const keep = r.buffer[discard_n..r.end];
...@@ -134,11 +138,27 @@ fn rebase(r: *Reader, capacity: usize) void {...@@ -134,11 +138,27 @@ fn rebase(r: *Reader, capacity: usize) void {
134 r.seek -= discard_n;138 r.seek -= discard_n;
135}139}
136140
141/// Rebase `d.reader.buffer` as much as needed for a discard limited by `limit`
142fn rebaseForDiscard(d: *Decompress, limit: std.Io.Limit) void {
143 // Number of bytes desired to rebase, always rebase for at least block_size
144 const desire_n = limit.max(Limit.limited(zstd.block_size_max));
145 // Maximum number of bytes possible to rebase
146 const max_n = d.reader.buffer.len -| d.window_len;
147 // Number of bytes to rebase
148 const n = desire_n.minInt(max_n);
149
150 // Current buffer free space
151 const current_cap = d.reader.buffer.len - d.reader.end;
152 if (current_cap < n) {
153 rebase(&d.reader, n);
154 }
155}
156
137/// This could be improved so that when an amount is discarded that includes an157/// This could be improved so that when an amount is discarded that includes an
138/// entire frame, skip decoding that frame.158/// entire frame, skip decoding that frame.
139fn discardDirect(r: *Reader, limit: std.Io.Limit) Reader.Error!usize {159fn discardDirect(r: *Reader, limit: std.Io.Limit) Reader.Error!usize {
140 const d: *Decompress = @alignCast(@fieldParentPtr("reader", r));160 const d: *Decompress = @alignCast(@fieldParentPtr("reader", r));
141 rebase(r, d.window_len);161 rebaseForDiscard(d, limit);
142 var writer: Writer = .{162 var writer: Writer = .{
143 .vtable = &.{163 .vtable = &.{
144 .drain = std.Io.Writer.Discarding.drain,164 .drain = std.Io.Writer.Discarding.drain,
...@@ -162,7 +182,7 @@ fn discardDirect(r: *Reader, limit: std.Io.Limit) Reader.Error!usize {...@@ -162,7 +182,7 @@ fn discardDirect(r: *Reader, limit: std.Io.Limit) Reader.Error!usize {
162182
163fn discardIndirect(r: *Reader, limit: std.Io.Limit) Reader.Error!usize {183fn discardIndirect(r: *Reader, limit: std.Io.Limit) Reader.Error!usize {
164 const d: *Decompress = @alignCast(@fieldParentPtr("reader", r));184 const d: *Decompress = @alignCast(@fieldParentPtr("reader", r));
165 rebase(r, d.window_len);185 rebaseForDiscard(d, limit);
166 var writer: Writer = .{186 var writer: Writer = .{
167 .buffer = r.buffer,187 .buffer = r.buffer,
168 .end = r.end,188 .end = r.end,