Re: --create-testdir eats CPU and does nothing

[Tim Rühsen]

On Sonntag, 6. August 2017 07:09:23 CEST Dmitry Selyutin wrote:
> Hi everyone!
> 
> I'm amazed that someone still remembers this project. Looking at its
> sources, I see that there is room for improvement, but still the overall
> project structure may still be integrated into gnulib. I would be glad to
> continue working on the project in my free time, though my Python skills
> are a bit rusty. As for parallelization, I think it may be a good idea to
> use Python 3 since it now has some form of Go-like coroutines IIRC.
> 
> Do you think it's a good idea to revive the old project? It seems that it
> may still be useful.

It sounds like you have been pretty far with that project.

I would love to see it working with the current gnulib.
Please let me know when it is ready to be tested.

Regards, Tim

signature.asc
Description: This is a digitally signed message part.

[PATCH 2/2] dirent-safer: fix cloexec race

[Paul Eggert]

* lib/opendir-safer.c: Include fcntl.h instead of unistd-safer.h.
(opendir_safer): Use F_DUPFD_CLOEXEC.
* modules/dirent-safer (Depends-on): Add fcntl.  Remove unistd-safer.
* tests/test-dirent-safer.c: Do not include unistd-safer.h,
as it is no longer a prerequisite.  Use F_DUPFD_CLOEXEC
instead of dup_safer.
---
 ChangeLog | 8 
 lib/opendir-safer.c   | 4 ++--
 modules/dirent-safer  | 2 +-
 tests/test-dirent-safer.c | 4 +---
 4 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c725f5d91..514b503e5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
 2017-08-12  Paul Eggert  
 
+   dirent-safer: fix cloexec race
+   * lib/opendir-safer.c: Include fcntl.h instead of unistd-safer.h.
+   (opendir_safer): Use F_DUPFD_CLOEXEC.
+   * modules/dirent-safer (Depends-on): Add fcntl.  Remove unistd-safer.
+   * tests/test-dirent-safer.c: Do not include unistd-safer.h,
+   as it is no longer a prerequisite.  Use F_DUPFD_CLOEXEC
+   instead of dup_safer.
+
fts: fix cloexec races
* lib/fts.c [!_LIBC]: Do not include dirent--.h, unistd--.h, cloexec.h.
(opendirat, diropen): Use O_CLOEXEC instead of set_cloexec_flag.
diff --git a/lib/opendir-safer.c b/lib/opendir-safer.c
index b05ff549c..5f1f49e32 100644
--- a/lib/opendir-safer.c
+++ b/lib/opendir-safer.c
@@ -22,8 +22,8 @@
 #include "dirent-safer.h"
 
 #include 
+#include 
 #include 
-#include "unistd-safer.h"
 
 /* Like opendir, but do not clobber stdin, stdout, or stderr.  */
 
@@ -49,7 +49,7 @@ opendir_safer (char const *name)
   DIR *newdp;
   int e;
 #if HAVE_FDOPENDIR || GNULIB_FDOPENDIR
