std.posix: Use separate clock ID enums for clock_gettime() and timerfd_create() (#22627)

Author: chrboesch

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));
 

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,
@@ -9256,7 +9273,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,

lib/std/os/linux.zig

@@ -2215,7 +2215,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: timerfd_clockid_t, flags: TFD) usize {
     return syscall2(
         .timerfd_create,
         @intFromEnum(clockid),
@@ -4696,8 +4696,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 TIMERFD_CLOCK = timerfd_clockid_t;
+pub const timerfd_clockid_t = enum(u32) {
+    REALTIME = 0,
+    MONOTONIC = 1,
     _,
 };
 

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.TIMERFD_CLOCK.MONOTONIC, .{});
     try expect(linux.E.init(timer_fd) == .SUCCESS);
 
     const time_interval = linux.timespec{

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;
@@ -5653,21 +5655,21 @@ 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(clock_id: clockid_t) ClockGetTimeError!timespec {
+    var tp: timespec = undefined;
     if (native_os == .wasi and !builtin.link_libc) {
-        var ts: wasi.timestamp_t = undefined;
+        var ts: timestamp_t = undefined;
         switch (system.clock_time_get(clock_id, 1, &ts)) {
             .SUCCESS => {
-                tp.* = .{
+                tp = .{
                     .sec = @intCast(ts / std.time.ns_per_s),
                     .nsec = @intCast(ts % std.time.ns_per_s),
                 };
             },
             .INVAL => return error.UnsupportedClock,
             else => |err| return unexpectedErrno(err),
         }
-        return;
+        return tp;
     }
     if (native_os == .windows) {
         if (clock_id == .REALTIME) {
@@ -5676,19 +5678,19 @@ pub fn clock_gettime(clock_id: clockid_t, tp: *timespec) ClockGetTimeError!void
             // 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(clock_id, &tp))) {
+        .SUCCESS => return tp,
         .FAULT => unreachable,
         .INVAL => return error.UnsupportedClock,
         else => |err| return unexpectedErrno(err),
@@ -5697,7 +5699,7 @@ pub fn clock_gettime(clock_id: clockid_t, tp: *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: wasi.timestamp_t = undefined;
+        var ts: timestamp_t = undefined;
         switch (system.clock_res_get(@bitCast(clock_id), &ts)) {
             .SUCCESS => res.* = .{
                 .sec = @intCast(ts / std.time.ns_per_s),
@@ -5910,8 +5912,7 @@ pub fn res_mkquery(
     q[i + 3] = class;
 
     // Make a reasonably unpredictable id
-    var ts: timespec = undefined;
-    clock_gettime(.REALTIME, &ts) catch {};
+    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);
@@ -7293,7 +7294,7 @@ 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 {
+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),

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 };
     }