Skip to content

std.posix: remove unchecked std.os.linux usage #20303

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ifreund merged 1 commit into from
Jun 18, 2024
Merged

std.posix: remove unchecked std.os.linux usage #20303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions lib/std/c/linux.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub const Sigaction = linux.Sigaction;
pub const T = linux.T;
pub const TCP = linux.TCP;
pub const TCSA = linux.TCSA;
pub const TFD = linux.TFD;
pub const VDSO = linux.VDSO;
pub const W = linux.W;
pub const W_OK = linux.W_OK;
Expand All @@ -68,13 +69,15 @@ pub const fd_t = linux.fd_t;
pub const gid_t = linux.gid_t;
pub const ifreq = linux.ifreq;
pub const ino_t = linux.ino_t;
pub const itimerspec = linux.itimerspec;
pub const mcontext_t = linux.mcontext_t;
pub const mode_t = linux.mode_t;
pub const msghdr = linux.msghdr;
pub const msghdr_const = linux.msghdr_const;
pub const nfds_t = linux.nfds_t;
pub const nlink_t = linux.nlink_t;
pub const off_t = linux.off_t;
pub const perf_event_attr = linux.perf_event_attr;
pub const pid_t = linux.pid_t;
pub const pollfd = linux.pollfd;
pub const rlim_t = linux.rlim_t;
Expand Down Expand Up @@ -344,3 +347,12 @@ pub const dirent64 = struct {
type: u8,
name: [256]u8,
};

pub extern "c" fn timerfd_create(clockid: c_int, flags: c_int) c_int;
pub extern "c" fn timerfd_settime(
fd: c_int,
flags: c_int,
new_value: *const itimerspec,
old_value: ?*itimerspec,
) c_int;
pub extern "c" fn timerfd_gettime(fd: c_int, curr_value: *itimerspec) c_int;
67 changes: 35 additions & 32 deletions lib/std/posix.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3968,7 +3968,7 @@ pub const EpollCtlError = error{
FileDescriptorIncompatibleWithEpoll,
} || UnexpectedError;

