Skip to content

isatty is not required to set errno #467

Closed
@jgallagher

Description

@jgallagher

Hi! We noticed that the nightly version of cargo is panicking inside rustix on illumos:

$ RUST_BACKTRACE=1 cargo +nightly --version | cat
thread 'main' panicked at 'unexpected error from isatty: 0', /cargo/registry/src/github.com-1ecc6299db9ec823/rustix-0.36.3/src/backend/libc/termios/syscalls.rs:141:20
stack backtrace:
   0: rust_begin_unwind
             at /rustc/e0098a5cc3a87d857e597af824d0ce1ed1ad85e0/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/e0098a5cc3a87d857e597af824d0ce1ed1ad85e0/library/core/src/panicking.rs:65:14
   2: rustix::backend::termios::syscalls::isatty
   3: <cargo::core::shell::Shell>::new
   4: <cargo::util::config::Config>::default
   5: <cargo::cli::LazyConfig>::get_mut
   6: cargo::cli::main
   7: cargo::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Digging in a bit, we found that the issue is that illumos currently chooses not to set errno in isatty(), but rustix expects errno to be set to ENOTTY if isatty() returns 0, and panics otherwise. I think illumos's behavior here is not ideal (and we're also filing a bug there), but is technically allowed by the wording on the POSIX manpage (emphasis mine):

The isatty() function shall return 1 if fildes is associated with a terminal; otherwise, it shall return 0 and may set errno to indicate the error.

Would you be opposed to changing the implementation of isatty() to only look at the return value and not errno?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions