From ed612f40af7d2ad6aa4f7760256927651f164191 Mon Sep 17 00:00:00 2001 From: "kj4tmp@gmail.com" Date: Fri, 4 Oct 2024 22:14:38 -0700 Subject: [PATCH 1/4] support packed structs in std.io.Reader and std.io.Writer --- lib/std/io/Reader.zig | 45 +++++++++++++++++++++++++++++-------- lib/std/io/Reader/test.zig | 42 ++++++++++++++++++++++++++++++++++ lib/std/io/Writer.zig | 46 +++++++++++++++++++++++++++++++------- lib/std/io/Writer/test.zig | 35 +++++++++++++++++++++++++++++ 4 files changed, 151 insertions(+), 17 deletions(-) create mode 100644 lib/std/io/Writer/test.zig diff --git a/lib/std/io/Reader.zig b/lib/std/io/Reader.zig index 33187125b8..7bf474b002 100644 --- a/lib/std/io/Reader.zig +++ b/lib/std/io/Reader.zig @@ -324,20 +324,47 @@ pub fn isBytes(self: Self, slice: []const u8) anyerror!bool { return matches; } +/// Read a struct from the stream. +/// Only packed and extern structs are supported. +/// Packed structs must have a `@bitSizeOf` that is a multiple of eight. pub fn readStruct(self: Self, comptime T: type) anyerror!T { - // Only extern and packed structs have defined in-memory layout. - comptime assert(@typeInfo(T).@"struct".layout != .auto); - var res: [1]T = undefined; - try self.readNoEof(mem.sliceAsBytes(res[0..])); - return res[0]; + switch (@typeInfo(T).@"struct".layout) { + .auto => @compileError("readStruct only supports packed and extern structs."), + .@"extern" => { + var res: [1]T = undefined; + try self.readNoEof(mem.sliceAsBytes(res[0..])); + return res[0]; + }, + .@"packed" => { + var bytes: [@divExact(@bitSizeOf(T), 8)]u8 = undefined; + try self.readNoEof(&bytes); + return @bitCast(bytes); + }, + } } +/// Read a struct having the specified endianness into the host endianness representation. +/// Only packed and extern structs are supported. +/// Packed structs must have a `@bitSizeOf` that is a multiple of eight. pub fn readStructEndian(self: Self, comptime T: type, endian: std.builtin.Endian) anyerror!T { - var res = try self.readStruct(T); - if (native_endian != endian) { - mem.byteSwapAllFields(T, &res); + switch (@typeInfo(T).@"struct".layout) { + .auto => @compileError("readStructEndian only supports packed and extern structs."), + .@"extern" => { + var res = try self.readStruct(T); + if (native_endian != endian) { + mem.byteSwapAllFields(T, &res); + } + return res; + }, + .@"packed" => { + var bytes: [@divExact(@bitSizeOf(T), 8)]u8 = undefined; + try self.readNoEof(&bytes); + if (native_endian != endian) { + mem.reverse(u8, &bytes); + } + return @bitCast(bytes); + }, } - return res; } /// Reads an integer with the same size as the given enum's tag type. If the integer matches diff --git a/lib/std/io/Reader/test.zig b/lib/std/io/Reader/test.zig index 30f0e1269c..2e43214b14 100644 --- a/lib/std/io/Reader/test.zig +++ b/lib/std/io/Reader/test.zig @@ -1,6 +1,7 @@ const builtin = @import("builtin"); const std = @import("../../std.zig"); const testing = std.testing; +const native_endian = @import("builtin").target.cpu.arch.endian(); test "Reader" { var buf = "a\x02".*; @@ -370,3 +371,44 @@ test "readIntoBoundedBytes correctly reads into a provided bounded array" { try reader.readIntoBoundedBytes(10000, &bounded_array); try testing.expectEqualStrings(bounded_array.slice(), test_string); } + +test "readStructEndian reads packed structs without padding and in correct field order" { + const buf = [3]u8{ 11, 12, 13 }; + var fis = std.io.fixedBufferStream(&buf); + const reader = fis.reader(); + + const PackedStruct = packed struct(u24) { a: u8, b: u8, c: u8 }; + + try testing.expectEqualDeep( + PackedStruct{ .a = 11, .b = 12, .c = 13 }, + reader.readStructEndian(PackedStruct, .little), + ); + fis.reset(); + try testing.expectEqualDeep( + PackedStruct{ .a = 13, .b = 12, .c = 11 }, + reader.readStructEndian(PackedStruct, .big), + ); +} + +test "readStruct reads packed structs without padding and in correct field order" { + const buf = [3]u8{ 11, 12, 13 }; + var fis = std.io.fixedBufferStream(&buf); + const reader = fis.reader(); + + const PackedStruct = packed struct(u24) { a: u8, b: u8, c: u8 }; + + switch (native_endian) { + .little => { + try testing.expectEqualDeep( + PackedStruct{ .a = 11, .b = 12, .c = 13 }, + reader.readStruct(PackedStruct), + ); + }, + .big => { + try testing.expectEqualDeep( + PackedStruct{ .a = 13, .b = 12, .c = 11 }, + reader.readStruct(PackedStruct), + ); + }, + } +} diff --git a/lib/std/io/Writer.zig b/lib/std/io/Writer.zig index 26d4f88def..ca8979ed74 100644 --- a/lib/std/io/Writer.zig +++ b/lib/std/io/Writer.zig @@ -54,20 +54,46 @@ pub inline fn writeInt(self: Self, comptime T: type, value: T, endian: std.built return self.writeAll(&bytes); } +/// Write a struct to the stream. +/// Only packed and extern structs are supported. +/// Packed structs must have a `@bitSizeOf` that is a multiple of eight. pub fn writeStruct(self: Self, value: anytype) anyerror!void { // Only extern and packed structs have defined in-memory layout. - comptime assert(@typeInfo(@TypeOf(value)).@"struct".layout != .auto); - return self.writeAll(mem.asBytes(&value)); + switch (@typeInfo(@TypeOf(value)).@"struct".layout) { + .auto => @compileError("writeStruct only supports packed and extern structs."), + .@"extern" => { + return try self.writeAll(mem.asBytes(&value)); + }, + .@"packed" => { + const bytes: [@divExact(@bitSizeOf(@TypeOf(value)), 8)]u8 = @bitCast(value); + try self.writeAll(&bytes); + }, + } } +/// Write a struct to the stream in the specified endianness. +/// Only packed and extern structs are supported. +/// Packed structs must have a `@bitSizeOf` that is a multiple of eight. pub fn writeStructEndian(self: Self, value: anytype, endian: std.builtin.Endian) anyerror!void { // TODO: make sure this value is not a reference type - if (native_endian == endian) { - return self.writeStruct(value); - } else { - var copy = value; - mem.byteSwapAllFields(@TypeOf(value), ©); - return self.writeStruct(copy); + switch (@typeInfo(@TypeOf(value)).@"struct".layout) { + .auto => @compileError("writeStructEndian only supports packed and extern structs."), + .@"extern" => { + if (native_endian == endian) { + return try self.writeStruct(value); + } else { + var copy = value; + mem.byteSwapAllFields(@TypeOf(value), ©); + return try self.writeStruct(copy); + } + }, + .@"packed" => { + var bytes: [@divExact(@bitSizeOf(@TypeOf(value)), 8)]u8 = @bitCast(value); + if (native_endian != endian) { + mem.reverse(u8, &bytes); + } + return try self.writeAll(&bytes); + }, } } @@ -81,3 +107,7 @@ pub fn writeFile(self: Self, file: std.fs.File) anyerror!void { if (n < buf.len) return; } } + +test { + _ = @import("Writer/test.zig"); +} diff --git a/lib/std/io/Writer/test.zig b/lib/std/io/Writer/test.zig new file mode 100644 index 0000000000..01ba451673 --- /dev/null +++ b/lib/std/io/Writer/test.zig @@ -0,0 +1,35 @@ +const std = @import("../../std.zig"); +const testing = std.testing; +const native_endian = @import("builtin").target.cpu.arch.endian(); + +test "writeStruct writes packed structs without padding" { + var buf: [3]u8 = undefined; + var fis = std.io.fixedBufferStream(&buf); + const writer = fis.writer(); + + const PackedStruct = packed struct(u24) { a: u8, b: u8, c: u8 }; + + try writer.writeStruct(PackedStruct{ .a = 11, .b = 12, .c = 13 }); + switch (native_endian) { + .little => { + try testing.expectEqualSlices(u8, &.{ 11, 12, 13 }, &buf); + }, + .big => { + try testing.expectEqualSlices(u8, &.{ 13, 12, 11 }, &buf); + }, + } +} + +test "writeStructEndian writes packed structs without padding and in correct field order" { + var buf: [3]u8 = undefined; + var fis = std.io.fixedBufferStream(&buf); + const writer = fis.writer(); + + const PackedStruct = packed struct(u24) { a: u8, b: u8, c: u8 }; + + try writer.writeStructEndian(PackedStruct{ .a = 11, .b = 12, .c = 13 }, .little); + try testing.expectEqualSlices(u8, &.{ 11, 12, 13 }, &buf); + fis.reset(); + try writer.writeStructEndian(PackedStruct{ .a = 11, .b = 12, .c = 13 }, .big); + try testing.expectEqualSlices(u8, &.{ 13, 12, 11 }, &buf); +} From a9dbd33061bcd26809240a1ae4fb7c2e9d72ac73 Mon Sep 17 00:00:00 2001 From: "kj4tmp@gmail.com" Date: Sat, 5 Oct 2024 16:20:31 -0700 Subject: [PATCH 2/4] retain old doc comment about in-memory layout, improve compile error message --- lib/std/io/Reader.zig | 10 ++++++---- lib/std/io/Writer.zig | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/std/io/Reader.zig b/lib/std/io/Reader.zig index 7bf474b002..b29e15ca59 100644 --- a/lib/std/io/Reader.zig +++ b/lib/std/io/Reader.zig @@ -325,11 +325,12 @@ pub fn isBytes(self: Self, slice: []const u8) anyerror!bool { } /// Read a struct from the stream. -/// Only packed and extern structs are supported. +/// Only packed and extern structs are supported, as they have a defined in-memory layout. /// Packed structs must have a `@bitSizeOf` that is a multiple of eight. pub fn readStruct(self: Self, comptime T: type) anyerror!T { switch (@typeInfo(T).@"struct".layout) { - .auto => @compileError("readStruct only supports packed and extern structs."), + .auto => @compileError("readStruct only supports packed and extern structs, " ++ + "but the given type: " ++ @typeName(T) ++ " is a normal struct."), .@"extern" => { var res: [1]T = undefined; try self.readNoEof(mem.sliceAsBytes(res[0..])); @@ -344,11 +345,12 @@ pub fn readStruct(self: Self, comptime T: type) anyerror!T { } /// Read a struct having the specified endianness into the host endianness representation. -/// Only packed and extern structs are supported. +/// Only packed and extern structs are supported, as they have a defined in-memory layout. /// Packed structs must have a `@bitSizeOf` that is a multiple of eight. pub fn readStructEndian(self: Self, comptime T: type, endian: std.builtin.Endian) anyerror!T { switch (@typeInfo(T).@"struct".layout) { - .auto => @compileError("readStructEndian only supports packed and extern structs."), + .auto => @compileError("readStructEndian only supports packed and extern structs, " ++ + "but the given type: " ++ @typeName(T) ++ " is a normal struct."), .@"extern" => { var res = try self.readStruct(T); if (native_endian != endian) { diff --git a/lib/std/io/Writer.zig b/lib/std/io/Writer.zig index ca8979ed74..d027ecec2f 100644 --- a/lib/std/io/Writer.zig +++ b/lib/std/io/Writer.zig @@ -55,12 +55,13 @@ pub inline fn writeInt(self: Self, comptime T: type, value: T, endian: std.built } /// Write a struct to the stream. -/// Only packed and extern structs are supported. +/// Only packed and extern structs are supported, as they have a defined in-memory layout. /// Packed structs must have a `@bitSizeOf` that is a multiple of eight. pub fn writeStruct(self: Self, value: anytype) anyerror!void { // Only extern and packed structs have defined in-memory layout. switch (@typeInfo(@TypeOf(value)).@"struct".layout) { - .auto => @compileError("writeStruct only supports packed and extern structs."), + .auto => @compileError("writeStruct only supports packed and extern structs, " ++ + "but the given type: " ++ @typeName(@TypeOf(value)) ++ " is a normal struct."), .@"extern" => { return try self.writeAll(mem.asBytes(&value)); }, @@ -72,12 +73,13 @@ pub fn writeStruct(self: Self, value: anytype) anyerror!void { } /// Write a struct to the stream in the specified endianness. -/// Only packed and extern structs are supported. +/// Only packed and extern structs are supported, as they have a defined in-memory layout. /// Packed structs must have a `@bitSizeOf` that is a multiple of eight. pub fn writeStructEndian(self: Self, value: anytype, endian: std.builtin.Endian) anyerror!void { // TODO: make sure this value is not a reference type switch (@typeInfo(@TypeOf(value)).@"struct".layout) { - .auto => @compileError("writeStructEndian only supports packed and extern structs."), + .auto => @compileError("writeStructEndian only supports packed and extern structs, " ++ + "but the given type: " ++ @typeName(@TypeOf(value)) ++ " is a normal struct."), .@"extern" => { if (native_endian == endian) { return try self.writeStruct(value); From 8a17490efca5649cbc4581b022ed81fcde9b4eec Mon Sep 17 00:00:00 2001 From: "kj4tmp@gmail.com" Date: Sat, 5 Oct 2024 17:20:31 -0700 Subject: [PATCH 3/4] add tests for endianness-affected types --- lib/std/io/Reader/test.zig | 86 ++++++++++++++++++++++++++++++++++++++ lib/std/io/Writer/test.zig | 28 +++++++++++++ 2 files changed, 114 insertions(+) diff --git a/lib/std/io/Reader/test.zig b/lib/std/io/Reader/test.zig index 2e43214b14..876eb1a509 100644 --- a/lib/std/io/Reader/test.zig +++ b/lib/std/io/Reader/test.zig @@ -412,3 +412,89 @@ test "readStruct reads packed structs without padding and in correct field order }, } } + +test "readStruct writeStruct round-trip with packed structs" { + var buf: [8]u8 = undefined; + var fis = std.io.fixedBufferStream(&buf); + const reader = fis.reader(); + const writer = fis.writer(); + + const PackedStruct = packed struct(u64) { + a: u16 = 123, + b: u16 = 245, + c: u16 = 456, + d: i13 = -345, + e: i3 = 2, + }; + + const expected_packed_struct = PackedStruct{}; + try writer.writeStruct(expected_packed_struct); + fis.reset(); + try testing.expectEqualDeep(expected_packed_struct, try reader.readStruct(PackedStruct)); +} + +test "readStructEndian writeStructEndian round-trip with packed structs" { + var buf: [8]u8 = undefined; + var fis = std.io.fixedBufferStream(&buf); + const reader = fis.reader(); + const writer = fis.writer(); + + const PackedStruct = packed struct(u64) { + a: u13 = 123, + b: i7 = -24, + c: u20 = 83, + d: enum(i24) { val = 3452 } = .val, + }; + + const expected_packed_struct = PackedStruct{}; + // round-trip little endian + try writer.writeStructEndian(expected_packed_struct, .big); + fis.reset(); + try testing.expectEqualDeep(expected_packed_struct, try reader.readStructEndian(PackedStruct, .big)); + // round-trip big endian + fis.reset(); + try writer.writeStructEndian(expected_packed_struct, .little); + fis.reset(); + try testing.expectEqualDeep(expected_packed_struct, try reader.readStructEndian(PackedStruct, .little)); +} + +test "readStruct a packed struct with endianness-affected types" { + const buf = [4]u8{ 0x12, 0x34, 0x56, 0x78 }; + var fis = std.io.fixedBufferStream(&buf); + const reader = fis.reader(); + + const PackedStruct = packed struct(u32) { a: u16, b: u16 }; + + switch (native_endian) { + .little => { + try testing.expectEqualDeep( + PackedStruct{ .a = 0x3412, .b = 0x7856 }, + reader.readStruct(PackedStruct), + ); + }, + .big => { + try testing.expectEqualDeep( + PackedStruct{ .a = 0x5678, .b = 0x1234 }, + reader.readStruct(PackedStruct), + ); + }, + } +} + +test "readStructEndian a packed struct with endianness-affected types" { + const buf = [4]u8{ 0x12, 0x34, 0x56, 0x78 }; + var fis = std.io.fixedBufferStream(&buf); + const reader = fis.reader(); + + const PackedStruct = packed struct(u32) { a: u16, b: u16 }; + + try testing.expectEqualDeep( + PackedStruct{ .a = 0x3412, .b = 0x7856 }, + reader.readStructEndian(PackedStruct, .little), + ); + fis.reset(); + try testing.expectEqualDeep( + PackedStruct{ .a = 0x5678, .b = 0x1234 }, + reader.readStructEndian(PackedStruct, .big), + ); +} diff --git a/lib/std/io/Writer/test.zig b/lib/std/io/Writer/test.zig index 01ba451673..2ca6013f95 100644 --- a/lib/std/io/Writer/test.zig +++ b/lib/std/io/Writer/test.zig @@ -33,3 +33,31 @@ test "writeStructEndian writes packed structs without padding and in correct fie try writer.writeStructEndian(PackedStruct{ .a = 11, .b = 12, .c = 13 }, .big); try testing.expectEqualSlices(u8, &.{ 13, 12, 11 }, &buf); } + +test "writeStruct a packed struct with endianness-affected types" { + var buf: [4]u8 = undefined; + var fis = std.io.fixedBufferStream(&buf); + const writer = fis.writer(); + + const PackedStruct = packed struct(u32) { a: u16, b: u16 }; + + try writer.writeStruct(PackedStruct{ .a = 0x1234, .b = 0x5678 }); + switch (native_endian) { + .little => try testing.expectEqualSlices(u8, &.{ 0x34, 0x12, 0x78, 0x56 }, &buf), + .big => try testing.expectEqualSlices(u8, &.{ 0x56, 0x78, 0x12, 0x34 }, &buf), + } +} + +test "writeStructEndian a packed struct with endianness-affected types" { + var buf: [4]u8 = undefined; + var fis = std.io.fixedBufferStream(&buf); + const writer = fis.writer(); + + const PackedStruct = packed struct(u32) { a: u16, b: u16 }; + + try writer.writeStructEndian(PackedStruct{ .a = 0x1234, .b = 0x5678 }, .little); + try testing.expectEqualSlices(u8, &.{ 0x34, 0x12, 0x78, 0x56 }, &buf); + fis.reset(); + try writer.writeStructEndian(PackedStruct{ .a = 0x1234, .b = 0x5678 }, .big); + try testing.expectEqualSlices(u8, &.{ 0x56, 0x78, 0x12, 0x34 }, &buf); +} From 98c4e6ea456beac642161f61f06f7b5fb99b152b Mon Sep 17 00:00:00 2001 From: "kj4tmp@gmail.com" Date: Sun, 13 Oct 2024 18:12:08 -0700 Subject: [PATCH 4/4] use expectEqual for packed struct equality in tests --- lib/std/io/Reader/test.zig | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/std/io/Reader/test.zig b/lib/std/io/Reader/test.zig index 876eb1a509..c6250cc10f 100644 --- a/lib/std/io/Reader/test.zig +++ b/lib/std/io/Reader/test.zig @@ -379,12 +379,12 @@ test "readStructEndian reads packed structs without padding and in correct field const PackedStruct = packed struct(u24) { a: u8, b: u8, c: u8 }; - try testing.expectEqualDeep( + try testing.expectEqual( PackedStruct{ .a = 11, .b = 12, .c = 13 }, reader.readStructEndian(PackedStruct, .little), ); fis.reset(); - try testing.expectEqualDeep( + try testing.expectEqual( PackedStruct{ .a = 13, .b = 12, .c = 11 }, reader.readStructEndian(PackedStruct, .big), ); @@ -399,13 +399,13 @@ test "readStruct reads packed structs without padding and in correct field order switch (native_endian) { .little => { - try testing.expectEqualDeep( + try testing.expectEqual( PackedStruct{ .a = 11, .b = 12, .c = 13 }, reader.readStruct(PackedStruct), ); }, .big => { - try testing.expectEqualDeep( + try testing.expectEqual( PackedStruct{ .a = 13, .b = 12, .c = 11 }, reader.readStruct(PackedStruct), ); @@ -430,7 +430,7 @@ test "readStruct writeStruct round-trip with packed structs" { const expected_packed_struct = PackedStruct{}; try writer.writeStruct(expected_packed_struct); fis.reset(); - try testing.expectEqualDeep(expected_packed_struct, try reader.readStruct(PackedStruct)); + try testing.expectEqual(expected_packed_struct, try reader.readStruct(PackedStruct)); } test "readStructEndian writeStructEndian round-trip with packed structs" { @@ -450,12 +450,12 @@ test "readStructEndian writeStructEndian round-trip with packed structs" { // round-trip little endian try writer.writeStructEndian(expected_packed_struct, .big); fis.reset(); - try testing.expectEqualDeep(expected_packed_struct, try reader.readStructEndian(PackedStruct, .big)); + try testing.expectEqual(expected_packed_struct, try reader.readStructEndian(PackedStruct, .big)); // round-trip big endian fis.reset(); try writer.writeStructEndian(expected_packed_struct, .little); fis.reset(); - try testing.expectEqualDeep(expected_packed_struct, try reader.readStructEndian(PackedStruct, .little)); + try testing.expectEqual(expected_packed_struct, try reader.readStructEndian(PackedStruct, .little)); } test "readStruct a packed struct with endianness-affected types" { @@ -467,13 +467,13 @@ test "readStruct a packed struct with endianness-affected types" { switch (native_endian) { .little => { - try testing.expectEqualDeep( + try testing.expectEqual( PackedStruct{ .a = 0x3412, .b = 0x7856 }, reader.readStruct(PackedStruct), ); }, .big => { - try testing.expectEqualDeep( + try testing.expectEqual( PackedStruct{ .a = 0x5678, .b = 0x1234 }, reader.readStruct(PackedStruct), ); @@ -488,12 +488,12 @@ test "readStructEndian a packed struct with endianness-affected types" { const PackedStruct = packed struct(u32) { a: u16, b: u16 }; - try testing.expectEqualDeep( + try testing.expectEqual( PackedStruct{ .a = 0x3412, .b = 0x7856 }, reader.readStructEndian(PackedStruct, .little), ); fis.reset(); - try testing.expectEqualDeep( + try testing.expectEqual( PackedStruct{ .a = 0x5678, .b = 0x1234 }, reader.readStructEndian(PackedStruct, .big), );