From 1fb9c5348e3f7d4dd80e5b924aaca93632c651f9 Mon Sep 17 00:00:00 2001 From: Alex Trotsenko Date: Tue, 22 Dec 2020 18:45:55 +0200 Subject: [PATCH] QProcess: consolidate channel management We have the same channel forwarding, redirecting, and merging rules for all platforms. This makes it possible to introduce the openChannels() function, which consolidates the logic and performs high-level general processing of the channels configuration properties. Change-Id: Id3574fc42a56829328369b6a1a6ec9c95fce8eca Reviewed-by: Oswald Buddenhagen --- src/corelib/io/qprocess.cpp | 72 ++++++++++++++++++++++++++++++++ src/corelib/io/qprocess_p.h | 2 + src/corelib/io/qprocess_unix.cpp | 34 +-------------- src/corelib/io/qprocess_win.cpp | 29 ++----------- 4 files changed, 80 insertions(+), 57 deletions(-) diff --git a/src/corelib/io/qprocess.cpp b/src/corelib/io/qprocess.cpp index 095847122a6..9c6dafd7ef9 100644 --- a/src/corelib/io/qprocess.cpp +++ b/src/corelib/io/qprocess.cpp @@ -940,6 +940,78 @@ void QProcessPrivate::setErrorAndEmit(QProcess::ProcessError error, const QStrin emit q->errorOccurred(processError); } +/*! + \internal +*/ +bool QProcessPrivate::openChannels() +{ + // stdin channel. + if (inputChannelMode == QProcess::ForwardedInputChannel) { + if (stdinChannel.type != Channel::Normal) + qWarning("QProcess::openChannels: Inconsistent stdin channel configuration"); + } else if (!openChannel(stdinChannel)) { + return false; + } + + // stdout channel. + if (processChannelMode == QProcess::ForwardedChannels + || processChannelMode == QProcess::ForwardedOutputChannel) { + if (stdoutChannel.type != Channel::Normal) + qWarning("QProcess::openChannels: Inconsistent stdout channel configuration"); + } else if (!openChannel(stdoutChannel)) { + return false; + } + + // stderr channel. + if (processChannelMode == QProcess::ForwardedChannels + || processChannelMode == QProcess::ForwardedErrorChannel + || processChannelMode == QProcess::MergedChannels) { + if (stderrChannel.type != Channel::Normal) + qWarning("QProcess::openChannels: Inconsistent stderr channel configuration"); + } else if (!openChannel(stderrChannel)) { + return false; + } + + return true; +} + +/*! + \internal +*/ +bool QProcessPrivate::openChannelsForDetached() +{ + // stdin channel. + if (stdinChannel.type != Channel::Normal + && (stdinChannel.type != Channel::Redirect + || inputChannelMode == QProcess::ForwardedInputChannel)) { + qWarning("QProcess::openChannelsForDetached: Inconsistent stdin channel configuration"); + } + if (stdinChannel.type == Channel::Redirect && !openChannel(stdinChannel)) + return false; + + // stdout channel. + if (stdoutChannel.type != Channel::Normal + && (stdoutChannel.type != Channel::Redirect + || processChannelMode == QProcess::ForwardedChannels + || processChannelMode == QProcess::ForwardedOutputChannel)) { + qWarning("QProcess::openChannelsForDetached: Inconsistent stdout channel configuration"); + } + if (stdoutChannel.type == Channel::Redirect && !openChannel(stdoutChannel)) + return false; + + // stderr channel. + if (processChannelMode == QProcess::MergedChannels || (stderrChannel.type != Channel::Normal + && (stderrChannel.type != Channel::Redirect + || processChannelMode == QProcess::ForwardedChannels + || processChannelMode == QProcess::ForwardedErrorChannel))) { + qWarning("QProcess::openChannelsForDetached: Inconsistent stderr channel configuration"); + } + if (stderrChannel.type == Channel::Redirect && !openChannel(stderrChannel)) + return false; + + return true; +} + /*! \internal Returns \c true if we emitted readyRead(). diff --git a/src/corelib/io/qprocess_p.h b/src/corelib/io/qprocess_p.h index 023d69e8d51..3be73f479ec 100644 --- a/src/corelib/io/qprocess_p.h +++ b/src/corelib/io/qprocess_p.h @@ -316,6 +316,8 @@ public: Channel stdinChannel; Channel stdoutChannel; Channel stderrChannel; + bool openChannels(); + bool openChannelsForDetached(); bool openChannel(Channel &channel); void closeChannel(Channel *channel); void closeWriteChannel(); diff --git a/src/corelib/io/qprocess_unix.cpp b/src/corelib/io/qprocess_unix.cpp index 07d427047d0..f12d92b2531 100644 --- a/src/corelib/io/qprocess_unix.cpp +++ b/src/corelib/io/qprocess_unix.cpp @@ -221,36 +221,11 @@ void QProcessPrivate::closeChannel(Channel *channel) /* Create the pipes to a QProcessPrivate::Channel. - - This function must be called in order: stdin, stdout, stderr */ bool QProcessPrivate::openChannel(Channel &channel) { Q_Q(QProcess); - // Handle forwarding of the process channels. - if (&channel == &stdinChannel) { - if (inputChannelMode == QProcess::ForwardedInputChannel) - return true; - } else { - switch (processChannelMode) { - case QProcess::ForwardedChannels: - return true; - case QProcess::ForwardedOutputChannel: - if (&channel == &stdoutChannel) - return true; - break; - case QProcess::ForwardedErrorChannel: - case QProcess::MergedChannels: - if (&channel == &stderrChannel) - return true; - break; - default: - break; - } - } - - // Create pipes and handle redirections. if (channel.type == Channel::Normal) { // we're piping this channel to our own process if (qt_create_pipe(channel.pipe) != 0) @@ -378,10 +353,7 @@ void QProcessPrivate::startProcess() #endif // Initialize pipes - if (!openChannel(stdinChannel) || - !openChannel(stdoutChannel) || - !openChannel(stderrChannel) || - qt_create_pipe(childStartedPipe) != 0) { + if (!openChannels() || qt_create_pipe(childStartedPipe) != 0) { setErrorAndEmit(QProcess::FailedToStart, qt_error_string(errno)); cleanup(); return; @@ -902,9 +874,7 @@ bool QProcessPrivate::startDetached(qint64 *pid) return false; } - if ((stdinChannel.type == Channel::Redirect && !openChannel(stdinChannel)) - || (stdoutChannel.type == Channel::Redirect && !openChannel(stdoutChannel)) - || (stderrChannel.type == Channel::Redirect && !openChannel(stderrChannel))) { + if (!openChannelsForDetached()) { closeChannel(&stdinChannel); closeChannel(&stdoutChannel); closeChannel(&stderrChannel); diff --git a/src/corelib/io/qprocess_win.cpp b/src/corelib/io/qprocess_win.cpp index 886c4e85876..c9223443e21 100644 --- a/src/corelib/io/qprocess_win.cpp +++ b/src/corelib/io/qprocess_win.cpp @@ -180,40 +180,23 @@ static bool qt_create_pipe(Q_PIPE *pipe, bool isInputPipe) /* Create the pipes to a QProcessPrivate::Channel. - - This function must be called in order: stdin, stdout, stderr */ bool QProcessPrivate::openChannel(Channel &channel) { Q_Q(QProcess); - if (&channel == &stderrChannel && processChannelMode == QProcess::MergedChannels) - return true; - switch (channel.type) { case Channel::Normal: { // we're piping this channel to our own process - if (&channel == &stdinChannel) { - return inputChannelMode == QProcess::ForwardedInputChannel - || qt_create_pipe(channel.pipe, true); - } + if (&channel == &stdinChannel) + return qt_create_pipe(channel.pipe, true); if (&channel == &stdoutChannel) { - if (processChannelMode == QProcess::ForwardedChannels - || processChannelMode == QProcess::ForwardedOutputChannel) { - return true; - } - if (!stdoutChannel.reader) { stdoutChannel.reader = new QWindowsPipeReader(q); q->connect(stdoutChannel.reader, SIGNAL(readyRead()), SLOT(_q_canReadStandardOutput())); } } else /* if (&channel == &stderrChannel) */ { - if (processChannelMode == QProcess::ForwardedChannels - || processChannelMode == QProcess::ForwardedErrorChannel) { - return true; - } - if (!stderrChannel.reader) { stderrChannel.reader = new QWindowsPipeReader(q); q->connect(stderrChannel.reader, SIGNAL(readyRead()), SLOT(_q_canReadStandardError())); @@ -545,9 +528,7 @@ void QProcessPrivate::startProcess() q->setProcessState(QProcess::Starting); - if (!openChannel(stdinChannel) || - !openChannel(stdoutChannel) || - !openChannel(stderrChannel)) { + if (!openChannels()) { QString errorString = QProcess::tr("Process failed to start: %1").arg(qt_error_string()); cleanup(); setErrorAndEmit(QProcess::FailedToStart, errorString); @@ -913,9 +894,7 @@ bool QProcessPrivate::startDetached(qint64 *pid) { static const DWORD errorElevationRequired = 740; - if ((stdinChannel.type == Channel::Redirect && !openChannel(stdinChannel)) - || (stdoutChannel.type == Channel::Redirect && !openChannel(stdoutChannel)) - || (stderrChannel.type == Channel::Redirect && !openChannel(stderrChannel))) { + if (!openChannelsForDetached()) { closeChannel(&stdinChannel); closeChannel(&stdoutChannel); closeChannel(&stderrChannel);