Skip to content

Commit

Permalink
QProcess::startDetached: set the error condition on failure to start
Browse files Browse the repository at this point in the history
And set *pid to -1.

[ChangeLog][QtCore][QProcess] If a startDetached() fails to start the
target application, the QProcess object should now have a proper error
string in errorString().

Pick-to: 6.1
Change-Id: Ic90d8429a0eb4837971dfffd1664e825ffcb923e
Reviewed-by: Oswald Buddenhagen <[email protected]>
  • Loading branch information
thiagomacieira committed Feb 22, 2021
1 parent 86ebdc0 commit 73a04ed
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 14 deletions.
11 changes: 6 additions & 5 deletions src/corelib/io/qprocess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2132,11 +2132,12 @@ void QProcess::startCommand(const QString &command, OpenMode mode)
temporarily freeze.
If the function is successful then *\a pid is set to the process identifier
of the started process. Note that the child process may exit and the PID
may become invalid without notice. Furthermore, after the child process
exits, the same PID may be recycled and used by a completely different
process. User code should be careful when using this variable, especially
if one intends to forcibly terminate the process by operating system means.
of the started process; otherwise, it's set to -1. Note that the child
process may exit and the PID may become invalid without notice.
Furthermore, after the child process exits, the same PID may be recycled
and used by a completely different process. User code should be careful
when using this variable, especially if one intends to forcibly terminate
the process by operating system means.
Only the following property setters are supported by startDetached():
\list
Expand Down
28 changes: 19 additions & 9 deletions src/corelib/io/qprocess_unix.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
/****************************************************************************
**
** Copyright (C) 2016 The Qt Company Ltd.
** Copyright (C) 2016 Intel Corporation.
** Copyright (C) 2020 The Qt Company Ltd.
** Copyright (C) 2021 Intel Corporation.
** Copyright (C) 2021 Alex Trotsenko.
** Contact: https://www.qt.io/licensing/
**
** This file is part of the QtCore module of the Qt Toolkit.
Expand Down Expand Up @@ -883,17 +884,21 @@ bool QProcessPrivate::startDetached(qint64 *pid)

// To catch the startup of the child
int startedPipe[2];
if (qt_safe_pipe(startedPipe) != 0)
if (qt_safe_pipe(startedPipe) != 0) {
setErrorAndEmit(QProcess::FailedToStart, QLatin1String("pipe: ") + qt_error_string(errno));
return false;
}
// To communicate the pid of the child
int pidPipe[2];
if (qt_safe_pipe(pidPipe) != 0) {
setErrorAndEmit(QProcess::FailedToStart, QLatin1String("pipe: ") + qt_error_string(errno));
qt_safe_close(startedPipe[0]);
qt_safe_close(startedPipe[1]);
return false;
}

if (!openChannelsForDetached()) {
// openChannel sets the error string
closeChannel(&stdinChannel);
closeChannel(&stdoutChannel);
closeChannel(&stderrChannel);
Expand Down Expand Up @@ -981,6 +986,7 @@ bool QProcessPrivate::startDetached(qint64 *pid)
::_exit(1);
}

int savedErrno = errno;
closeChannel(&stdinChannel);
closeChannel(&stdoutChannel);
closeChannel(&stderrChannel);
Expand All @@ -990,6 +996,7 @@ bool QProcessPrivate::startDetached(qint64 *pid)
if (childPid == -1) {
qt_safe_close(startedPipe[0]);
qt_safe_close(pidPipe[0]);
setErrorAndEmit(QProcess::FailedToStart, QLatin1String("fork: ") + qt_error_string(savedErrno));
return false;
}

Expand All @@ -998,14 +1005,17 @@ bool QProcessPrivate::startDetached(qint64 *pid)
int result;
qt_safe_close(startedPipe[0]);
qt_safe_waitpid(childPid, &result, 0);

bool success = (startResult != -1 && reply == '\0');
if (success && pid) {
pid_t actualPid = 0;
if (qt_safe_read(pidPipe[0], (char *)&actualPid, sizeof(pid_t)) == sizeof(pid_t)) {
*pid = actualPid;
} else {
*pid = 0;
}
pid_t actualPid;
if (qt_safe_read(pidPipe[0], &actualPid, sizeof(pid_t)) != sizeof(pid_t))
actualPid = 0;
*pid = actualPid;
} else if (!success) {
if (pid)
*pid = -1;
setErrorAndEmit(QProcess::FailedToStart);
}
qt_safe_close(pidPipe[0]);
return success;
Expand Down
6 changes: 6 additions & 0 deletions src/corelib/io/qprocess_win.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,7 @@ bool QProcessPrivate::startDetached(qint64 *pid)
static const DWORD errorElevationRequired = 740;

if (!openChannelsForDetached()) {
// openChannel sets the error string
closeChannel(&stdinChannel);
closeChannel(&stdoutChannel);
closeChannel(&stderrChannel);
Expand Down Expand Up @@ -913,6 +914,11 @@ bool QProcessPrivate::startDetached(qint64 *pid)
success = startDetachedUacPrompt(program, arguments, nativeArguments,
workingDirectory, pid);
}
if (!success) {
if (pid)
*pid = -1;
setErrorAndEmit(QProcess::FailedToStart);
}

closeChannel(&stdinChannel);
closeChannel(&stdoutChannel);
Expand Down
2 changes: 2 additions & 0 deletions tests/auto/corelib/io/qprocess/tst_qprocess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2279,6 +2279,8 @@ void tst_QProcess::detachedSetNonExistentWorkingDirectory()
qint64 pid = -1;
QVERIFY(!process.startDetached(&pid));
QCOMPARE(pid, -1);
QCOMPARE(process.error(), QProcess::FailedToStart);
QVERIFY(process.errorString() != "Unknown error");
}

void tst_QProcess::startFinishStartFinish()
Expand Down

0 comments on commit 73a04ed

Please sign in to comment.