Skip to content

Conversation

@SquareMan
Copy link

Closes #25054

This handles packed structs as a base case before recursing into struct fields.

@SquareMan
Copy link
Author

I think I might need a little bit of help with the test failure. It looks like this fix has exposed another edge case with byteSwapAllFields: It can't deal with byte-swapping a struct that contains an inner struct with custom alignment. For example, trying to byteswap this struct will fail on master now:

    const A = extern struct {
        f0: u32,
        f1: extern struct {
            f0: u64,
        } align(4),
        f2: u32,
    };

std.zig.Server.Message.TestResults gets away with it right now because std.zig.Server.Message.TestResults.Flags is actually a packed struct, so it hits the packed struct special case when iterating fields and doesn't have to take a pointer to flags and recurse.

I'm not sure if there's a clean way to solve this. Making ptr anytype allows the alignment of the pointer to be captured and fix the error, but that's pretty hacky and I assume not acceptable.

@SquareMan
Copy link
Author

I've come up with one solution here of renaming byteSwapAllFields to byteSwapAllFieldsAligned and modified the signature to accept an alignment parameter. Then I converted byteSwapAllFields into a wrapper function that will call byteSwapAllFieldsAligned with standard aligment for 99% use case where the root struct you're trying to byteswap is itself aligned in memory.

Open to suggestions on how to improve this further, if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

std.mem.byteSwapAllFields not working with packed structs.

1 participant