Commit 7fd937fef4

Andrew Kelley <andrew@ziglang.org>
2020-06-02 21:28:46
cleanups
* improve docs * add TODO comments for things that don't have open issues * remove redundant namespacing of struct fields * guard against ioctl returning EINTR * remove the general std.os.ioctl function in favor of the specific ioctl_SIOCGIFINDEX function. This allows us to have a more precise error set, and more type-safe API.
1 parent 0d091dc
Changed files (5)
lib/std/net/test.zig
@@ -48,6 +48,7 @@ test "parse and render IPv6 addresses" {
     testing.expectError(error.InvalidEnd, net.Address.parseIp6("FF01:0:0:0:0:0:0:FB:", 0));
     testing.expectError(error.Incomplete, net.Address.parseIp6("FF01:", 0));
     testing.expectError(error.InvalidIpv4Mapping, net.Address.parseIp6("::123.123.123.123", 0));
+    // TODO Make this test pass on other operating systems.
     if (std.builtin.os.tag == .linux) {
         testing.expectError(error.Incomplete, net.Address.resolveIp6("ff01::fb%", 0));
         testing.expectError(error.Overflow, net.Address.resolveIp6("ff01::fb%wlp3s0s0s0s0s0s0s0s0", 0));
@@ -56,8 +57,9 @@ test "parse and render IPv6 addresses" {
 }
 
 test "invalid but parseable IPv6 scope ids" {
-    // Currently, resolveIp6 with alphanumerical scope IDs only works on Linux.
     if (std.builtin.os.tag != .linux) {
+        // Currently, resolveIp6 with alphanumerical scope IDs only works on Linux.
+        // TODO Make this test pass on other operating systems.
         return error.SkipZigTest;
     }
 
lib/std/os/bits/linux.zig
@@ -1719,21 +1719,21 @@ pub const ifmap = extern struct {
 };
 
 pub const ifreq = extern struct {
-    ifr_ifrn: extern union {
+    ifrn: extern union {
         name: [IFNAMESIZE]u8,
     },
-    ifr_ifru: extern union {
-        ifru_addr: sockaddr,
-        ifru_dstaddr: sockaddr,
-        ifru_broadaddr: sockaddr,
-        ifru_netmask: sockaddr,
-        ifru_hwaddr: sockaddr,
-        ifru_flags: i16,
-        ifru_ivalue: i32,
-        ifru_mtu: i32,
-        ifru_map: ifmap,
-        ifru_slave: [IFNAMESIZE - 1:0]u8,
-        ifru_newname: [IFNAMESIZE - 1:0]u8,
-        ifru_data: ?[*]u8,
+    ifru: extern union {
+        addr: sockaddr,
+        dstaddr: sockaddr,
+        broadaddr: sockaddr,
+        netmask: sockaddr,
+        hwaddr: sockaddr,
+        flags: i16,
+        ivalue: i32,
+        mtu: i32,
+        map: ifmap,
+        slave: [IFNAMESIZE - 1:0]u8,
+        newname: [IFNAMESIZE - 1:0]u8,
+        data: ?[*]u8,
     },
 };
lib/std/os/linux.zig
@@ -1186,15 +1186,15 @@ pub fn getrusage(who: i32, usage: *rusage) usize {
 }
 
 pub fn tcgetattr(fd: fd_t, termios_p: *termios) usize {
-    return ioctl(fd, TCGETS, @ptrToInt(termios_p));
+    return syscall3(.ioctl, @bitCast(usize, @as(isize, fd)), TCGETS, @ptrToInt(termios_p));
 }
 
 pub fn tcsetattr(fd: fd_t, optional_action: TCSA, termios_p: *const termios) usize {
-    return ioctl(fd, TCSETS + @enumToInt(optional_action), @ptrToInt(termios_p));
+    return syscall3(.ioctl, @bitCast(usize, @as(isize, fd)), TCSETS + @enumToInt(optional_action), @ptrToInt(termios_p));
 }
 
-pub fn ioctl(fd: fd_t, request: i32, arg: var) usize {
-    return syscall3(.ioctl, @bitCast(usize, @as(isize, fd)), @bitCast(usize, @as(isize, request)), arg);
+pub fn ioctl(fd: fd_t, request: u32, arg: usize) usize {
+    return syscall3(.ioctl, @bitCast(usize, @as(isize, fd)), request, arg);
 }
 
 test "" {
lib/std/net.zig
@@ -22,7 +22,7 @@ pub const Address = extern union {
     //pub const localhost = initIp4(parseIp4("127.0.0.1") catch unreachable, 0);
 
     /// Parse the given IP address string into an Address value.
-    /// It is recommended to use Address.resolveIp instead, to handle
+    /// It is recommended to use `resolveIp` instead, to handle
     /// IPv6 link-local unix addresses.
     pub fn parseIp(name: []const u8, port: u16) !Address {
         if (parseIp4(name, port)) |ip4| return ip4 else |err| switch (err) {
@@ -78,6 +78,7 @@ pub const Address = extern union {
 
     /// Parse a given IPv6 address string into an Address.
     /// Assumes the Scope ID of the address is fully numeric.
+    /// For non-numeric addresses, see `resolveIp6`.
     pub fn parseIp6(buf: []const u8, port: u16) !Address {
         var result = Address{
             .in6 = os.sockaddr_in6{
@@ -185,8 +186,7 @@ pub const Address = extern union {
     }
 
     pub fn resolveIp6(buf: []const u8, port: u16) !Address {
-        // FIXME: this is a very bad implementation, since it's only a copy
-        // of parseIp6 with alphanumerical scope id support
+        // TODO: Unify the implementations of resolveIp6 and parseIp6.
         var result = Address{
             .in6 = os.sockaddr_in6{
                 .scope_id = 0,
@@ -543,17 +543,13 @@ fn if_nametoindex(name: []const u8) !u32 {
     var sockfd = try os.socket(os.AF_UNIX, os.SOCK_DGRAM | os.SOCK_CLOEXEC, 0);
     defer os.close(sockfd);
 
-    std.mem.copy(u8, &ifr.ifr_ifrn.name, name);
-    ifr.ifr_ifrn.name[name.len] = 0;
+    std.mem.copy(u8, &ifr.ifrn.name, name);
+    ifr.ifrn.name[name.len] = 0;
 
-    os.ioctl(sockfd, os.system.SIOCGIFINDEX, @ptrToInt(&ifr)) catch |err| {
-        switch (err) {
-            error.NoDevice => return error.InterfaceNotFound,
-            else => return err,
-        }
-    };
+    // TODO investigate if this needs to be integrated with evented I/O.
+    try os.ioctl_SIOCGIFINDEX(sockfd, &ifr);
 
-    return @bitCast(u32, ifr.ifr_ifru.ifru_ivalue);
+    return @bitCast(u32, ifr.ifru.ivalue);
 }
 
 pub const AddressList = struct {
lib/std/os.zig
@@ -2390,8 +2390,15 @@ pub fn isatty(handle: fd_t) bool {
         return true;
     }
     if (builtin.os.tag == .linux) {
-        var wsz: linux.winsize = undefined;
-        return linux.ioctl(handle, linux.TIOCGWINSZ, @ptrToInt(&wsz)) == 0;
+        while (true) {
+            var wsz: linux.winsize = undefined;
+            const fd = @bitCast(usize, @as(isize, handle));
+            switch (linux.syscall3(.ioctl, fd, linux.TIOCGWINSZ, @ptrToInt(&wsz))) {
+                0 => return true,
+                EINTR => continue,
+                else => return false,
+            }
+        }
     }
     unreachable;
 }
@@ -4880,12 +4887,15 @@ pub fn getrusage(who: i32) rusage {
 pub const TermiosGetError = error{NotATerminal} || UnexpectedError;
 
 pub fn tcgetattr(handle: fd_t) TermiosGetError!termios {
-    var term: termios = undefined;
-    switch (errno(system.tcgetattr(handle, &term))) {
-        0 => return term,
-        EBADF => unreachable,
-        ENOTTY => return error.NotATerminal,
-        else => |err| return unexpectedErrno(err),
+    while (true) {
+        var term: termios = undefined;
+        switch (errno(system.tcgetattr(handle, &term))) {
+            0 => return term,
+            EINTR => continue,
+            EBADF => unreachable,
+            ENOTTY => return error.NotATerminal,
+            else => |err| return unexpectedErrno(err),
+        }
     }
 }
 
@@ -4905,16 +4915,24 @@ pub fn tcsetattr(handle: fd_t, optional_action: TCSA, termios_p: termios) Termio
     }
 }
 
-pub fn ioctl(handle: fd_t, request: i32, arg: var) !void {
-    switch (errno(system.ioctl(handle, request, arg))) {
-        0 => {},
-        EINVAL => unreachable,
-        ENOTTY => unreachable,
-        ENXIO => unreachable,
-        EBADF => return error.BadFile,
-        EINTR => return error.CaughtSignal,
-        EIO => return error.FileSystem,
-        ENODEV => return error.NoDevice,
-        else => |err| return unexpectedErrno(err),
+const IoCtl_SIOCGIFINDEX_Error = error{
+    FileSystem,
+    InterfaceNotFound,
+} || UnexpectedError;
+
+pub fn ioctl_SIOCGIFINDEX(fd: fd_t, ifr: *ifreq) IoCtl_SIOCGIFINDEX_Error!void {
+    while (true) {
+        switch (errno(system.ioctl(fd, SIOCGIFINDEX, @ptrToInt(ifr)))) {
+            0 => return,
+            EINVAL => unreachable, // Bad parameters.
+            ENOTTY => unreachable,
+            ENXIO => unreachable,
+            EBADF => unreachable, // Always a race condition.
+            EFAULT => unreachable, // Bad pointer parameter.
+            EINTR => continue,
+            EIO => return error.FileSystem,
+            ENODEV => return error.InterfaceNotFound,
+            else => |err| return unexpectedErrno(err),
+        }
     }
 }