Make sure a fd is properly closed if initialization fails

src/posix/tty.rs

@@ -56,6 +56,24 @@ pub enum BreakDuration {
     Arbitrary(std::num::NonZeroI32),
 }
 
+/// Wrapper for RawFd to assure that it's properly closed,
+/// even if the enclosing function exits early.
+///
+/// This is similar to the (nightly-only) std::os::unix::io::OwnedFd.
+struct OwnedFd(RawFd);
+
+impl Drop for OwnedFd {
+    fn drop(&mut self) {
+        let _ = close(self.0);
+    }
+}
+
+impl OwnedFd {
+    fn into_raw(self) -> RawFd {
+        self.0
+    }
+}
+
 impl TTYPort {
     /// Opens a TTY device as a serial port.
     ///
@@ -75,65 +93,50 @@ impl TTYPort {
         use nix::libc::{cfmakeraw, tcflush, tcgetattr, tcsetattr};
 
         let path = Path::new(&builder.path);
-        let fd = nix::fcntl::open(
+        let fd = OwnedFd(nix::fcntl::open(
             path,
             OFlag::O_RDWR | OFlag::O_NOCTTY | OFlag::O_NONBLOCK,
             nix::sys::stat::Mode::empty(),
-        )?;
+        )?);
 
         let mut termios = MaybeUninit::uninit();
-        let res = unsafe { tcgetattr(fd, termios.as_mut_ptr()) };
-        if let Err(e) = nix::errno::Errno::result(res) {
-            close(fd);
-            return Err(e.into());
-        }
+        nix::errno::Errno::result(unsafe { tcgetattr(fd.0, termios.as_mut_ptr()) })?;
         let mut termios = unsafe { termios.assume_init() };
 
-        // If any of these steps fail, then we should abort creation of the
-        // TTYPort and ensure the file descriptor is closed.
-        // So we wrap these calls in a block and check the result.
+        // setup TTY for binary serial port access
+        // Enable reading from the port and ignore all modem control lines
+        termios.c_cflag |= libc::CREAD | libc::CLOCAL;
+        // Enable raw mode which disables any implicit processing of the input or output data streams
+        // This also sets no timeout period and a read will block until at least one character is
+        // available.
+        unsafe { cfmakeraw(&mut termios) };
+
+        // write settings to TTY
+        unsafe { tcsetattr(fd.0, libc::TCSANOW, &termios) };
+
+        // Read back settings from port and confirm they were applied correctly
+        let mut actual_termios = MaybeUninit::uninit();
+        unsafe { tcgetattr(fd.0, actual_termios.as_mut_ptr()) };
+        let actual_termios = unsafe { actual_termios.assume_init() };
+
+        if actual_termios.c_iflag != termios.c_iflag
+            || actual_termios.c_oflag != termios.c_oflag
+            || actual_termios.c_lflag != termios.c_lflag
+            || actual_termios.c_cflag != termios.c_cflag
         {
-            // setup TTY for binary serial port access
-            // Enable reading from the port and ignore all modem control lines
-            termios.c_cflag |= libc::CREAD | libc::CLOCAL;
-            // Enable raw mode which disables any implicit processing of the input or output data streams
-            // This also sets no timeout period and a read will block until at least one character is
-            // available.
-            unsafe { cfmakeraw(&mut termios) };
-
-            // write settings to TTY
-            unsafe { tcsetattr(fd, libc::TCSANOW, &termios) };
-
-            // Read back settings from port and confirm they were applied correctly
-            let mut actual_termios = MaybeUninit::uninit();
-            unsafe { tcgetattr(fd, actual_termios.as_mut_ptr()) };
-            let actual_termios = unsafe { actual_termios.assume_init() };
-
-            if actual_termios.c_iflag != termios.c_iflag
-                || actual_termios.c_oflag != termios.c_oflag
-                || actual_termios.c_lflag != termios.c_lflag
-                || actual_termios.c_cflag != termios.c_cflag
-            {
-                return Err(Error::new(
-                    ErrorKind::Unknown,
-                    "Settings did not apply correctly",
-                ));
-            };
-
-            unsafe { tcflush(fd, libc::TCIOFLUSH) };
-
-            // clear O_NONBLOCK flag
-            fcntl(fd, F_SETFL(nix::fcntl::OFlag::empty()))?;
+            return Err(Error::new(
+                ErrorKind::Unknown,
+                "Settings did not apply correctly",
+            ));
+        };
 
-            Ok(())
-        }
-        .map_err(|e: Error| {
-            close(fd);
-            e
-        })?;
+        unsafe { tcflush(fd.0, libc::TCIOFLUSH) };
+
+        // clear O_NONBLOCK flag
+        fcntl(fd.0, F_SETFL(nix::fcntl::OFlag::empty()))?;
 
         // Configure the low-level port settings
-        let mut termios = termios::get_termios(fd)?;
+        let mut termios = termios::get_termios(fd.0)?;
         termios::set_parity(&mut termios, builder.parity);
         termios::set_flow_control(&mut termios, builder.flow_control);
         termios::set_data_bits(&mut termios, builder.data_bits);
@@ -143,11 +146,11 @@ impl TTYPort {
         #[cfg(any(target_os = "ios", target_os = "macos"))]
         termios::set_termios(fd, &termios, builder.baud_rate)?;
         #[cfg(not(any(target_os = "ios", target_os = "macos")))]
-        termios::set_termios(fd, &termios)?;
+        termios::set_termios(fd.0, &termios)?;
 
         // Return the final port object
         Ok(TTYPort {
-            fd,
+            fd: fd.into_raw(),
             timeout: builder.timeout,
             exclusive: false,
             port_name: Some(builder.path.clone()),