From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS17314 8.43.84.0/22 X-Spam-Status: No, score=-4.2 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 5E3801F5AE for ; Mon, 3 May 2021 20:37:36 +0000 (UTC) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id EA9653854804; Mon, 3 May 2021 20:37:34 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org EA9653854804 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1620074255; bh=aIyxS1j/hUcW4DUfHe5weW3IbmcMoN1wRhIBoP1kaZQ=; h=Subject:To:Date:In-Reply-To:List-Id:List-Unsubscribe:List-Archive: List-Post:List-Help:List-Subscribe:From:Reply-To:From; b=G+6TxWDYNLTfkwGPwILwF8Y1jPbB2PxTdPW+xQ/Lzd2rhya3aOzi9HKnaIE+HyAW5 P2QcLH3aFuZvfj52yiv63MNjR56LfM4sob+VhjwM6+0EmfWgevxp58EvltCN16NAPq 1/G+qidmD2TUtXJOGXhVkxzHQoKUJrRfAAOQReGI= Received: from knopi.disroot.org (knopi.disroot.org [178.21.23.139]) by sourceware.org (Postfix) with ESMTPS id ECCDB385703C for ; Mon, 3 May 2021 20:37:30 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org ECCDB385703C Received: from localhost (localhost [127.0.0.1]) by disroot.org (Postfix) with ESMTP id A2CC8539A8; Mon, 3 May 2021 22:37:28 +0200 (CEST) X-Virus-Scanned: Debian amavisd-new at disroot.org Received: from knopi.disroot.org ([127.0.0.1]) by localhost (disroot.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id I83PD0ZaRPW5; Mon, 3 May 2021 22:37:27 +0200 (CEST) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Subject: Re: [RFC v2] linux: use __fd_to_filename helper function instead of snprintf. To: "Adhemerval Zanella" , Date: Mon, 03 May 2021 17:26:01 -0300 Message-Id: In-Reply-To: X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: =?utf-8?q?=C3=89rico_Nogueira_via_Libc-alpha?= Reply-To: =?utf-8?q?=C3=89rico_Nogueira?= Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" On Mon May 3, 2021 at 4:26 PM -03, Adhemerval Zanella wrote: > > > On 27/04/2021 10:09, =C3=89rico Nogueira via Libc-alpha wrote: > > Change made to fchmodat and fexecve. There are tests using xasprintf > > instead of this helper as well, but this commit doesn't touch them. > > To adapt the tests it would require either to build it as > internal/static > or add fd_to_filename to libsupport. I am not this is really an > improvement. That might explain some of the build failures I got when I tried changing the tests. > > LGTM, thanks. I will commit this for you. Thanks! I found a few more places where __fd_to_filename could be used, they are currently using _fitoa_word together with stpcpy. Would you be interested in making __fd_to_filename use _fitoa_word or _itoa_word? Or should it be left as is? > > Reviewed-by: Adhemerval Zanella > > > --- > >=20 > > Differences from v1: > > - actually builds > > - doesn't use an intermediary buf variable > >=20 > > sysdeps/unix/sysv/linux/fchmodat.c | 13 +++---------- > > sysdeps/unix/sysv/linux/fexecve.c | 10 ++++------ > > 2 files changed, 7 insertions(+), 16 deletions(-) > >=20 > > diff --git a/sysdeps/unix/sysv/linux/fchmodat.c b/sysdeps/unix/sysv/lin= ux/fchmodat.c > > index f264f0c09d..5bd1eb96a5 100644 > > --- a/sysdeps/unix/sysv/linux/fchmodat.c > > +++ b/sysdeps/unix/sysv/linux/fchmodat.c > > @@ -18,6 +18,7 @@ > > =20 > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -69,16 +70,8 @@ fchmodat (int fd, const char *file, mode_t mode, int= flag) > > =20 > > /* For most file systems, fchmod does not operate on O_PATH > > descriptors, so go through /proc. */ > > - char buf[32]; > > - if (__snprintf (buf, sizeof (buf), "/proc/self/fd/%d", pathfd) <= 0) > > - { > > - /* This also may report strange error codes to the caller > > - (although snprintf really should not fail). */ > > - __close_nocancel (pathfd); > > - return -1; > > - } > > - > > - int ret =3D __chmod (buf, mode); > > + struct fd_to_filename filename; > > + int ret =3D __chmod (__fd_to_filename (pathfd, &filename), mode)= ; > > if (ret !=3D 0) > > { > > if (errno =3D=3D ENOENT) > > diff --git a/sysdeps/unix/sysv/linux/fexecve.c b/sysdeps/unix/sysv/linu= x/fexecve.c > > index f37c245396..df25c2acb8 100644 > > --- a/sysdeps/unix/sysv/linux/fexecve.c > > +++ b/sysdeps/unix/sysv/linux/fexecve.c > > @@ -22,6 +22,7 @@ > > #include > > #include > > =20 > > +#include > > #include > > #include > > #include > > @@ -49,12 +50,9 @@ fexecve (int fd, char *const argv[], char *const env= p[]) > > =20 > > #ifndef __ASSUME_EXECVEAT > > /* We use the /proc filesystem to get the information. If it is not > > - mounted we fail. */ > > - char buf[sizeof "/proc/self/fd/" + sizeof (int) * 3]; > > - __snprintf (buf, sizeof (buf), "/proc/self/fd/%d", fd); > > - > > - /* We do not need the return value. */ > > - __execve (buf, argv, envp); > > + mounted we fail. We do not need the return value. */ > > + struct fd_to_filename filename; > > + __execve (__fd_to_filename (fd, &filename), argv, envp); > > =20 > > int save =3D errno; > > =20 > >=20