std/c: Restore missing Mach declarations #21218
Conversation
Following ziglang@8c4a2dc, some Mach declarations where removed from std/c.zig. This commit restores them.
steeve
force-pushed
the
steeve/machfixes
branch
from
2c9fb3e to
ea4ac56
Compare
| pub fn getKernError(err: std.c.kern_return_t) KernE { | ||
| return @as(KernE, @enumFromInt(@as(u32, @truncate(@as(usize, @intCast(err)))))); | ||
| } | ||
|
|
||
| pub fn unexpectedKernError(err: KernE) std.posix.UnexpectedError { | ||
| if (std.posix.unexpected_error_tracing) { | ||
| std.debug.print("unexpected error: {d}\n", .{@intFromEnum(err)}); | ||
| std.debug.dumpCurrentStackTrace(null); | ||
| } | ||
| return error.Unexpected; | ||
| } |
There was a problem hiding this comment.
These functions do not belong in the C namespace.
There was a problem hiding this comment.
Where would you see them? They used to live in std/c.zig so that's where code used to import them from (rather through std.posix.system).
Eg: https://github.com/mitchellh/libxev/blob/dbe22910a43e9e8ec9948d3cbd73d8488a074967/src/backend/kqueue.zig#L2460
Perhaps in KERN ?
There was a problem hiding this comment.
Yes, I'm sorry. The original contributor put these in the wrong place. I think the best place for such abstractions is in std.os.darwin. I can help get this PR landed; I realize it's a bit of a messy situation.
| } | ||
|
|
||
| /// Kernel return values | ||
| pub const KernE = enum(u32) { |
There was a problem hiding this comment.
according to kern_return.h this should be
| pub const KernE = enum(u32) { | |
| pub const KERN = enum(u32) { |
There was a problem hiding this comment.
will do. how do you deal with prefix collision between different types? such as MachMsgE (a bit further) for instance? https://github.com/ziglang/zig/pull/21218/files#diff-f69178bf0db31205e046265d33d0eff15e5ddfdce2a7a18dc39309db4da64a5eR9674
There was a problem hiding this comment.
MachMsgE is a made up name that doesn't seem to be based on any C bits, so that's a problem. If it's so messy that nonexhaustive enums and packed structs cannot be used, then the best move would be to give up and use a single MACH empty struct namespace for all of them. It is not obvious to me based on a quick glance how these bits relate to each other.
| pub const SEND_SYNC_BOOTSTRAP_CHECKIN = 0x00800000; | ||
|
|
||
| pub const RCV_TIMEOUT = 0x00000100; | ||
| pub const RCV_NOTIFY = 0x00000000; |
There was a problem hiding this comment.
don't include the legacy and deprecated values please
#define MACH_RCV_NOTIFY 0x00000000 /* legacy name (value was: 0x00000200) */#define MACH_RCV_OVERWRITE 0x00000000 /* scatter receive (deprecated) */| pub const MACH_RCV_SYNC_PEEK = 0x00008000; | ||
|
|
||
| pub const MACH_MSG_STRICT_REPLY = 0x00000200; | ||
| pub const MACH_MSG_OPTION = struct { |
There was a problem hiding this comment.
if these definitions really all go together, could you please put them into a packed struct? the name is unfortunately not obvious based on Apple's extremely sloppy C code, so maybe the name can be determined by looking at the function(s) that these flags are passed to. Which is what? Might be nice to leave a little comment trail for the next person here.
There was a problem hiding this comment.
The reason I didn't go for a packed struct are these two definitions, which have the same value but different semantic meanings:
#define MACH_SEND_ALWAYS 0x00010000 /* ignore qlimits - kernel only */
#define MACH_SEND_FILTER_NONFATAL 0x00010000 /* rejection by message filter should return failure - user only */OTOH MACH_SEND_ALWAYS is "kernel only" so it might be ok to remove it.
There was a problem hiding this comment.
In this case, the function is mach_msg. Should I change the signature to accept the packed struct ? It currently accepts a mach_msg_option_t at the moment (on which the packed struct is based)
Following 8c4a2dc, some Mach declarations where removed from
std/c.zig. This commit restores them. After which I can compile and use libxev properly on macOS.I did not use an
enumorpacked structforMACH_MSG_OPTIONbecause semantically different options may have the same value, so I stuck to astruct.Closes #21200
Refs mitchellh/libxev#114