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 A9B709E4E9 for ; Mon, 27 Nov 2023 14:15:36 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 907387B45 for ; Mon, 27 Nov 2023 14:15:06 +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 14:15:05 +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 9E7A744B47 for ; Mon, 27 Nov 2023 14:15:05 +0100 (CET) Date: Mon, 27 Nov 2023 14:15:03 +0100 From: Wolfgang Bumiller To: Fabian =?utf-8?Q?Gr=C3=BCnbichler?= Cc: Proxmox Backup Server development discussion Message-ID: References: <20231120081630.53770-1-m.sandoval@proxmox.com> <1701076239.pskhtz7vig.astroid@yuna.none> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1701076239.pskhtz7vig.astroid@yuna.none> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.098 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 - 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 13:15:36 -0000 On Mon, Nov 27, 2023 at 10:15:02AM +0100, Fabian Grünbichler wrote: > On November 20, 2023 9:16 am, Maximiliano Sandoval R wrote: > > Use the `std::io::IsTerminal` trait introduced in Rust 1.70. > > > > Internally it calls `libc::isatty`, see [1, 2]. Note that it switches > > the comparison from `== 1` to `!= 0` which shouldn't make a difference > > assuming that libc::isatty upholds the promises made in its man page. > > > > The MSRV was set on the workspace to reflect this change. > > > > [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 > > > > Signed-off-by: Maximiliano Sandoval R > > --- > > Differences from v1: > > - Rebased > > - Set MSRV to 1.70 > > > > > > Cargo.toml | 1 + > > proxmox-sys/src/linux/tty.rs | 20 +++++++------------- > > 2 files changed, 8 insertions(+), 13 deletions(-) > > > > diff --git a/Cargo.toml b/Cargo.toml > > index d4d532b..b89d9d7 100644 > > --- a/Cargo.toml > > +++ b/Cargo.toml > > @@ -42,6 +42,7 @@ license = "AGPL-3" > > repository = "https://git.proxmox.com/?p=proxmox.git" > > homepage = "https://proxmox.com" > > exclude = [ "debian" ] > > +rust-version = "1.70" > > > > [workspace.dependencies] > > # any features enabled here are enabled on all members using 'workspace = 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}; > > > > @@ -25,20 +25,14 @@ pub fn stdout_terminal_size() -> (usize, usize) { > > (winsize.ws_row as usize, winsize.ws_col as usize) > > } > > > > -/// 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()) == 1 } > > + std::io::stdout().is_terminal() > > } > > > > -/// 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()) == 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.. Is this actually the case code-wise, though? Because generally speaking this could just be to check whether we should use table-output or colors etc (though we don't have colors... but anyway, we might just want to know what stdout is and then still use println!() ...)? But I don't mind deprecating them on the grounds of them being quite trivial now.