From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 4B3B49E1D2 for ; Mon, 27 Nov 2023 10:16:34 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 281332F92 for ; Mon, 27 Nov 2023 10:16:04 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS for ; Mon, 27 Nov 2023 10:16:02 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id A9A8C44A40 for ; Mon, 27 Nov 2023 10:16:02 +0100 (CET) Date: Mon, 27 Nov 2023 10:15:02 +0100 From: Fabian =?iso-8859-1?q?Gr=FCnbichler?= To: Proxmox Backup Server development discussion References: <20231120081630.53770-1-m.sandoval@proxmox.com> In-Reply-To: <20231120081630.53770-1-m.sandoval@proxmox.com> MIME-Version: 1.0 User-Agent: astroid/0.16.0 (https://github.com/astroidmail/astroid) Message-Id: <1701076239.pskhtz7vig.astroid@yuna.none> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-LEVEL: Spam detection results: 0 AWL 0.065 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [proxmox.com, tty.rs, rust-lang.org] Subject: Re: [pbs-devel] [PATCH proxmox v2 1/2] sys: Use safe wrapper for libc::isatty X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 27 Nov 2023 09:16:34 -0000 On November 20, 2023 9:16 am, Maximiliano Sandoval R wrote: > Use the `std::io::IsTerminal` trait introduced in Rust 1.70. >=20 > Internally it calls `libc::isatty`, see [1, 2]. Note that it switches > the comparison from `=3D=3D 1` to `!=3D 0` which shouldn't make a differe= nce > assuming that libc::isatty upholds the promises made in its man page. >=20 > The MSRV was set on the workspace to reflect this change. >=20 > [1] https://doc.rust-lang.org/src/std/io/stdio.rs.html#1079 > [2] https://doc.rust-lang.org/src/std/sys/unix/io.rs.html#79 >=20 > Signed-off-by: Maximiliano Sandoval R > --- > Differences from v1: > - Rebased > - Set MSRV to 1.70 >=20 >=20 > Cargo.toml | 1 + > proxmox-sys/src/linux/tty.rs | 20 +++++++------------- > 2 files changed, 8 insertions(+), 13 deletions(-) >=20 > diff --git a/Cargo.toml b/Cargo.toml > index d4d532b..b89d9d7 100644 > --- a/Cargo.toml > +++ b/Cargo.toml > @@ -42,6 +42,7 @@ license =3D "AGPL-3" > repository =3D "https://git.proxmox.com/?p=3Dproxmox.git" > homepage =3D "https://proxmox.com" > exclude =3D [ "debian" ] > +rust-version =3D "1.70" > =20 > [workspace.dependencies] > # any features enabled here are enabled on all members using 'workspace = =3D true'! > diff --git a/proxmox-sys/src/linux/tty.rs b/proxmox-sys/src/linux/tty.rs > index fdea162..375e0ec 100644 > --- a/proxmox-sys/src/linux/tty.rs > +++ b/proxmox-sys/src/linux/tty.rs > @@ -1,4 +1,4 @@ > -use std::io::{self, Read, Write}; > +use std::io::{self, IsTerminal, Read, Write}; > use std::mem::MaybeUninit; > use std::os::unix::io::{AsRawFd, OwnedFd}; > =20 > @@ -25,20 +25,14 @@ pub fn stdout_terminal_size() -> (usize, usize) { > (winsize.ws_row as usize, winsize.ws_col as usize) > } > =20 > -/// Returns whether the current stdout is a tty . > -/// # Safety > -/// > -/// uses unsafe call to libc::isatty > +/// Returns whether the current stdout is a tty. > pub fn stdout_isatty() -> bool { > - unsafe { libc::isatty(std::io::stdin().as_raw_fd()) =3D=3D 1 } > + std::io::stdout().is_terminal() > } > =20 > -/// Returns whether the current stdin is a tty . > -/// # Safety > -/// > -/// uses unsafe call to libc::isatty > +/// Returns whether the current stdin is a tty. > pub fn stdin_isatty() -> bool { > - unsafe { libc::isatty(std::io::stdin().as_raw_fd()) =3D=3D 1 } > + std::io::stdin().is_terminal() > } do we still need these two? I assume everything calling this would then do something with stdin/stdout anyway, and could just as well call is_terminal() on them? might be a candidate for deprecation, since the original reason for their existence (pulling out the `unsafe` part into a helper) is gone.. > =20 > pub enum TtyOutput { > @@ -75,7 +69,7 @@ impl TtyOutput { > /// Get an output file descriptor for the current terminal. > pub fn open() -> io::Result> { > let stdout =3D std::io::stdout(); > - if unsafe { libc::isatty(stdout.as_raw_fd()) } =3D=3D 1 { > + if stdout.is_terminal() { > Ok(Some(TtyOutput::Stdout(stdout))) > } else { > match crate::fd::open( > @@ -97,7 +91,7 @@ impl TtyOutput { > /// first. > pub fn read_password(query: &str) -> Result, Error> { > let input =3D std::io::stdin(); > - if unsafe { libc::isatty(input.as_raw_fd()) } !=3D 1 { > + if !input.is_terminal() { > let mut out =3D String::new(); > input.read_line(&mut out)?; > return Ok(out.into_bytes()); > --=20 > 2.39.2 >=20 >=20 >=20 > _______________________________________________ > pbs-devel mailing list > pbs-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel >=20 >=20 >=20