QProcess/Unix: enable setChildProcessModifier for startDetached

Do this by making the actual child-execution code common between
startProcess() and startDetached(). It does mean we've moved the chdir()
operation from the child to the grandchild process, though.

[ChangeLog][QtCore][QProcess] The modifier function set with
setChildProcessModifier() will now also be executed when the process is
started with startDetached().

Change-Id: Icfe44ecf285a480fafe4fffd174d9aa57dd7dfff
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>

2 files changed, 50 insertions, 46 deletions

diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp
index 669c067430..5bdfc0e208 100644
--- a/src/corelib/io/qprocess_unix.cpp
+++ b/src/corelib/io/qprocess_unix.cpp
@@ -576,18 +576,40 @@ void QProcessPrivate::startProcess()
         ::fcntl(stderrChannel.pipe[0], F_SETFL, ::fcntl(stderrChannel.pipe[0], F_GETFL) | O_NONBLOCK);
 }
 
-static bool callChildProcessModifier(const QProcessPrivate::UnixExtras *unixExtras) noexcept
+static const char *callChildProcessModifier(const QProcessPrivate::UnixExtras *unixExtras) noexcept
 {
     QT_TRY {
         if (unixExtras->childProcessModifier)
             unixExtras->childProcessModifier();
     } QT_CATCH (...) {
         errno = FakeErrnoForThrow;
-        return false;
+        return "throw";
     }
-    return true;
+    return nullptr;
+}
+
+// this function doesn't return if the execution succeeds
+static const char *doExecChild(char **argv, char **envp, int workingDirFd,
+                               const QProcessPrivate::UnixExtras *unixExtras) noexcept
+{
+    // enter the working directory
+    if (workingDirFd != -1 && fchdir(workingDirFd) == -1)
+        return "fchdir";
+
+    if (unixExtras) {
+        if (const char *what = callChildProcessModifier(unixExtras))
+            return what;
+    }
+
+    // execute the process
+    if (!envp)
+        qt_safe_execv(argv[0], argv);
+    else
+        qt_safe_execve(argv[0], argv, envp);
+    return "execve";
 }
 
+
 // IMPORTANT:
 //
 // This function is called in a vfork() context on some OSes (notably, Linux
@@ -605,36 +627,12 @@ void QProcessPrivate::execChild(int workingDir, char **argv, char **envp) const
     // make sure this fd is closed if execv() succeeds
     qt_safe_close(childStartedPipe[0]);
 
-    // enter the working directory
-    if (workingDir != -1 && fchdir(workingDir) == -1) {
-        // failed, stop the process
-        strcpy(error.function, "fchdir");
-        goto report_errno;
-    }
-
-    if (unixExtras) {
-        if (!callChildProcessModifier(unixExtras.get())) {
-            std::strcpy(error.function, "throw");
-            goto report_errno;
-        }
-    }
-
-    // execute the process
-    if (!envp) {
-        qt_safe_execv(argv[0], argv);
-        strcpy(error.function, "execvp");
-    } else {
-#if defined (QPROCESS_DEBUG)
-        fprintf(stderr, "QProcessPrivate::execChild() starting %s\n", argv[0]);
-#endif
-        qt_safe_execve(argv[0], argv, envp);
-        strcpy(error.function, "execve");
-    }
+    const char *what = doExecChild(argv, envp, workingDir, unixExtras.get());
+    strcpy(error.function, what);
 
     // notify failure
     // don't use strerror or any other routines that may allocate memory, since
     // some buggy libc versions can deadlock on locked mutexes.
-report_errno:
     error.code = errno;
     qt_safe_write(childStartedPipe[1], &error, sizeof(error));
 }
@@ -1038,20 +1036,13 @@ bool QProcessPrivate::startDetached(qint64 *pid)
             ::_exit(1);
         };
 
-        if (workingDirFd != -1 && fchdir(workingDirFd) == -1)
-            reportFailed("fchdir: ");
-
         pid_t doubleForkPid = fork();
         if (doubleForkPid == 0) {
             // Render channels configuration.
             commitChannels();
 
-            if (envp.pointers)
-                qt_safe_execve(argv.pointers[0], argv.pointers.get(), envp.pointers.get());
-            else
-                qt_safe_execv(argv.pointers[0], argv.pointers.get());
-
-            reportFailed("execv: ");
+            reportFailed(doExecChild(argv.pointers.get(), envp.pointers.get(), workingDirFd,
+                                     unixExtras.get()));
         } else if (doubleForkPid == -1) {
             reportFailed("fork: ");
         }
diff --git a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
index f51721e966..285126b826 100644
--- a/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
+++ b/tests/auto/corelib/io/qprocess/tst_qprocess.cpp
@@ -111,6 +111,7 @@ private slots:
     void createProcessArgumentsModifier();
 #endif // Q_OS_WIN
 #if defined(Q_OS_UNIX)
+    void setChildProcessModifier_data();
     void setChildProcessModifier();
     void throwInChildProcessModifier();
 #endif
@@ -1446,8 +1447,16 @@ static void childProcessModifier(int fd)
     QT_CLOSE(fd);
 }
 
+void tst_QProcess::setChildProcessModifier_data()
+{
+    QTest::addColumn<bool>("detached");
+    QTest::newRow("normal") << false;
+    QTest::newRow("detached") << true;
+}
+
 void tst_QProcess::setChildProcessModifier()
 {
+    QFETCH(bool, detached);
     int pipes[2] = { -1 , -1 };
     QVERIFY(qt_safe_pipe(pipes) == 0);
 
@@ -1455,20 +1464,24 @@ void tst_QProcess::setChildProcessModifier()
     process.setChildProcessModifier([pipes]() {
         ::childProcessModifier(pipes[1]);
     });
-    process.start("testProcessNormal/testProcessNormal");
-    if (process.state() != QProcess::Starting)
-        QCOMPARE(process.state(), QProcess::Running);
-    QVERIFY2(process.waitForStarted(5000), qPrintable(process.errorString()));
+    process.setProgram("testProcessNormal/testProcessNormal");
+    if (detached) {
+        process.startDetached();
+    } else {
+        process.start("testProcessNormal/testProcessNormal");
+        if (process.state() != QProcess::Starting)
+            QCOMPARE(process.state(), QProcess::Running);
+        QVERIFY2(process.waitForStarted(5000), qPrintable(process.errorString()));
+        QVERIFY2(process.waitForFinished(5000), qPrintable(process.errorString()));
+        QCOMPARE(process.exitStatus(), QProcess::NormalExit);
+        QCOMPARE(process.exitCode(), 0);
+    }
 
     char buf[sizeof messageFromChildProcess] = {};
     qt_safe_close(pipes[1]);
     QCOMPARE(qt_safe_read(pipes[0], buf, sizeof(buf)), qint64(sizeof(messageFromChildProcess)) - 1);
     QCOMPARE(buf, messageFromChildProcess);
     qt_safe_close(pipes[0]);
-
-    QVERIFY2(process.waitForFinished(5000), qPrintable(process.errorString()));
-    QCOMPARE(process.exitStatus(), QProcess::NormalExit);
-    QCOMPARE(process.exitCode(), 0);
 }
 
 void tst_QProcess::throwInChildProcessModifier()