pub fn epoll_ctl(epfd: i32, op: u32, fd: i32, event: ?*linux.epoll_event) EpollCtlError!void {
pub fn epoll_ctl(epfd: i32, op: u32, fd: i32, event: ?*system.epoll_event) EpollCtlError!void {
const rc = system.epoll_ctl(epfd, op, fd, event);
switch (errno(rc)) {
.SUCCESS => return,
Expand All @@ -3988,7 +3988,7 @@ pub fn epoll_ctl(epfd: i32, op: u32, fd: i32, event: ?*linux.epoll_event) EpollC
/// Waits for an I/O event on an epoll file descriptor.
/// Returns the number of file descriptors ready for the requested I/O,
/// or zero if no file descriptor became ready during the requested timeout milliseconds.
pub fn epoll_wait(epfd: i32, events: []linux.epoll_event, timeout: i32) usize {
pub fn epoll_wait(epfd: i32, events: []system.epoll_event, timeout: i32) usize {
while (true) {
// TODO get rid of the @intCast
const rc = system.epoll_wait(epfd, events.ptr, @intCast(events.len), timeout);
Expand Down Expand Up @@ -7142,32 +7142,35 @@ pub const PerfEventOpenError = error{
} || UnexpectedError;

pub fn perf_event_open(
attr: *linux.perf_event_attr,
attr: *system.perf_event_attr,
pid: pid_t,
cpu: i32,
group_fd: fd_t,
flags: usize,
) PerfEventOpenError!fd_t {
const rc = linux.perf_event_open(attr, pid, cpu, group_fd, flags);
switch (errno(rc)) {
.SUCCESS => return @intCast(rc),
.@"2BIG" => return error.TooBig,
.ACCES => return error.PermissionDenied,
.BADF => unreachable, // group_fd file descriptor is not valid.
.BUSY => return error.DeviceBusy,
.FAULT => unreachable, // Segmentation fault.
.INVAL => unreachable, // Bad attr settings.
.INTR => unreachable, // Mixed perf and ftrace handling for a uprobe.
.MFILE => return error.ProcessResources,
.NODEV => return error.EventRequiresUnsupportedCpuFeature,
.NOENT => unreachable, // Invalid type setting.
.NOSPC => return error.TooManyBreakpoints,
.NOSYS => return error.SampleStackNotSupported,
.OPNOTSUPP => return error.EventNotSupported,
.OVERFLOW => return error.SampleMaxStackOverflow,
.PERM => return error.PermissionDenied,
.SRCH => return error.ProcessNotFound,
else => |err| return unexpectedErrno(err),
if (native_os == .linux) {
Copy link
Member

@andrewrk andrewrk Jun 18, 2024

Choose a reason for hiding this comment

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

I'm fine with how you have it, but I'd probably write it like this:

Suggested change
if (native_os == .linux) {
if (native_os != .linux) @compileError("unsupported target OS");

Copy link
Member Author

@ifreund ifreund Jun 18, 2024

Choose a reason for hiding this comment

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

My current code is a bit more resistant to regressions in my opinion, though it does seem highly unlikely that some other OS would add a linux-compatible perf interface :D

Either way, I don't think changing this style is worth the energy cost of another CI run, I'll press the merge button shortly. Thanks for the review!

// There is no syscall wrapper for this function exposed by libcs
const rc = linux.perf_event_open(attr, pid, cpu, group_fd, flags);
switch (errno(rc)) {
.SUCCESS => return @intCast(rc),
.@"2BIG" => return error.TooBig,
.ACCES => return error.PermissionDenied,
.BADF => unreachable, // group_fd file descriptor is not valid.
.BUSY => return error.DeviceBusy,
.FAULT => unreachable, // Segmentation fault.
.INVAL => unreachable, // Bad attr settings.
.INTR => unreachable, // Mixed perf and ftrace handling for a uprobe.
.MFILE => return error.ProcessResources,
.NODEV => return error.EventRequiresUnsupportedCpuFeature,
.NOENT => unreachable, // Invalid type setting.
.NOSPC => return error.TooManyBreakpoints,
.NOSYS => return error.SampleStackNotSupported,
.OPNOTSUPP => return error.EventNotSupported,
.OVERFLOW => return error.SampleMaxStackOverflow,
.PERM => return error.PermissionDenied,
.SRCH => return error.ProcessNotFound,
else => |err| return unexpectedErrno(err),
}
}
}

Expand All @@ -7182,8 +7185,8 @@ pub const TimerFdCreateError = error{
pub const TimerFdGetError = error{InvalidHandle} || UnexpectedError;
pub const TimerFdSetError = TimerFdGetError || error{Canceled};

pub fn timerfd_create(clokid: i32, flags: linux.TFD) TimerFdCreateError!fd_t {
const rc = linux.timerfd_create(clokid, flags);
pub fn timerfd_create(clokid: i32, flags: system.TFD) TimerFdCreateError!fd_t {
const rc = system.timerfd_create(clokid, @bitCast(flags));
return switch (errno(rc)) {
.SUCCESS => @intCast(rc),
.INVAL => unreachable,
Expand All @@ -7198,11 +7201,11 @@ pub fn timerfd_create(clokid: i32, flags: linux.TFD) TimerFdCreateError!fd_t {

pub fn timerfd_settime(
fd: i32,
flags: linux.TFD.TIMER,
new_value: *const linux.itimerspec,
old_value: ?*linux.itimerspec,
flags: system.TFD.TIMER,
new_value: *const system.itimerspec,
old_value: ?*system.itimerspec,
) TimerFdSetError!void {
const rc = linux.timerfd_settime(fd, flags, new_value, old_value);
const rc = system.timerfd_settime(fd, @bitCast(flags), new_value, old_value);
return switch (errno(rc)) {
.SUCCESS => {},
.BADF => error.InvalidHandle,
Expand All @@ -7213,9 +7216,9 @@ pub fn timerfd_settime(
};
}

pub fn timerfd_gettime(fd: i32) TimerFdGetError!linux.itimerspec {
var curr_value: linux.itimerspec = undefined;
const rc = linux.timerfd_gettime(fd, &curr_value);
pub fn timerfd_gettime(fd: i32) TimerFdGetError!system.itimerspec {
var curr_value: system.itimerspec = undefined;
const rc = system.timerfd_gettime(fd, &curr_value);
return switch (errno(rc)) {
.SUCCESS => return curr_value,
.BADF => error.InvalidHandle,
Expand Down