Skip to content

Commit

Permalink
[temp] switch 'check_pipe' to runtime option
Browse files Browse the repository at this point in the history
  • Loading branch information
belovdv committed Mar 23, 2023
1 parent 65bf01a commit c78d26d
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 19 deletions.
3 changes: 0 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ edition = "2018"
[target.'cfg(unix)'.dependencies]
libc = "0.2.50"

[features]
do_not_check_pipe = []

[dev-dependencies]
futures = "0.1"
num_cpus = "1.0"
Expand Down
14 changes: 9 additions & 5 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,8 +250,8 @@ impl Client {
/// with `CLOEXEC` so they're not automatically inherited by spawned
/// children.
///
/// There is a feature to configure, will this function check if provided
/// files are actually pipes.
/// On unix if `unix_check_is_pipe` enabled this function will check if
/// provided files are actually pipes.
///
/// # Safety
///
Expand All @@ -269,7 +269,7 @@ impl Client {
///
/// Note, though, that on Windows it should be safe to call this function
/// any number of times.
pub unsafe fn from_env_ext() -> Result<Client, ErrFromEnv> {
pub unsafe fn from_env_ext(check_pipe: bool) -> Result<Client, ErrFromEnv> {
let (env, var) = ["CARGO_MAKEFLAGS", "MAKEFLAGS", "MFLAGS"]
.iter()
.map(|&env| env::var(env).map(|var| (env, var)))
Expand All @@ -283,7 +283,11 @@ impl Client {
.ok_or(ErrFromEnv::IsNotConfigured)?;

let s = var[pos + arg.len()..].split(' ').next().unwrap();
match imp::Client::open(s) {
#[cfg(unix)]
let imp_client = imp::Client::open(s, check_pipe);
#[cfg(not(unix))]
let imp_client = imp::Client::open(s);
match imp_client {
Ok(c) => Ok(Client { inner: Arc::new(c) }),
Err(err) => Err(ErrFromEnv::PlatformSpecific { err, env, var }),
}
Expand All @@ -294,7 +298,7 @@ impl Client {
///
/// Wraps `from_env_ext` and discards error details.
pub unsafe fn from_env() -> Option<Client> {
Self::from_env_ext().ok()
Self::from_env_ext(false).ok()
}

/// Acquires a token from this jobserver client.
Expand Down
19 changes: 8 additions & 11 deletions src/unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,11 @@ impl Client {
Ok(Client::from_fds(pipes[0], pipes[1]))
}

pub unsafe fn open(s: &str) -> io::Result<Client> {
pub unsafe fn open(s: &str, check_pipe: bool) -> io::Result<Client> {
if let Some(client) = Self::from_fifo(s)? {
return Ok(client);
}
if let Some(client) = Self::from_pipe(s)? {
if let Some(client) = Self::from_pipe(s, check_pipe)? {
return Ok(client);
}
Err(io::Error::new(
Expand All @@ -111,7 +111,7 @@ impl Client {
}

/// `--jobserver-auth=R,W`
unsafe fn from_pipe(s: &str) -> io::Result<Option<Client>> {
unsafe fn from_pipe(s: &str, check_pipe: bool) -> io::Result<Option<Client>> {
let mut parts = s.splitn(2, ',');
let read = parts.next().unwrap();
let write = match parts.next() {
Expand All @@ -133,8 +133,8 @@ impl Client {
// If we're called from `make` *without* the leading + on our rule
// then we'll have `MAKEFLAGS` env vars but won't actually have
// access to the file descriptors.
check_fd(read)?;
check_fd(write)?;
check_fd(read, check_pipe)?;
check_fd(write, check_pipe)?;
drop(set_cloexec(read, true));
drop(set_cloexec(write, true));
Ok(Some(Client::from_fds(read, write)))
Expand Down Expand Up @@ -386,9 +386,8 @@ impl Helper {
}
}

fn check_fd(fd: c_int) -> io::Result<()> {
#[cfg(not(feature = "do_not_check_pipe"))]
unsafe {
unsafe fn check_fd(fd: c_int, check_pipe: bool) -> io::Result<()> {
if check_pipe {
let mut stat = mem::zeroed();
if libc::fstat(fd, &mut stat) == -1 {
Err(io::Error::last_os_error())
Expand All @@ -404,9 +403,7 @@ fn check_fd(fd: c_int) -> io::Result<()> {
}
Err(io::Error::last_os_error()) //
}
}
#[cfg(feature = "do_not_check_pipe")]
unsafe {
} else {
match libc::fcntl(fd, libc::F_GETFD) {
r if r == -1 => Err(io::Error::new(
io::ErrorKind::InvalidData,
Expand Down

0 comments on commit c78d26d

Please sign in to comment.