[PATCH 3/5] vircommand: Unify mass FD closing

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

We have two version of mass FD closing: one for FreeBSD (because
it has closefrom()) and the other for everything else. But now
that we have closefrom() wrapper even for Linux, we can unify
these two.

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
 src/util/vircommand.c | 85 +++++++++++++------------------------------
 1 file changed, 25 insertions(+), 60 deletions(-)

diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 49abb53c28..867f45b57b 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -526,60 +526,6 @@ virCommandMassCloseGetFDsGeneric(virCommand *cmd G_GNUC_UNUSED,
 }
 # endif /* !__linux__ */
 
-# ifdef __FreeBSD__
-
-static int
-virCommandMassClose(virCommand *cmd,
-                    int childin,
-                    int childout,
-                    int childerr)
-{
-    int lastfd = -1;
-    int fd = -1;
-    size_t i;
-
-    /*
-     * Two phases of closing.
-     *
-     * The first (inefficient) phase iterates over FDs,
-     * preserving certain FDs we need to pass down, and
-     * closing others. The number of iterations is bounded
-     * to the number of the biggest FD we need to preserve.
-     *
-     * The second (speedy) phase uses closefrom() to cull
-     * all remaining FDs in the process.
-     *
-     * Usually the first phase will be fairly quick only
-     * processing a handful of low FD numbers, and thus using
-     * closefrom() is a massive win for high ulimit() NFILES
-     * values.
-     */
-    lastfd = MAX(lastfd, childin);
-    lastfd = MAX(lastfd, childout);
-    lastfd = MAX(lastfd, childerr);
-
-    for (i = 0; i < cmd->npassfd; i++)
-        lastfd = MAX(lastfd, cmd->passfd[i].fd);
-
-    for (fd = 0; fd <= lastfd; fd++) {
-        if (fd == childin || fd == childout || fd == childerr)
-            continue;
-        if (!virCommandFDIsSet(cmd, fd)) {
-            int tmpfd = fd;
-            VIR_MASS_CLOSE(tmpfd);
-        } else if (virSetInherit(fd, true) < 0) {
-            virReportSystemError(errno, _("failed to preserve fd %1$d"), fd);
-            return -1;
-        }
-    }
-
-    closefrom(lastfd + 1);
-
-    return 0;
-}
-
-# else /* ! __FreeBSD__ */
-
 static int
 virCommandMassClose(virCommand *cmd,
                     int childin,
@@ -588,7 +534,9 @@ virCommandMassClose(virCommand *cmd,
 {
     g_autoptr(virBitmap) fds = NULL;
     int openmax = sysconf(_SC_OPEN_MAX);
+    int lastfd = -1;
     int fd = -1;
+    size_t i;
 
     /* In general, it is not safe to call malloc() between fork() and exec()
      * because the child might have forked at the worst possible time, i.e.
@@ -605,16 +553,23 @@ virCommandMassClose(virCommand *cmd,
 
     fds = virBitmapNew(openmax);
 
-#  ifdef __linux__
+# ifdef __linux__
     if (virCommandMassCloseGetFDsLinux(cmd, fds) < 0)
         return -1;
-#  else
+# else
     if (virCommandMassCloseGetFDsGeneric(cmd, fds) < 0)
         return -1;
-#  endif
+# endif
+
+    lastfd = MAX(lastfd, childin);
+    lastfd = MAX(lastfd, childout);
+    lastfd = MAX(lastfd, childerr);
+
+    for (i = 0; i < cmd->npassfd; i++)
+        lastfd = MAX(lastfd, cmd->passfd[i].fd);
 
     fd = virBitmapNextSetBit(fds, 2);
-    for (; fd >= 0; fd = virBitmapNextSetBit(fds, fd)) {
+    for (; fd >= 0 && fd <= lastfd; fd = virBitmapNextSetBit(fds, fd)) {
         if (fd == childin || fd == childout || fd == childerr)
             continue;
         if (!virCommandFDIsSet(cmd, fd)) {
@@ -626,11 +581,21 @@ virCommandMassClose(virCommand *cmd,
         }
     }
 
+    if (virCloseFrom(lastfd + 1) < 0) {
+        if (errno != ENOSYS)
+            return -1;
+
+        if (fd > 0) {
+            for (; fd >= 0; fd = virBitmapNextSetBit(fds, fd)) {
+                int tmpfd = fd;
+                VIR_MASS_CLOSE(tmpfd);
+            }
+        }
+    }
+
     return 0;
 }
 
-# endif /* ! __FreeBSD__ */
-
 
 /*
  * virExec:
-- 
2.41.0