From patchwork Tue Apr 27 03:09:37 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?=C3=89rico_Nogueira?= X-Patchwork-Id: 43160 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 8C0A8383583D; Tue, 27 Apr 2021 03:10:13 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 8C0A8383583D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1619493013; bh=Pzhgoi3Prls3KXEG4LCjHkE1+wtoXnRoIcnlcZjIELU=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=Raho/eZVviOTjLSfxDW2c6w9XC6z0bYg+qcifazzWova9dA0WS2Cu3nT5q7WEAcXR yIZ3shsmcus+iPd8KxLVAPpfvcd3EcULdvaYXj+XrezIo/ZEdkqHtSqlbwmHQdtxm6 1bNI9W1EY9Xi1EzZJWnz3NCvInPwvM00mOO5j/XI= X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from knopi.disroot.org (knopi.disroot.org [178.21.23.139]) by sourceware.org (Postfix) with ESMTPS id 866213857810 for ; Tue, 27 Apr 2021 03:10:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 866213857810 Received: from localhost (localhost [127.0.0.1]) by disroot.org (Postfix) with ESMTP id 7057C53A86; Tue, 27 Apr 2021 05:10:08 +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 2keiCVu9zQgD; Tue, 27 Apr 2021 05:10:07 +0200 (CEST) To: libc-alpha@sourceware.org Subject: [RFC] linux: use __fd_to_filename helper function instead of snprintf. Date: Tue, 27 Apr 2021 00:09:37 -0300 Message-Id: <20210427030937.10380-1-ericonr@disroot.org> Mime-Version: 1.0 X-Spam-Status: No, score=-13.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org 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: , X-Patchwork-Original-From: =?utf-8?q?=C3=89rico_Nogueira_via_Libc-alpha?= From: =?utf-8?q?=C3=89rico_Nogueira?= Reply-To: =?utf-8?q?=C3=89rico_Nogueira?= Cc: =?utf-8?q?=C3=89rico_Nogueira?= Errors-To: libc-alpha-bounces@sourceware.org Sender: "Libc-alpha" snprintf() is a very big hammer and pulls in a lot of code that might be unnecessary. It was also being used in an inconsistent manner between the affected files. Change made to fchmodat and fexecve. There are tests using xasprintf instead of this helper as well, but this commit doesn't touch them. --- I found these call sites using `grep -R 'printf.*proc/self/fd'`, but there are probably other instances that weren't as simple. support/support_descriptors.c: char *path = xasprintf ("/proc/self/fd/%ld", fd); sysdeps/unix/sysv/linux/tst-ttyname.c: char *linkname = xasprintf ("/proc/self/fd/%d", slave); sysdeps/unix/sysv/linux/tst-ttyname.c: char *linkname = xasprintf ("/proc/self/fd/%d", slave); sysdeps/unix/sysv/linux/tst-ttyname.c: char *linkname = xasprintf ("/proc/self/fd/%d", slave); sysdeps/unix/sysv/linux/tst-memfd_create.c: char *fd_path = xasprintf ("/proc/self/fd/%d", fd); io/tst-open-tmpfile.c: char *proc_fd_path = xasprintf ("/proc/self/fd/%d", fd); io/tst-open-tmpfile.c: char *proc_fd_path = xasprintf ("/proc/self/fd/%d", fd); Some of the tests can be moved to use __fd_to_filename, but I held off on doing this to hear feedback on the idea first. It seems not all files follow the same include order / grouping policy, so I'd appreciate pointers on that as well. sysdeps/unix/sysv/linux/fchmodat.c | 11 +++-------- sysdeps/unix/sysv/linux/fexecve.c | 5 +++-- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/sysdeps/unix/sysv/linux/fchmodat.c b/sysdeps/unix/sysv/linux/fchmodat.c index f264f0c09d..4154192cc4 100644 --- a/sysdeps/unix/sysv/linux/fchmodat.c +++ b/sysdeps/unix/sysv/linux/fchmodat.c @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -69,14 +70,8 @@ fchmodat (int fd, const char *file, mode_t mode, int flag) /* 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; - } + struct fd_to_filename filename; + char *buf = __fd_to_filename(pathfd, &filename); int ret = __chmod (buf, mode); if (ret != 0) diff --git a/sysdeps/unix/sysv/linux/fexecve.c b/sysdeps/unix/sysv/linux/fexecve.c index f37c245396..218337a3b5 100644 --- a/sysdeps/unix/sysv/linux/fexecve.c +++ b/sysdeps/unix/sysv/linux/fexecve.c @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -50,8 +51,8 @@ fexecve (int fd, char *const argv[], char *const envp[]) #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); + struct fd_to_filename filename; + char *buf = fd_to_filename(fd, &filename); /* We do not need the return value. */ __execve (buf, argv, envp);