Re: fts: optimize by using fstatat instead of lstat

[Philip Guenther]

On Wed, 19 Nov 2014, enh wrote:
> On Tue, Nov 18, 2014 at 10:55 PM, Philip Guenther <guent...@gmail.com> wrote:
> > enh, this look as good in the perf measurement cited in that googlesource
> > link?
> 
> yes. performance seems roughly identical. thanks!

Committed, which leads to the next diff: use O_CLOEXEC on all internal 
fds, O_DIRECTORY when opening a directory other than ".", and drop the 3rd 
argument to open() as unnecessary when O_CREAT isn't used.

(I think doug@ wondered about using O_CLOEXEC before but I demurred then 
because these fds _are_ accessible to applications via members in the 
visible header file and we had lower-hanging fruit to pick then, but 
seeing that FreeBSD has done this, maybe it's safe?

(I've pinged sthen@ for a ports src scan...)

Philip Guenther


Index: gen/fts.c
===================================================================
RCS file: /cvs/src/lib/libc/gen/fts.c,v
retrieving revision 1.48
diff -u -p -r1.48 fts.c
--- gen/fts.c   20 Nov 2014 04:14:15 -0000      1.48
+++ gen/fts.c   20 Nov 2014 04:18:31 -0000
@@ -159,7 +159,8 @@ fts_open(char * const *argv, int options
         * and ".." are all fairly nasty problems.  Note, if we can't get the
         * descriptor we run anyway, just more slowly.
         */
-       if (!ISSET(FTS_NOCHDIR) && (sp->fts_rfd = open(".", O_RDONLY, 0)) < 0)
+       if (!ISSET(FTS_NOCHDIR) &&
+           (sp->fts_rfd = open(".", O_RDONLY | O_CLOEXEC)) < 0)
                SET(FTS_NOCHDIR);
 
        if (nitems == 0)
@@ -284,7 +285,8 @@ fts_read(FTS *sp)
            (p->fts_info == FTS_SL || p->fts_info == FTS_SLNONE)) {
                p->fts_info = fts_stat(sp, p, 1, -1);
                if (p->fts_info == FTS_D && !ISSET(FTS_NOCHDIR)) {
-                       if ((p->fts_symfd = open(".", O_RDONLY, 0)) < 0) {
+                       if ((p->fts_symfd =
+                           open(".", O_RDONLY | O_CLOEXEC)) < 0) {
                                p->fts_errno = errno;
                                p->fts_info = FTS_ERR;
                        } else
@@ -374,7 +376,7 @@ next:       tmp = p;
                        p->fts_info = fts_stat(sp, p, 1, -1);
                        if (p->fts_info == FTS_D && !ISSET(FTS_NOCHDIR)) {
                                if ((p->fts_symfd =
-                                   open(".", O_RDONLY, 0)) < 0) {
+                                   open(".", O_RDONLY | O_CLOEXEC)) < 0) {
                                        p->fts_errno = errno;
                                        p->fts_info = FTS_ERR;
                                } else
@@ -513,7 +515,7 @@ fts_children(FTS *sp, int instr)
            ISSET(FTS_NOCHDIR))
                return (sp->fts_child = fts_build(sp, instr));
 
-       if ((fd = open(".", O_RDONLY, 0)) < 0)
+       if ((fd = open(".", O_RDONLY | O_CLOEXEC)) < 0)
                return (NULL);
        sp->fts_child = fts_build(sp, instr);
        if (fchdir(fd)) {
@@ -1026,7 +1028,7 @@ fts_safe_changedir(FTS *sp, FTSENT *p, i
        newfd = fd;
        if (ISSET(FTS_NOCHDIR))
                return (0);
-       if (fd < 0 && (newfd = open(path, O_RDONLY, 0)) < 0)
+       if (fd < 0 && (newfd = open(path, O_RDONLY|O_DIRECTORY|O_CLOEXEC)) < 0)
                return (-1);
        if (fstat(newfd, &sb)) {
                ret = -1;