Merge pull request #1181 from sozu-proxy/devel/edemolis/fix/raw-fd

Fix OwnedFd closing RawFd

Author: FlorentinDUBOIS

bin/src/ctl/request_builder.rs

@@ -80,11 +80,7 @@ impl CommandManager {
 
     pub fn reload_configuration(&mut self, path: Option<String>) -> Result<(), CtlError> {
         debug!("Reloading configuration…");
-        let path = match path {
-            Some(p) => p,
-            None => String::new(),
-        };
-        self.send_request(RequestType::ReloadConfiguration(path).into())
+        self.send_request(RequestType::ReloadConfiguration(path.unwrap_or_default()).into())
     }
 
     pub fn list_frontends(
@@ -248,12 +244,9 @@ impl CommandManager {
                     address: address.into(),
                     hostname,
                     path: PathRule::from_cli_options(path_prefix, path_regex, path_equals),
-                    method: method.map(String::from),
+                    method,
                     position: RulePosition::Tree.into(),
-                    tags: match tags {
-                        Some(tags) => tags,
-                        None => BTreeMap::new(),
-                    },
+                    tags: tags.unwrap_or_default(),
                 })
                 .into(),
             ),
@@ -271,7 +264,7 @@ impl CommandManager {
                     address: address.into(),
                     hostname,
                     path: PathRule::from_cli_options(path_prefix, path_regex, path_equals),
-                    method: method.map(String::from),
+                    method,
                     ..Default::default()
                 })
                 .into(),
@@ -296,12 +289,9 @@ impl CommandManager {
                     address: address.into(),
                     hostname,
                     path: PathRule::from_cli_options(path_prefix, path_regex, path_equals),
-                    method: method.map(String::from),
+                    method,
                     position: RulePosition::Tree.into(),
-                    tags: match tags {
-                        Some(tags) => tags,
-                        None => BTreeMap::new(),
-                    },
+                    tags: tags.unwrap_or_default(),
                 })
                 .into(),
             ),
@@ -319,7 +309,7 @@ impl CommandManager {
                     address: address.into(),
                     hostname,
                     path: PathRule::from_cli_options(path_prefix, path_regex, path_equals),
-                    method: method.map(String::from),
+                    method,
                     ..Default::default()
                 })
                 .into(),

bin/src/util.rs

@@ -2,10 +2,7 @@ use std::{
     ffi::OsString,
     fs::{File, read_link},
     io::{Error as IoError, Write},
-    os::{
-        fd::{FromRawFd, OwnedFd},
-        unix::io::RawFd,
-    },
+    os::{fd::BorrowedFd, unix::io::RawFd},
     path::PathBuf,
 };
 
@@ -52,37 +49,31 @@ pub enum UtilError {
 /// FD_CLOEXEC is set by default on every fd in Rust standard lib,
 /// so we need to remove the flag on the client, otherwise
 /// it won't be accessible
-pub fn enable_close_on_exec(fd: RawFd) -> Result<i32, UtilError> {
-    let file_descriptor = fcntl(unsafe { OwnedFd::from_raw_fd(fd) }, FcntlArg::F_GETFD)
-        .map_err(|err_no| UtilError::GetFlags(fd, err_no))?;
+pub fn enable_close_on_exec(raw_fd: RawFd) -> Result<i32, UtilError> {
+    let fd = unsafe { BorrowedFd::borrow_raw(raw_fd) };
+    let old_flags =
+        fcntl(fd, FcntlArg::F_GETFD).map_err(|err_no| UtilError::GetFlags(raw_fd, err_no))?;
 
-    let mut new_flags = FdFlag::from_bits(file_descriptor).ok_or(UtilError::ConvertFlags(fd))?;
+    let mut new_flags = FdFlag::from_bits(old_flags).ok_or(UtilError::ConvertFlags(raw_fd))?;
 
     new_flags.insert(FdFlag::FD_CLOEXEC);
 
-    fcntl(
-        unsafe { OwnedFd::from_raw_fd(fd) },
-        FcntlArg::F_SETFD(new_flags),
-    )
-    .map_err(|err_no| UtilError::SetFlags(fd, err_no))
+    fcntl(fd, FcntlArg::F_SETFD(new_flags)).map_err(|err_no| UtilError::SetFlags(raw_fd, err_no))
 }
 
 /// FD_CLOEXEC is set by default on every fd in Rust standard lib,
 /// so we need to remove the flag on the client, otherwise
 /// it won't be accessible
-pub fn disable_close_on_exec(fd: RawFd) -> Result<i32, UtilError> {
-    let old_flags = fcntl(unsafe { OwnedFd::from_raw_fd(fd) }, FcntlArg::F_GETFD)
-        .map_err(|err_no| UtilError::GetFlags(fd, err_no))?;
+pub fn disable_close_on_exec(raw_fd: RawFd) -> Result<i32, UtilError> {
+    let fd = unsafe { BorrowedFd::borrow_raw(raw_fd) };
+    let old_flags =
+        fcntl(fd, FcntlArg::F_GETFD).map_err(|err_no| UtilError::GetFlags(raw_fd, err_no))?;
 
-    let mut new_flags = FdFlag::from_bits(old_flags).ok_or(UtilError::ConvertFlags(fd))?;
+    let mut new_flags = FdFlag::from_bits(old_flags).ok_or(UtilError::ConvertFlags(raw_fd))?;
 
     new_flags.remove(FdFlag::FD_CLOEXEC);
 
-    fcntl(
-        unsafe { OwnedFd::from_raw_fd(fd) },
-        FcntlArg::F_SETFD(new_flags),
-    )
-    .map_err(|err_no| UtilError::SetFlags(fd, err_no))
+    fcntl(fd, FcntlArg::F_SETFD(new_flags)).map_err(|err_no| UtilError::SetFlags(raw_fd, err_no))
 }
 
 pub fn setup_metrics(config: &Config) -> Result<(), UtilError> {

command/src/proto/display.rs

@@ -157,7 +157,10 @@ impl ResponseContent {
     fn display(&self, json: bool) -> Result<(), DisplayError> {
         let content_type = match &self.content_type {
             Some(content_type) => content_type,
-            None => return Ok(println!("No content")),
+            None => {
+                println!("No content");
+                return Ok(());
+            }
         };
 
         if json {
@@ -876,7 +879,8 @@ pub fn print_certificates_with_validity(
     certs: &CertificatesWithFingerprints,
 ) -> Result<(), DisplayError> {
     if certs.certs.is_empty() {
-        return Ok(println!("No certificates match your request."));
+        println!("No certificates match your request.");
+        return Ok(());
     }
 
     let mut table = Table::new();

lib/src/backends.rs

@@ -311,12 +311,11 @@ impl BackendMap {
                 let conn = borrowed.try_connect();
 
                 conn.map(|tcp_stream| (backend.clone(), tcp_stream))
-                    .map_err(|e| {
+                    .inspect_err(|_| {
                         error!(
                             "could not connect {} to {:?} using session {} ({} failures)",
                             cluster_id, borrowed.address, sticky_session, borrowed.failures
-                        );
-                        e
+                        )
                     })
             });