From 7fd937fef4547a98d7c33ea67eca76e6336f9152 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Tue, 2 Jun 2020 15:28:46 -0400 Subject: [PATCH] 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. --- lib/std/net.zig | 20 ++++++-------- lib/std/net/test.zig | 4 ++- lib/std/os.zig | 56 ++++++++++++++++++++++++++------------- lib/std/os/bits/linux.zig | 28 ++++++++++---------- lib/std/os/linux.zig | 8 +++--- 5 files changed, 66 insertions(+), 50 deletions(-) diff --git a/lib/std/net.zig b/lib/std/net.zig index a0bc99f1a5..34af916d13 100644 --- a/lib/std/net.zig +++ b/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 { diff --git a/lib/std/net/test.zig b/lib/std/net/test.zig index cec9185eae..5327e88ffc 100644 --- a/lib/std/net/test.zig +++ b/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; } diff --git a/lib/std/os.zig b/lib/std/os.zig index 497dd0f8dd..c56f6e0db0 100644 --- a/lib/std/os.zig +++ b/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), + } } } diff --git a/lib/std/os/bits/linux.zig b/lib/std/os/bits/linux.zig index 1a8ff84ac0..64832673f1 100644 --- a/lib/std/os/bits/linux.zig +++ b/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, }, }; diff --git a/lib/std/os/linux.zig b/lib/std/os/linux.zig index 08739dc1d9..3b8df3d173 100644 --- a/lib/std/os/linux.zig +++ b/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 "" {