Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error printing in process_posix.rs #85

Merged
merged 1 commit into from
Oct 6, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 23 additions & 14 deletions src/process_posix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//! See run_command comments for why.

use crate::process::Termination;
use std::io::Read;
use std::io::{Error, Read};
use std::os::fd::FromRawFd;
use std::os::unix::process::ExitStatusExt;

Expand All @@ -12,22 +12,31 @@ extern "C" {
static environ: *const *mut libc::c_char;
}

fn check_posix(func: &str, ret: libc::c_int) -> anyhow::Result<()> {
if ret < 0 {
fn check_posix_spawn(func: &str, ret: libc::c_int) -> anyhow::Result<()> {
if ret != 0 {
let err_str = unsafe { std::ffi::CStr::from_ptr(libc::strerror(ret)) };
anyhow::bail!("{}: {}", func, err_str.to_str().unwrap());
}
Ok(())
}

fn check_ret_errno(func: &str, ret: libc::c_int) -> anyhow::Result<()> {
if ret < 0 {
let errno = Error::last_os_error().raw_os_error().unwrap();
let err_str = unsafe { std::ffi::CStr::from_ptr(libc::strerror(errno)) };
anyhow::bail!("{}: {}", func, err_str.to_str().unwrap());
}
Ok(())
}

/// Wraps libc::posix_spawnattr_t, in particular to implement Drop.
struct PosixSpawnAttr(libc::posix_spawnattr_t);

impl PosixSpawnAttr {
fn new() -> anyhow::Result<Self> {
unsafe {
let mut attr: libc::posix_spawnattr_t = std::mem::zeroed();
check_posix(
check_posix_spawn(
"posix_spawnattr_init",
libc::posix_spawnattr_init(&mut attr),
)?;
Expand All @@ -41,7 +50,7 @@ impl PosixSpawnAttr {

fn setflags(&mut self, flags: libc::c_short) -> anyhow::Result<()> {
unsafe {
check_posix(
check_posix_spawn(
"posix_spawnattr_setflags",
libc::posix_spawnattr_setflags(self.as_ptr(), flags),
)
Expand All @@ -64,7 +73,7 @@ impl PosixSpawnFileActions {
fn new() -> anyhow::Result<Self> {
unsafe {
let mut actions: libc::posix_spawn_file_actions_t = std::mem::zeroed();
check_posix(
check_posix_spawn(
"posix_spawn_file_actions_init",
libc::posix_spawn_file_actions_init(&mut actions),
)?;
Expand All @@ -84,7 +93,7 @@ impl PosixSpawnFileActions {
mode: libc::mode_t,
) -> anyhow::Result<()> {
unsafe {
check_posix(
check_posix_spawn(
"posix_spawn_file_actions_addopen",
libc::posix_spawn_file_actions_addopen(
self.as_ptr(),
Expand All @@ -99,7 +108,7 @@ impl PosixSpawnFileActions {

fn adddup2(&mut self, fd: i32, newfd: i32) -> anyhow::Result<()> {
unsafe {
check_posix(
check_posix_spawn(
"posix_spawn_file_actions_adddup2",
libc::posix_spawn_file_actions_adddup2(self.as_ptr(), fd, newfd),
)
Expand All @@ -108,7 +117,7 @@ impl PosixSpawnFileActions {

fn addclose(&mut self, fd: i32) -> anyhow::Result<()> {
unsafe {
check_posix(
check_posix_spawn(
"posix_spawn_file_actions_addclose",
libc::posix_spawn_file_actions_addclose(self.as_ptr(), fd),
)
Expand All @@ -131,11 +140,11 @@ fn pipe2() -> anyhow::Result<[libc::c_int; 2]> {

// Mac: specially handled below with POSIX_SPAWN_CLOEXEC_DEFAULT
#[cfg(target_os = "macos")]
check_posix("pipe", libc::pipe(pipe.as_mut_ptr()))?;
check_ret_errno("pipe", libc::pipe(pipe.as_mut_ptr()))?;

// Assume all non-Mac have pipe2; we can refine this on user feedback.
#[cfg(all(unix, not(target_os = "macos")))]
check_posix("pipe", libc::pipe2(pipe.as_mut_ptr(), libc::O_CLOEXEC))?;
check_ret_errno("pipe", libc::pipe2(pipe.as_mut_ptr(), libc::O_CLOEXEC))?;

Ok(pipe)
}
Expand Down Expand Up @@ -180,7 +189,7 @@ pub fn run_command(cmdline: &str, mut output_cb: impl FnMut(&[u8])) -> anyhow::R
std::ptr::null(),
];

check_posix(
check_posix_spawn(
"posix_spawn",
libc::posix_spawn(
&mut pid,
Expand All @@ -194,7 +203,7 @@ pub fn run_command(cmdline: &str, mut output_cb: impl FnMut(&[u8])) -> anyhow::R
),
)?;

check_posix("close", libc::close(pipe[1]))?;
check_ret_errno("close", libc::close(pipe[1]))?;

(pid, std::fs::File::from_raw_fd(pipe[0]))
};
Expand All @@ -211,7 +220,7 @@ pub fn run_command(cmdline: &str, mut output_cb: impl FnMut(&[u8])) -> anyhow::R

let status = unsafe {
let mut status: i32 = 0;
check_posix("waitpid", libc::waitpid(pid, &mut status, 0))?;
check_ret_errno("waitpid", libc::waitpid(pid, &mut status, 0))?;
std::process::ExitStatus::from_raw(status)
};

Expand Down
Loading