From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/9468 Path: news.gmane.org!not-for-mail From: Rich Felker Newsgroups: gmane.linux.lib.musl.general Subject: Re: [RFC PATCH] micro-optimize __procfdname Date: Sat, 5 Mar 2016 00:24:59 -0500 Message-ID: <20160305052459.GD9349@brightrain.aerifal.cx> References: Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Trace: ger.gmane.org 1457155516 16499 80.91.229.3 (5 Mar 2016 05:25:16 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Sat, 5 Mar 2016 05:25:16 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-9481-gllmg-musl=m.gmane.org@lists.openwall.com Sat Mar 05 06:25:16 2016 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1ac4iJ-00063A-1o for gllmg-musl@m.gmane.org; Sat, 05 Mar 2016 06:25:15 +0100 Original-Received: (qmail 17873 invoked by uid 550); 5 Mar 2016 05:25:12 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Original-Received: (qmail 17850 invoked from network); 5 Mar 2016 05:25:12 -0000 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Original-Sender: Rich Felker Xref: news.gmane.org gmane.linux.lib.musl.general:9468 Archived-At: On Sun, Feb 21, 2016 at 02:41:21PM +0300, Alexander Monakov wrote: > Hello, > > I've noticed that internal function __procfdname can be slightly cleaned up by > filling the supplied buffer right-to-left and returning the last filled > position. The patch below implements what I have in mind, and changes one > call site to demonstrate (I'll be happy to submit a patch that converts all > calls, if overall this change is desirable). I really doubt this makes any major improvement, but it might help size a bit and it might be cleaner/more readable, so it's interesting. > diff --git a/src/internal/procfdname.c b/src/internal/procfdname.c > index 697e0bd..cfb3f90 100644 > --- a/src/internal/procfdname.c > +++ b/src/internal/procfdname.c > @@ -1,13 +1,9 @@ > -void __procfdname(char *buf, unsigned fd) > +char *__procfdname_impl(char *buf, unsigned fd) > { > - unsigned i, j; > - for (i=0; (buf[i] = "/proc/self/fd/"[i]); i++); > - if (!fd) { > - buf[i] = '0'; > - buf[i+1] = 0; > - return; > - } > - for (j=fd; j; j/=10, i++); > - buf[i] = 0; > - for (; fd; fd/=10) buf[--i] = '0' + fd%10; > + *buf = 0; > + do *--buf = '0' + fd % 10; > + while (fd /= 10); > + for (int i = 13; i >= 0; i--) > + *--buf = "/proc/self/fd/"[i]; > + return buf; > } > diff --git a/src/internal/procfdname.h b/src/internal/procfdname.h > index e69de29..6d3c6e2 100644 > --- a/src/internal/procfdname.h > +++ b/src/internal/procfdname.h > @@ -0,0 +1,9 @@ > +#ifndef PROCFDNAME_H > +#define PROCFDNAME_H > + > +char *__procfdname_impl(char *, unsigned); > + > +#define procfdbufsize sizeof "/proc/self/fd/0123456789" + (3 * (sizeof(int)-4)) What is the motivation behind changing the size expression to use the "012...9" part? It's nonobvious to me. > +#define procfdname(buf, fd) __procfdname_impl(buf + procfdbufsize - 1, fd) I suppose the idea of putting the offset to the end in a macro in the header rather than in the callee is both optimization and allowing the compiler to detect out-of-bounds pointer arithmetic? > + > +#endif > diff --git a/src/process/fexecve.c b/src/process/fexecve.c > index 6507b42..88e6b9d 100644 > --- a/src/process/fexecve.c > +++ b/src/process/fexecve.c > @@ -1,13 +1,11 @@ > #include > #include > - > -void __procfdname(char *, unsigned); > +#include "procfdname.h" > > int fexecve(int fd, char *const argv[], char *const envp[]) > { > - char buf[15 + 3*sizeof(int)]; > - __procfdname(buf, fd); > - execve(buf, argv, envp); > + char buf[procfdbufsize]; > + execve(procfdname(buf, fd), argv, envp); > if (errno == ENOENT) errno = EBADF; > return -1; > } Here using the return value directly is nice but at some other call points might we need to introduce a pointer variable to store the pointer returned? I haven't checked yet. Rich