From 4a769acd1b1551dabce48a606d4d31d42e898865 Mon Sep 17 00:00:00 2001 From: Chris Boesch <48591413+chrboesch@users.noreply.github.com> Date: Mon, 27 Jan 2025 16:52:52 +0100 Subject: [PATCH 1/5] Rewriting enums to seperate clock IDs for clock_gettime() and timerfd_create() - Follow-up --- lib/std/Thread/Futex.zig | 2 +- lib/std/c.zig | 19 ++++++++++++++++++- lib/std/c/freebsd.zig | 1 + lib/std/os/linux.zig | 30 +++++++++++++++++++++++++++--- lib/std/os/linux/test.zig | 2 +- lib/std/posix.zig | 38 ++++++++++++++++++++------------------ lib/std/time.zig | 6 ++---- 7 files changed, 70 insertions(+), 28 deletions(-) diff --git a/lib/std/Thread/Futex.zig b/lib/std/Thread/Futex.zig index fc02b8407cf1..c18caec7a6c5 100644 --- a/lib/std/Thread/Futex.zig +++ b/lib/std/Thread/Futex.zig @@ -543,7 +543,7 @@ const PosixImpl = struct { // This can be changed with pthread_condattr_setclock, but it's an extension and may not be available everywhere. var ts: c.timespec = undefined; if (timeout) |timeout_ns| { - std.posix.clock_gettime(c.CLOCK.REALTIME, &ts) catch unreachable; + ts = std.posix.clock_gettime(c.CLOCK.REALTIME) catch unreachable; ts.sec +|= @as(@TypeOf(ts.sec), @intCast(timeout_ns / std.time.ns_per_s)); ts.nsec += @as(@TypeOf(ts.nsec), @intCast(timeout_ns % std.time.ns_per_s)); diff --git a/lib/std/c.zig b/lib/std/c.zig index e00196f453c1..b0e44b3deaca 100644 --- a/lib/std/c.zig +++ b/lib/std/c.zig @@ -213,6 +213,23 @@ pub const ARCH = switch (native_os) { .linux => linux.ARCH, else => void, }; + +// For use with posix.timerfd_create() +// Actually, the parameter for the timerfd_create() function is an integer, +// which means that the developer has to figure out which value is appropriate. +// To make this easier and, above all, safer, because an incorrect value leads +// to a panic, an enum is introduced which only allows the values +// that actually work. +pub const TIMERFD_CLOCK = timerfd_clockid_t; +pub const timerfd_clockid_t = switch (native_os) { + .linux, .freebsd => enum(u32) { + REALTIME = 0, + MONOTONIC = 1, + _, + }, + else => clockid_t, +}; + pub const CLOCK = clockid_t; pub const clockid_t = switch (native_os) { .linux, .emscripten => linux.clockid_t, @@ -9077,7 +9094,7 @@ pub extern "c" fn epoll_pwait( sigmask: *const sigset_t, ) c_int; -pub extern "c" fn timerfd_create(clockid: clockid_t, flags: c_int) c_int; +pub extern "c" fn timerfd_create(clockid: timerfd_clockid_t, flags: c_int) c_int; pub extern "c" fn timerfd_settime( fd: c_int, flags: c_int, diff --git a/lib/std/c/freebsd.zig b/lib/std/c/freebsd.zig index 6fd33c80b716..f6b39e9b7a15 100644 --- a/lib/std/c/freebsd.zig +++ b/lib/std/c/freebsd.zig @@ -20,6 +20,7 @@ const timespec = std.c.timespec; const uid_t = std.c.uid_t; const sf_hdtr = std.c.sf_hdtr; const clockid_t = std.c.clockid_t; +const clock_id = std.c.timerfd_clockid_t; comptime { assert(builtin.os.tag == .freebsd); // Prevent access of std.c symbols on wrong OS. diff --git a/lib/std/os/linux.zig b/lib/std/os/linux.zig index c030bd5208f0..bc38af92b57c 100644 --- a/lib/std/os/linux.zig +++ b/lib/std/os/linux.zig @@ -2204,7 +2204,7 @@ pub fn eventfd(count: u32, flags: u32) usize { return syscall2(.eventfd2, count, flags); } -pub fn timerfd_create(clockid: clockid_t, flags: TFD) usize { +pub fn timerfd_create(clockid: clock_id, flags: TFD) usize { return syscall2( .timerfd_create, @intFromEnum(clockid), @@ -4685,8 +4685,32 @@ pub const clockid_t = enum(u32) { BOOTTIME = 7, REALTIME_ALARM = 8, BOOTTIME_ALARM = 9, - SGI_CYCLE = 10, - TAI = 11, + // In the linux kernel header file (time.h) is the following note: + // * The driver implementing this got removed. The clock ID is kept as a + // * place holder. Do not reuse! + // Therefore, calling clock_gettime() with these IDs will result in an error. + // + // Some backgrond: + // - SGI_CYCLE was for Silicon Graphics (SGI) workstations, + // which are probably no longer in use, so it makes sense to disable + // - TAI_CLOCK was designed as CLOCK_REALTIME(UTC) + tai_offset, + // but tai_offset was always 0 in the kernel. + // So there is no point in using this clock. + // SGI_CYCLE = 10, + // TAI = 11, + _, +}; + +// For use with posix.timerfd_create() +// Actually, the parameter for the timerfd_create() function is in integer, +// which means that the developer has to figure out which value is appropriate. +// To make this easier and, above all, safer, because an incorrect value leads +// to a panic, an enum is introduced which only allows the values +// that actually work. +pub const CLOCK_ID = clock_id; +pub const clock_id = enum(u32) { + REALTIME = 0, + MONOTONIC = 1, _, }; diff --git a/lib/std/os/linux/test.zig b/lib/std/os/linux/test.zig index 0bc982140bc8..00653a153c9e 100644 --- a/lib/std/os/linux/test.zig +++ b/lib/std/os/linux/test.zig @@ -41,7 +41,7 @@ test "timer" { var err: linux.E = linux.E.init(epoll_fd); try expect(err == .SUCCESS); - const timer_fd = linux.timerfd_create(linux.CLOCK.MONOTONIC, .{}); + const timer_fd = linux.timerfd_create(linux.CLOCK_ID.MONOTONIC, .{}); try expect(linux.E.init(timer_fd) == .SUCCESS); const time_interval = linux.timespec{ diff --git a/lib/std/posix.zig b/lib/std/posix.zig index 94d63cf9ef85..5e474790544d 100644 --- a/lib/std/posix.zig +++ b/lib/std/posix.zig @@ -121,6 +121,7 @@ pub const blkcnt_t = system.blkcnt_t; pub const blksize_t = system.blksize_t; pub const clock_t = system.clock_t; pub const clockid_t = system.clockid_t; +pub const timerfd_clockid_t = system.timerfd_clockid_t; pub const cpu_set_t = system.cpu_set_t; pub const dev_t = system.dev_t; pub const dl_phdr_info = system.dl_phdr_info; @@ -155,6 +156,7 @@ pub const socklen_t = system.socklen_t; pub const stack_t = system.stack_t; pub const time_t = system.time_t; pub const timespec = system.timespec; +pub const timestamp_t = system.timestamp_t; pub const timeval = system.timeval; pub const timezone = system.timezone; pub const ucontext_t = system.ucontext_t; @@ -5624,13 +5626,13 @@ pub fn dl_iterate_phdr( pub const ClockGetTimeError = error{UnsupportedClock} || UnexpectedError; -/// TODO: change this to return the timespec as a return value -pub fn clock_gettime(clock_id: clockid_t, tp: *timespec) ClockGetTimeError!void { +pub fn clock_gettime(clk_id: clockid_t) ClockGetTimeError!timespec { + var tp: timespec = undefined; if (native_os == .wasi and !builtin.link_libc) { - var ts: wasi.timestamp_t = undefined; - switch (system.clock_time_get(clock_id, 1, &ts)) { + var ts: timestamp_t = undefined; + switch (system.clock_time_get(clk_id, 1, &ts)) { .SUCCESS => { - tp.* = .{ + tp = .{ .sec = @intCast(ts / std.time.ns_per_s), .nsec = @intCast(ts % std.time.ns_per_s), }; @@ -5638,38 +5640,38 @@ pub fn clock_gettime(clock_id: clockid_t, tp: *timespec) ClockGetTimeError!void .INVAL => return error.UnsupportedClock, else => |err| return unexpectedErrno(err), } - return; + return tp; } if (native_os == .windows) { - if (clock_id == .REALTIME) { + if (clk_id == .REALTIME) { var ft: windows.FILETIME = undefined; windows.kernel32.GetSystemTimeAsFileTime(&ft); // FileTime has a granularity of 100 nanoseconds and uses the NTFS/Windows epoch. const ft64 = (@as(u64, ft.dwHighDateTime) << 32) | ft.dwLowDateTime; const ft_per_s = std.time.ns_per_s / 100; - tp.* = .{ + tp = .{ .sec = @as(i64, @intCast(ft64 / ft_per_s)) + std.time.epoch.windows, .nsec = @as(c_long, @intCast(ft64 % ft_per_s)) * 100, }; - return; + return tp; } else { // TODO POSIX implementation of CLOCK.MONOTONIC on Windows. return error.UnsupportedClock; } } - switch (errno(system.clock_gettime(clock_id, tp))) { - .SUCCESS => return, + switch (errno(system.clock_gettime(clk_id, &tp))) { + .SUCCESS => return tp, .FAULT => unreachable, .INVAL => return error.UnsupportedClock, else => |err| return unexpectedErrno(err), } } -pub fn clock_getres(clock_id: clockid_t, res: *timespec) ClockGetTimeError!void { +pub fn clock_getres(clk_id: clockid_t, res: *timespec) ClockGetTimeError!void { if (native_os == .wasi and !builtin.link_libc) { - var ts: wasi.timestamp_t = undefined; - switch (system.clock_res_get(@bitCast(clock_id), &ts)) { + var ts: timestamp_t = undefined; + switch (system.clock_res_get(@bitCast(clk_id), &ts)) { .SUCCESS => res.* = .{ .sec = @intCast(ts / std.time.ns_per_s), .nsec = @intCast(ts % std.time.ns_per_s), @@ -5680,7 +5682,7 @@ pub fn clock_getres(clock_id: clockid_t, res: *timespec) ClockGetTimeError!void return; } - switch (errno(system.clock_getres(clock_id, res))) { + switch (errno(system.clock_getres(clk_id, res))) { .SUCCESS => return, .FAULT => unreachable, .INVAL => return error.UnsupportedClock, @@ -5871,7 +5873,7 @@ pub fn res_mkquery( q[i + 3] = class; // Make a reasonably unpredictable id - var ts: timespec = undefined; + const ts = clock_gettime(.REALTIME) catch undefined; clock_gettime(.REALTIME, &ts) catch {}; const UInt = std.meta.Int(.unsigned, @bitSizeOf(@TypeOf(ts.nsec))); const unsec: UInt = @bitCast(ts.nsec); @@ -7254,8 +7256,8 @@ pub const TimerFdCreateError = error{ pub const TimerFdGetError = error{InvalidHandle} || UnexpectedError; pub const TimerFdSetError = TimerFdGetError || error{Canceled}; -pub fn timerfd_create(clock_id: clockid_t, flags: system.TFD) TimerFdCreateError!fd_t { - const rc = system.timerfd_create(clock_id, @bitCast(flags)); +pub fn timerfd_create(clockid: system.timerfd_clockid_t, flags: system.TFD) TimerFdCreateError!fd_t { + const rc = system.timerfd_create(clockid, @bitCast(flags)); return switch (errno(rc)) { .SUCCESS => @intCast(rc), .INVAL => unreachable, diff --git a/lib/std/time.zig b/lib/std/time.zig index d253b6351270..3dbe2e837ba1 100644 --- a/lib/std/time.zig +++ b/lib/std/time.zig @@ -68,8 +68,7 @@ pub fn nanoTimestamp() i128 { return value.toEpoch(); }, else => { - var ts: posix.timespec = undefined; - posix.clock_gettime(.REALTIME, &ts) catch |err| switch (err) { + const ts = posix.clock_gettime(.REALTIME) catch |err| switch (err) { error.UnsupportedClock, error.Unexpected => return 0, // "Precision of timing depends on hardware and OS". }; return (@as(i128, ts.sec) * ns_per_s) + ts.nsec; @@ -171,8 +170,7 @@ pub const Instant = struct { else => posix.CLOCK.MONOTONIC, }; - var ts: posix.timespec = undefined; - posix.clock_gettime(clock_id, &ts) catch return error.Unsupported; + const ts = posix.clock_gettime(clock_id) catch return error.Unsupported; return .{ .timestamp = ts }; } From c685733b6e7117bef364b327f7c0376467de4a7c Mon Sep 17 00:00:00 2001 From: Chris Boesch <48591413+chrboesch@users.noreply.github.com> Date: Mon, 27 Jan 2025 17:34:06 +0100 Subject: [PATCH 2/5] Fixed changes from clockid_t to timerfd_clockid_t --- lib/std/c/freebsd.zig | 2 +- lib/std/os/linux.zig | 6 +++--- lib/std/os/linux/test.zig | 2 +- lib/std/posix.zig | 13 ++++++------- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/lib/std/c/freebsd.zig b/lib/std/c/freebsd.zig index f6b39e9b7a15..f4ea95a55b6a 100644 --- a/lib/std/c/freebsd.zig +++ b/lib/std/c/freebsd.zig @@ -20,7 +20,7 @@ const timespec = std.c.timespec; const uid_t = std.c.uid_t; const sf_hdtr = std.c.sf_hdtr; const clockid_t = std.c.clockid_t; -const clock_id = std.c.timerfd_clockid_t; +const timerfd_clockid_t = std.c.timerfd_clockid_t; comptime { assert(builtin.os.tag == .freebsd); // Prevent access of std.c symbols on wrong OS. diff --git a/lib/std/os/linux.zig b/lib/std/os/linux.zig index bc38af92b57c..d42c55983820 100644 --- a/lib/std/os/linux.zig +++ b/lib/std/os/linux.zig @@ -2204,7 +2204,7 @@ pub fn eventfd(count: u32, flags: u32) usize { return syscall2(.eventfd2, count, flags); } -pub fn timerfd_create(clockid: clock_id, flags: TFD) usize { +pub fn timerfd_create(clockid: timerfd_clockid_t, flags: TFD) usize { return syscall2( .timerfd_create, @intFromEnum(clockid), @@ -4707,8 +4707,8 @@ pub const clockid_t = enum(u32) { // To make this easier and, above all, safer, because an incorrect value leads // to a panic, an enum is introduced which only allows the values // that actually work. -pub const CLOCK_ID = clock_id; -pub const clock_id = enum(u32) { +pub const TIMERFD_CLOCK = timerfd_clockid_t; +pub const timerfd_clockid_t = enum(u32) { REALTIME = 0, MONOTONIC = 1, _, diff --git a/lib/std/os/linux/test.zig b/lib/std/os/linux/test.zig index 00653a153c9e..dcde98688704 100644 --- a/lib/std/os/linux/test.zig +++ b/lib/std/os/linux/test.zig @@ -41,7 +41,7 @@ test "timer" { var err: linux.E = linux.E.init(epoll_fd); try expect(err == .SUCCESS); - const timer_fd = linux.timerfd_create(linux.CLOCK_ID.MONOTONIC, .{}); + const timer_fd = linux.timerfd_create(linux.TIMERFD_CLOCK.MONOTONIC, .{}); try expect(linux.E.init(timer_fd) == .SUCCESS); const time_interval = linux.timespec{ diff --git a/lib/std/posix.zig b/lib/std/posix.zig index 5e474790544d..57a9cbd4c8d2 100644 --- a/lib/std/posix.zig +++ b/lib/std/posix.zig @@ -5626,11 +5626,11 @@ pub fn dl_iterate_phdr( pub const ClockGetTimeError = error{UnsupportedClock} || UnexpectedError; -pub fn clock_gettime(clk_id: clockid_t) ClockGetTimeError!timespec { +pub fn clock_gettime(clock_id: clockid_t) ClockGetTimeError!timespec { var tp: timespec = undefined; if (native_os == .wasi and !builtin.link_libc) { var ts: timestamp_t = undefined; - switch (system.clock_time_get(clk_id, 1, &ts)) { + switch (system.clock_time_get(clock_id, 1, &ts)) { .SUCCESS => { tp = .{ .sec = @intCast(ts / std.time.ns_per_s), @@ -5643,7 +5643,7 @@ pub fn clock_gettime(clk_id: clockid_t) ClockGetTimeError!timespec { return tp; } if (native_os == .windows) { - if (clk_id == .REALTIME) { + if (clock_id == .REALTIME) { var ft: windows.FILETIME = undefined; windows.kernel32.GetSystemTimeAsFileTime(&ft); // FileTime has a granularity of 100 nanoseconds and uses the NTFS/Windows epoch. @@ -5660,7 +5660,7 @@ pub fn clock_gettime(clk_id: clockid_t) ClockGetTimeError!timespec { } } - switch (errno(system.clock_gettime(clk_id, &tp))) { + switch (errno(system.clock_gettime(clock_id, &tp))) { .SUCCESS => return tp, .FAULT => unreachable, .INVAL => return error.UnsupportedClock, @@ -5874,7 +5874,6 @@ pub fn res_mkquery( // Make a reasonably unpredictable id const ts = clock_gettime(.REALTIME) catch undefined; - clock_gettime(.REALTIME, &ts) catch {}; const UInt = std.meta.Int(.unsigned, @bitSizeOf(@TypeOf(ts.nsec))); const unsec: UInt = @bitCast(ts.nsec); const id: u32 = @truncate(unsec + unsec / 65536); @@ -7256,8 +7255,8 @@ pub const TimerFdCreateError = error{ pub const TimerFdGetError = error{InvalidHandle} || UnexpectedError; pub const TimerFdSetError = TimerFdGetError || error{Canceled}; -pub fn timerfd_create(clockid: system.timerfd_clockid_t, flags: system.TFD) TimerFdCreateError!fd_t { - const rc = system.timerfd_create(clockid, @bitCast(flags)); +pub fn timerfd_create(clock_id: system.timerfd_clockid_t, flags: system.TFD) TimerFdCreateError!fd_t { + const rc = system.timerfd_create(clock_id, @bitCast(flags)); return switch (errno(rc)) { .SUCCESS => @intCast(rc), .INVAL => unreachable, From d5c65be2d22a8288911bb4d08106c7a21d5c2e8a Mon Sep 17 00:00:00 2001 From: Chris Boesch <48591413+chrboesch@users.noreply.github.com> Date: Mon, 27 Jan 2025 17:42:03 +0100 Subject: [PATCH 3/5] Changed clk_id back to clock_id --- lib/std/posix.zig | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/std/posix.zig b/lib/std/posix.zig index 57a9cbd4c8d2..15621c91363a 100644 --- a/lib/std/posix.zig +++ b/lib/std/posix.zig @@ -5668,10 +5668,10 @@ pub fn clock_gettime(clock_id: clockid_t) ClockGetTimeError!timespec { } } -pub fn clock_getres(clk_id: clockid_t, res: *timespec) ClockGetTimeError!void { +pub fn clock_getres(clock_id: clockid_t, res: *timespec) ClockGetTimeError!void { if (native_os == .wasi and !builtin.link_libc) { var ts: timestamp_t = undefined; - switch (system.clock_res_get(@bitCast(clk_id), &ts)) { + switch (system.clock_res_get(@bitCast(clock_id), &ts)) { .SUCCESS => res.* = .{ .sec = @intCast(ts / std.time.ns_per_s), .nsec = @intCast(ts % std.time.ns_per_s), @@ -5682,7 +5682,7 @@ pub fn clock_getres(clk_id: clockid_t, res: *timespec) ClockGetTimeError!void { return; } - switch (errno(system.clock_getres(clk_id, res))) { + switch (errno(system.clock_getres(clock_id, res))) { .SUCCESS => return, .FAULT => unreachable, .INVAL => return error.UnsupportedClock, From c039135e293edba47e9c9c75e3996b7e34dadf0b Mon Sep 17 00:00:00 2001 From: Chris Boesch <48591413+chrboesch@users.noreply.github.com> Date: Wed, 29 Jan 2025 11:18:19 +0100 Subject: [PATCH 4/5] Changed error message for better understanding --- lib/std/posix.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/std/posix.zig b/lib/std/posix.zig index 15621c91363a..064f56d97fa7 100644 --- a/lib/std/posix.zig +++ b/lib/std/posix.zig @@ -5873,7 +5873,7 @@ pub fn res_mkquery( q[i + 3] = class; // Make a reasonably unpredictable id - const ts = clock_gettime(.REALTIME) catch undefined; + const ts = clock_gettime(.REALTIME) catch unreachable; const UInt = std.meta.Int(.unsigned, @bitSizeOf(@TypeOf(ts.nsec))); const unsec: UInt = @bitCast(ts.nsec); const id: u32 = @truncate(unsec + unsec / 65536); From 36e704c131537ffee608de8912e0831bdebc45ad Mon Sep 17 00:00:00 2001 From: Chris Boesch <48591413+chrboesch@users.noreply.github.com> Date: Wed, 29 Jan 2025 11:21:49 +0100 Subject: [PATCH 5/5] Removed unnecessary variable --- lib/std/c/freebsd.zig | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/std/c/freebsd.zig b/lib/std/c/freebsd.zig index f4ea95a55b6a..6fd33c80b716 100644 --- a/lib/std/c/freebsd.zig +++ b/lib/std/c/freebsd.zig @@ -20,7 +20,6 @@ const timespec = std.c.timespec; const uid_t = std.c.uid_t; const sf_hdtr = std.c.sf_hdtr; const clockid_t = std.c.clockid_t; -const timerfd_clockid_t = std.c.timerfd_clockid_t; comptime { assert(builtin.os.tag == .freebsd); // Prevent access of std.c symbols on wrong OS.