Skip to content

Conversation

@danielchasehooper
Copy link
Contributor

@danielchasehooper danielchasehooper commented Nov 11, 2024

This change fixes two issues:

  1. The Zig definition of std.c.EXC.MASK had the mask flags starting at bit 0, but the header it's copying has this:
#define EXC_BAD_ACCESS          1

...

/*
 * Masks for exception definitions, above
 * bit zero is unused, therefore 1 word = 31 exception types
 */

#define EXC_MASK_BAD_ACCESS             (1 << EXC_BAD_ACCESS)
  1. the structure wasn't padded to 32 bits

Obviously the mask isn't used anywhere is zig, because it would've caused a packed structure padding compile error. On the chance that it is used in the future, either by the compiler, or as an api provided by std, I figured I'd save future poor souls from mysterious bugs caused by the mask flags bits being off by one bit.

Copy link
Member

@alexrp alexrp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some naming nitpicks; for unused/padding fields in packed structs that mirror C enums, we prefer to use the bit position as the name.

Otherwise LGTM.

Co-authored-by: Alex Rønne Petersen <[email protected]>
@alexrp alexrp enabled auto-merge (squash) November 12, 2024 21:31
@alexrp alexrp merged commit 9996f8b into ziglang:master Nov 13, 2024
10 checks passed
@danielchasehooper danielchasehooper deleted the mach_exc_mask branch November 13, 2024 14:24
alexrp added a commit that referenced this pull request Jan 27, 2025
Turns out this was already fixed in #21964.

I have no idea why GitHub showed an incorrect diff in #21273, or how applying the diff to master was even possible, but here we are.
andrewrk pushed a commit that referenced this pull request Jan 27, 2025
Turns out this was already fixed in #21964.

I have no idea why GitHub showed an incorrect diff in #21273, or how applying the diff to master was even possible, but here we are.
Fri3dNstuff pushed a commit to Fri3dNstuff/zig that referenced this pull request Jan 27, 2025
Turns out this was already fixed in ziglang#21964.

I have no idea why GitHub showed an incorrect diff in ziglang#21273, or how applying the diff to master was even possible, but here we are.
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.

2 participants