-  int f = dup_safer (fd);
+  int f = fcntl (fd, F_DUPFD_CLOEXEC, STDERR_FILENO + 1);
   if (f < 0)
 {
   e = errno;
diff --git a/modules/dirent-safer b/modules/dirent-safer
index b38c58e64..4ea4a2561 100644
--- a/modules/dirent-safer
+++ b/modules/dirent-safer
@@ -11,8 +11,8 @@ Depends-on:
 dirent
 closedir
 dirfd
+fcntl
 opendir
-unistd-safer
 
 configure.ac:
 gl_DIRENT_SAFER
diff --git a/tests/test-dirent-safer.c b/tests/test-dirent-safer.c
index 2f8c3cde2..8c4e4e358 100644
--- a/tests/test-dirent-safer.c
+++ b/tests/test-dirent-safer.c
@@ -25,8 +25,6 @@
 #include 
 #include 
 
-#include "unistd-safer.h"
-
 /* This test intentionally closes stderr.  So, we arrange to have fd 10
(outside the range of interesting fd's during the test) set up to
duplicate the original stderr.  */
@@ -75,7 +73,7 @@ main (void)
 
 #if HAVE_FDOPENDIR || GNULIB_TEST_FDOPENDIR
   {
-int fd = dup_safer (dfd);
+int fd = fcntl (dfd, F_DUPFD_CLOEXEC, STDERR_FILENO + 1);
 ASSERT (STDERR_FILENO < fd);
 dp = fdopendir (fd);
 ASSERT (dp);
-- 
2.13.4

[PATCH 1/2] fts: fix cloexec races

[Paul Eggert]

* lib/fts.c [!_LIBC]: Do not include dirent--.h, unistd--.h, cloexec.h.
(opendirat, diropen): Use O_CLOEXEC instead of set_cloexec_flag.
(fts_build): Use F_DUPD_CLOEXEC rinstad of set_cloexec_flag.
(fd_ring_check): Set cloexec flag on new file descriptors.
(fts_build, fd_ring_check): While we’re at it, make sure the
resulting file descriptor is not 0, 1, or 2, since that is easy.
* modules/fts (Depends-on): Remove cloexec, dirent-safer, dup,
fcntl-safer, unistd-safer.  Add fcntl.
---
 ChangeLog   | 10 ++
 lib/fts.c   | 27 ---
 modules/fts |  6 +-
 3 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4b475437d..c725f5d91 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@
+2017-08-12  Paul Eggert  
+
+   fts: fix cloexec races
+   * lib/fts.c [!_LIBC]: Do not include dirent--.h, unistd--.h, cloexec.h.
+   (opendirat, diropen): Use O_CLOEXEC instead of set_cloexec_flag.
+   (fts_build): Use F_DUPD_CLOEXEC rinstad of set_cloexec_flag.
+   (fd_ring_check): Set cloexec flag on new file descriptors.
+   (fts_build, fd_ring_check): While we’re at it, make sure the
+   resulting file descriptor is not 0, 1, or 2, since that is easy.
+
 2017-08-11  Bruno Haible  
 
fts tests: Fix link error.
diff --git a/lib/fts.c b/lib/fts.c
index bf8edf821..ffa41130a 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -71,11 +71,6 @@ static char sccsid[] = "@(#)fts.c   8.6 (Berkeley) 
8/14/94";
 
 #if ! _LIBC
 # include "fcntl--.h"
-# include "dirent--.h"
-# include "unistd--.h"
-/* FIXME - use fcntl(F_DUPFD_CLOEXEC)/openat(O_CLOEXEC) once they are
-   supported.  */
-# include "cloexec.h"
 # include "flexmember.h"
 # include "openat.h"
 # include "same-inode.h"
@@ -305,14 +300,13 @@ static DIR *
 internal_function
 opendirat (int fd, char const *dir, int extra_flags, int *pdir_fd)
 {
-  int new_fd = openat (fd, dir,
-   (O_RDONLY | O_DIRECTORY | O_NOCTTY | O_NONBLOCK
-| extra_flags));
+  int open_flags = (O_RDONLY | O_CLOEXEC | O_DIRECTORY | O_NOCTTY
+| O_NONBLOCK | extra_flags);
+  int new_fd = openat (fd, dir, open_flags);
   DIR *dirp;
 
   if (new_fd < 0)
 return NULL;
-  set_cloexec_flag (new_fd, true);
   dirp = fdopendir (new_fd);
   if (dirp)
 *pdir_fd = new_fd;
@@ -375,15 +369,13 @@ static int
 internal_function
 diropen (FTS const *sp, char const *dir)
 {
-  int open_flags = (O_SEARCH | O_DIRECTORY | O_NOCTTY | O_NONBLOCK
+  int open_flags = (O_SEARCH | O_CLOEXEC | O_DIRECTORY | O_NOCTTY | O_NONBLOCK
 | (ISSET (FTS_PHYSICAL) ? O_NOFOLLOW : 0)
 | (ISSET (FTS_NOATIME) ? O_NOATIME : 0));
 
   int fd = (ISSET (FTS_CWDFD)
 ? openat (sp->fts_cwd_fd, dir, open_flags)
 : open (dir, open_flags));
-  if (0 <= fd)
-set_cloexec_flag (fd, true);
   return fd;
 }
 
@@ -1435,11 +1427,7 @@ fts_build (register FTS *sp, int type)
 if (descend || type == BREAD)
   {
 if (ISSET(FTS_CWDFD))
-  {
-dir_fd = dup (dir_fd);
-if (0 <= dir_fd)
-  set_cloexec_flag (dir_fd, true);
-  }
+  dir_fd = fcntl (dir_fd, F_DUPFD_CLOEXEC, STDERR_FILENO + 1);
 if (dir_fd < 0 || fts_safe_changedir(sp, cur, dir_fd, NULL)) {
 if (descend && type == BREAD)
 cur->fts_errno = errno;
@@ -1781,7 +1769,7 @@ fd_ring_check (FTS const *sp)
   I_ring fd_w = sp->fts_fd_ring;
 
   int cwd_fd = sp->fts_cwd_fd;
-  cwd_fd = dup (cwd_fd);
+  cwd_fd = fcntl (cwd_fd, F_DUPFD_CLOEXEC, STDERR_FILENO + 1);
   char *dot = getcwdat (cwd_fd, NULL, 0);
   error (0, 0, "= check = cwd: %s", dot);
   free (dot);
@@ -1790,7 +1778,8 @@ fd_ring_check (FTS const *sp)
   int fd = i_ring_pop (_w);
   if (0 <= fd)
 {
-  int parent_fd = openat (cwd_fd, "..", O_SEARCH | O_NOATIME);
+  int open_flags = O_SEARCH | O_CLOEXEC | O_NOATIME;
+  int parent_fd = openat (cwd_fd, "..", open_flags);
   if (parent_fd < 0)
 {
   // Warn?
diff --git a/modules/fts b/modules/fts
index 55c09e7c9..ecbca70b5 100644
--- a/modules/fts
+++ b/modules/fts
@@ -8,16 +8,13 @@ lib/fts-cycle.c
 m4/fts.m4
 
 Depends-on:
-cloexec
 closedir
 cycle-check
 d-ino
 d-type
-dirent-safer
-dup
 fchdir
+fcntl
 fcntl-h
-fcntl-safer
 fdopendir
 flexmember
 fstat
@@ -32,7 +29,6 @@ readdir
 stdalign
 stdbool
 stddef
-unistd-safer
 
 configure.ac:
 gl_FUNC_FTS
-- 
2.13.4