From af6ace62090aa03617fee2ef185363702634cff0 Mon Sep 17 00:00:00 2001 From: Marcel Hellwig Date: Mon, 6 May 2019 14:54:27 +0200 Subject: [PATCH 1/2] convert custom try macro to `?` resolves #60580 --- src/libstd/sys/redox/process.rs | 49 +++++++++------------ src/libstd/sys/unix/process/process_unix.rs | 27 +++++------- 2 files changed, 31 insertions(+), 45 deletions(-) diff --git a/src/libstd/sys/redox/process.rs b/src/libstd/sys/redox/process.rs index 8e6f50773abfe..5039c2e3ddfb3 100644 --- a/src/libstd/sys/redox/process.rs +++ b/src/libstd/sys/redox/process.rs @@ -254,44 +254,37 @@ impl Command { // have the drop glue anyway because this code never returns (the // child will either exec() or invoke syscall::exit) unsafe fn do_exec(&mut self, stdio: ChildPipes) -> io::Error { - macro_rules! t { - ($e:expr) => (match $e { - Ok(e) => e, - Err(e) => return e, - }) - } - if let Some(fd) = stdio.stderr.fd() { - t!(cvt(syscall::dup2(fd, 2, &[]))); - let mut flags = t!(cvt(syscall::fcntl(2, syscall::F_GETFD, 0))); + cvt(syscall::dup2(fd, 2, &[]))?; + let mut flags = cvt(syscall::fcntl(2, syscall::F_GETFD, 0))?; flags &= ! syscall::O_CLOEXEC; - t!(cvt(syscall::fcntl(2, syscall::F_SETFD, flags))); + cvt(syscall::fcntl(2, syscall::F_SETFD, flags))?; } if let Some(fd) = stdio.stdout.fd() { - t!(cvt(syscall::dup2(fd, 1, &[]))); - let mut flags = t!(cvt(syscall::fcntl(1, syscall::F_GETFD, 0))); + cvt(syscall::dup2(fd, 1, &[]))?; + let mut flags = cvt(syscall::fcntl(1, syscall::F_GETFD, 0))?; flags &= ! syscall::O_CLOEXEC; - t!(cvt(syscall::fcntl(1, syscall::F_SETFD, flags))); + cvt(syscall::fcntl(1, syscall::F_SETFD, flags))?; } if let Some(fd) = stdio.stdin.fd() { - t!(cvt(syscall::dup2(fd, 0, &[]))); - let mut flags = t!(cvt(syscall::fcntl(0, syscall::F_GETFD, 0))); + cvt(syscall::dup2(fd, 0, &[]))?; + let mut flags = cvt(syscall::fcntl(0, syscall::F_GETFD, 0))?; flags &= ! syscall::O_CLOEXEC; - t!(cvt(syscall::fcntl(0, syscall::F_SETFD, flags))); + cvt(syscall::fcntl(0, syscall::F_SETFD, flags))?; } if let Some(g) = self.gid { - t!(cvt(syscall::setregid(g as usize, g as usize))); + cvt(syscall::setregid(g as usize, g as usize))?; } if let Some(u) = self.uid { - t!(cvt(syscall::setreuid(u as usize, u as usize))); + cvt(syscall::setreuid(u as usize, u as usize))?; } if let Some(ref cwd) = self.cwd { - t!(cvt(syscall::chdir(cwd))); + cvt(syscall::chdir(cwd))?; } for callback in self.closures.iter_mut() { - t!(callback()); + callback()?; } self.env.apply(); @@ -313,7 +306,7 @@ impl Command { }; let mut file = if let Some(program) = program { - t!(File::open(program.as_os_str())) + File::open(program.as_os_str())? } else { return io::Error::from_raw_os_error(syscall::ENOENT); }; @@ -327,7 +320,7 @@ impl Command { let mut shebang = [0; 2]; let mut read = 0; loop { - match t!(reader.read(&mut shebang[read..])) { + match reader.read(&mut shebang[read..])? { 0 => break, n => read += n, } @@ -338,9 +331,9 @@ impl Command { // First of all, since we'll be passing another file to // fexec(), we need to manually check that we have permission // to execute this file: - let uid = t!(cvt(syscall::getuid())); - let gid = t!(cvt(syscall::getgid())); - let meta = t!(file.metadata()); + let uid = cvt(syscall::getuid())?; + let gid = cvt(syscall::getgid())?; + let meta = file.metadata()?; let mode = if uid == meta.uid() as usize { meta.mode() >> 3*2 & 0o7 @@ -355,7 +348,7 @@ impl Command { // Second of all, we need to actually read which interpreter it wants let mut interpreter = Vec::new(); - t!(reader.read_until(b'\n', &mut interpreter)); + reader.read_until(b'\n', &mut interpreter)?; // Pop one trailing newline, if any if interpreter.ends_with(&[b'\n']) { interpreter.pop().unwrap(); @@ -373,11 +366,11 @@ impl Command { }; if let Some(ref interpreter) = interpreter { let path: &OsStr = OsStr::from_bytes(&interpreter); - file = t!(File::open(path)); + file = File::open(path)?; args.push([interpreter.as_ptr() as usize, interpreter.len()]); } else { - t!(file.seek(SeekFrom::Start(0))); + file.seek(SeekFrom::Start(0))?; } args.push([self.program.as_ptr() as usize, self.program.len()]); diff --git a/src/libstd/sys/unix/process/process_unix.rs b/src/libstd/sys/unix/process/process_unix.rs index 220b1fd453131..07f813315e8c1 100644 --- a/src/libstd/sys/unix/process/process_unix.rs +++ b/src/libstd/sys/unix/process/process_unix.rs @@ -167,26 +167,19 @@ impl Command { ) -> io::Error { use crate::sys::{self, cvt_r}; - macro_rules! t { - ($e:expr) => (match $e { - Ok(e) => e, - Err(e) => return e, - }) - } - if let Some(fd) = stdio.stdin.fd() { - t!(cvt_r(|| libc::dup2(fd, libc::STDIN_FILENO))); + cvt_r(|| libc::dup2(fd, libc::STDIN_FILENO))?; } if let Some(fd) = stdio.stdout.fd() { - t!(cvt_r(|| libc::dup2(fd, libc::STDOUT_FILENO))); + cvt_r(|| libc::dup2(fd, libc::STDOUT_FILENO))?; } if let Some(fd) = stdio.stderr.fd() { - t!(cvt_r(|| libc::dup2(fd, libc::STDERR_FILENO))); + cvt_r(|| libc::dup2(fd, libc::STDERR_FILENO))?; } if cfg!(not(any(target_os = "l4re"))) { if let Some(u) = self.get_gid() { - t!(cvt(libc::setgid(u as gid_t))); + cvt(libc::setgid(u as gid_t))?; } if let Some(u) = self.get_uid() { // When dropping privileges from root, the `setgroups` call @@ -198,11 +191,11 @@ impl Command { // privilege dropping function. let _ = libc::setgroups(0, ptr::null()); - t!(cvt(libc::setuid(u as uid_t))); + cvt(libc::setuid(u as uid_t))?; } } if let Some(ref cwd) = *self.get_cwd() { - t!(cvt(libc::chdir(cwd.as_ptr()))); + cvt(libc::chdir(cwd.as_ptr()))?; } // emscripten has no signal support. @@ -225,10 +218,10 @@ impl Command { 0, mem::size_of::()); } else { - t!(cvt(libc::sigemptyset(&mut set))); + cvt(libc::sigemptyset(&mut set))?; } - t!(cvt(libc::pthread_sigmask(libc::SIG_SETMASK, &set, - ptr::null_mut()))); + cvt(libc::pthread_sigmask(libc::SIG_SETMASK, &set, + ptr::null_mut()))?; let ret = sys::signal(libc::SIGPIPE, libc::SIG_DFL); if ret == libc::SIG_ERR { return io::Error::last_os_error() @@ -236,7 +229,7 @@ impl Command { } for callback in self.get_closures().iter_mut() { - t!(callback()); + callback()?; } // Although we're performing an exec here we may also return with an From 5458b651b1ecad5cc334b494209352ac935360ce Mon Sep 17 00:00:00 2001 From: Marcel Hellwig Date: Mon, 6 May 2019 15:40:34 +0200 Subject: [PATCH 2/2] use exhaustive_patterns to be able to use `?` --- src/libstd/sys/redox/process.rs | 16 +++++++++------- src/libstd/sys/unix/process/process_unix.rs | 11 ++++++----- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/libstd/sys/redox/process.rs b/src/libstd/sys/redox/process.rs index 5039c2e3ddfb3..2a553b2c93bd3 100644 --- a/src/libstd/sys/redox/process.rs +++ b/src/libstd/sys/redox/process.rs @@ -150,7 +150,7 @@ impl Command { match cvt(syscall::clone(0))? { 0 => { drop(input); - let err = self.do_exec(theirs); + let Err(err) = self.do_exec(theirs); let errno = err.raw_os_error().unwrap_or(syscall::EINVAL) as u32; let bytes = [ (errno >> 24) as u8, @@ -218,7 +218,10 @@ impl Command { } match self.setup_io(default, true) { - Ok((_, theirs)) => unsafe { self.do_exec(theirs) }, + Ok((_, theirs)) => unsafe { + let Err(e) = self.do_exec(theirs); + e + }, Err(e) => e, } } @@ -253,7 +256,7 @@ impl Command { // allocation). Instead we just close it manually. This will never // have the drop glue anyway because this code never returns (the // child will either exec() or invoke syscall::exit) - unsafe fn do_exec(&mut self, stdio: ChildPipes) -> io::Error { + unsafe fn do_exec(&mut self, stdio: ChildPipes) -> Result { if let Some(fd) = stdio.stderr.fd() { cvt(syscall::dup2(fd, 2, &[]))?; let mut flags = cvt(syscall::fcntl(2, syscall::F_GETFD, 0))?; @@ -308,7 +311,7 @@ impl Command { let mut file = if let Some(program) = program { File::open(program.as_os_str())? } else { - return io::Error::from_raw_os_error(syscall::ENOENT); + return Err(io::Error::from_raw_os_error(syscall::ENOENT)); }; // Push all the arguments @@ -343,7 +346,7 @@ impl Command { meta.mode() & 0o7 }; if mode & 1 == 0 { - return io::Error::from_raw_os_error(syscall::EPERM); + return Err(io::Error::from_raw_os_error(syscall::EPERM)); } // Second of all, we need to actually read which interpreter it wants @@ -389,13 +392,12 @@ impl Command { } if let Err(err) = syscall::fexec(file.as_raw_fd(), &args, &vars) { - io::Error::from_raw_os_error(err.errno as i32) + Err(io::Error::from_raw_os_error(err.errno as i32)) } else { panic!("return from exec without err"); } } - fn setup_io(&self, default: Stdio, needs_stdin: bool) -> io::Result<(StdioPipes, ChildPipes)> { let null = Stdio::Null; diff --git a/src/libstd/sys/unix/process/process_unix.rs b/src/libstd/sys/unix/process/process_unix.rs index 07f813315e8c1..80fe763aecc88 100644 --- a/src/libstd/sys/unix/process/process_unix.rs +++ b/src/libstd/sys/unix/process/process_unix.rs @@ -47,7 +47,7 @@ impl Command { match result { 0 => { drop(input); - let err = self.do_exec(theirs, envp.as_ref()); + let Err(err) = self.do_exec(theirs, envp.as_ref()); let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32; let bytes = [ (errno >> 24) as u8, @@ -123,7 +123,8 @@ impl Command { // environment lock before we try to exec. let _lock = sys::os::env_lock(); - self.do_exec(theirs, envp.as_ref()) + let Err(e) = self.do_exec(theirs, envp.as_ref()); + e } } Err(e) => e, @@ -164,7 +165,7 @@ impl Command { &mut self, stdio: ChildPipes, maybe_envp: Option<&CStringArray> - ) -> io::Error { + ) -> Result { use crate::sys::{self, cvt_r}; if let Some(fd) = stdio.stdin.fd() { @@ -224,7 +225,7 @@ impl Command { ptr::null_mut()))?; let ret = sys::signal(libc::SIGPIPE, libc::SIG_DFL); if ret == libc::SIG_ERR { - return io::Error::last_os_error() + return Err(io::Error::last_os_error()) } } @@ -254,7 +255,7 @@ impl Command { } libc::execvp(self.get_argv()[0], self.get_argv().as_ptr()); - io::Error::last_os_error() + Err(io::Error::last_os_error()) } #[cfg(not(any(target_os = "macos", target_os = "freebsd",