Fix incomplete override of QIODevice::open in QProcess and QLocalSocket

The rule for a new override is that it must still work if the old
implementation is called. The catch is that any class that derives from
QProcess and isn't recompiled will still have QIODevice::open in its
virtual table. That is equivalent to overriding open() and calling
QIODevice::open() (like the tests).

In Qt 5.0, QProcess::start() called QIODevice::open directly, not the
virtual open(), so there's no expectation that a user-overridden open()
be called. With that in mind, simply fix QProcess::start to not call the
virtual open at all.

Similarly with QLocalSocket, the calls to open were always non-virtual.

Task-number: QTBUG-32284
Change-Id: I88925f0ba08bc23c849658b54582744997e69a4c
Reviewed-by: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>

src/corelib/io/qprocess.cpp

@@ -1959,7 +1959,7 @@ void QProcess::start(const QString &program, const QStringList &arguments, OpenM
     d->program = program;
     d->arguments = arguments;
 
-    open(mode);
+    d->start(mode);
 }
 
 /*!
@@ -1975,7 +1975,17 @@ void QProcess::start(const QString &program, const QStringList &arguments, OpenM
  */
 void QProcess::start(OpenMode mode)
 {
-    open(mode);
+    Q_D(QProcess);
+    if (d->processState != NotRunning) {
+        qWarning("QProcess::start: Process is already running");
+        return;
+    }
+    if (d->program.isEmpty()) {
+        qWarning("QProcess::start: program not set");
+        return;
+    }
+
+    d->start(mode);
 }
 
 /*!
@@ -2008,34 +2018,39 @@ bool QProcess::open(OpenMode mode)
         return false;
     }
 
+    d->start(mode);
+    return true;
+}
+
+void QProcessPrivate::start(QIODevice::OpenMode mode)
+{
+    Q_Q(QProcess);
 #if defined QPROCESS_DEBUG
     qDebug() << "QProcess::start(" << program << ',' << arguments << ',' << mode << ')';
 #endif
 
-    d->outputReadBuffer.clear();
-    d->errorReadBuffer.clear();
+    outputReadBuffer.clear();
+    errorReadBuffer.clear();
 
-    if (d->stdinChannel.type != QProcessPrivate::Channel::Normal)
-        mode &= ~WriteOnly;     // not open for writing
-    if (d->stdoutChannel.type != QProcessPrivate::Channel::Normal &&
-        (d->stderrChannel.type != QProcessPrivate::Channel::Normal ||
-         d->processChannelMode == MergedChannels))
-        mode &= ~ReadOnly;      // not open for reading
+    if (stdinChannel.type != QProcessPrivate::Channel::Normal)
+        mode &= ~QIODevice::WriteOnly;     // not open for writing
+    if (stdoutChannel.type != QProcessPrivate::Channel::Normal &&
+        (stderrChannel.type != QProcessPrivate::Channel::Normal ||
+         processChannelMode == QProcess::MergedChannels))
+        mode &= ~QIODevice::ReadOnly;      // not open for reading
     if (mode == 0)
-        mode = Unbuffered;
-    QIODevice::open(mode);
+        mode = QIODevice::Unbuffered;
+    q->QIODevice::open(mode);
 
-    d->stdinChannel.closed = false;
-    d->stdoutChannel.closed = false;
-    d->stderrChannel.closed = false;
+    stdinChannel.closed = false;
+    stdoutChannel.closed = false;
+    stderrChannel.closed = false;
 
-    d->exitCode = 0;
-    d->exitStatus = NormalExit;
-    d->processError = QProcess::UnknownError;
-    d->errorString.clear();
-    d->startProcess();
-
-    return true;
+    exitCode = 0;
+    exitStatus = QProcess::NormalExit;
+    processError = QProcess::UnknownError;
+    errorString.clear();
+    startProcess();
 }
 
 

src/corelib/io/qprocess_p.h

@@ -346,6 +346,7 @@ public:
     QWinEventNotifier *processFinishedNotifier;
 #endif
 
+    void start(QIODevice::OpenMode mode);
     void startProcess();
 #if defined(Q_OS_UNIX) && !defined(Q_OS_QNX)
     void execChild(const char *workingDirectory, char **path, char **argv, char **envp);

src/network/socket/qlocalsocket.cpp

@@ -70,6 +70,22 @@ QT_BEGIN_NAMESPACE
     \sa QLocalServer
 */
 
+/*!
+    \fn void QLocalSocket::connectToServer(OpenMode openMode)
+    \since 5.1
+
+    Attempts to make a connection to serverName().
+    setServerName() must be called before you open the connection.
+    Alternatively you can use connectToServer(const QString &name, OpenMode openMode);
+
+    The socket is opened in the given \a openMode and first enters ConnectingState.
+    If a connection is established, QLocalSocket enters ConnectedState and emits connected().
+
+    After calling this function, the socket can emit error() to signal that an error occurred.
+
+    \sa state(), serverName(), waitForConnected()
+*/
+
 /*!
     \fn void QLocalSocket::open(OpenMode openMode)
 
@@ -352,23 +368,10 @@ QLocalSocket::~QLocalSocket()
 #endif
 }
 
-/*!
-    \since 5.1
-
-    Attempts to make a connection to serverName().
-    setServerName() must be called before you open the connection.
-    Alternatively you can use connectToServer(const QString &name, OpenMode openMode);
-
-    The socket is opened in the given \a openMode and first enters ConnectingState.
-    If a connection is established, QLocalSocket enters ConnectedState and emits connected().
-
-    After calling this function, the socket can emit error() to signal that an error occurred.
-
-    \sa state(), serverName(), waitForConnected()
-*/
-void QLocalSocket::connectToServer(OpenMode openMode)
+bool QLocalSocket::open(OpenMode openMode)
 {
-    open(openMode);
+    connectToServer(openMode);
+    return isOpen();
 }
 
 /*! \overload
@@ -385,7 +388,7 @@ void QLocalSocket::connectToServer(OpenMode openMode)
 void QLocalSocket::connectToServer(const QString &name, OpenMode openMode)
 {
     setServerName(name);
-    open(openMode);
+    connectToServer(openMode);
 }
 
 /*!

src/network/socket/qlocalsocket_tcp.cpp

@@ -214,13 +214,13 @@ void QLocalSocketPrivate::errorOccurred(QLocalSocket::LocalSocketError error, co
         q->emit stateChanged(state);
 }
 
-bool QLocalSocket::open(OpenMode openMode)
+void QLocalSocket::connectToServer(OpenMode openMode)
 {
     Q_D(QLocalSocket);
     if (state() == ConnectedState || state() == ConnectingState) {
         setErrorString(tr("Trying to connect while connection is in progress"));
         emit error(QLocalSocket::OperationError);
-        return false;
+        return;
     }
 
     d->errorString.clear();
@@ -230,7 +230,7 @@ bool QLocalSocket::open(OpenMode openMode)
     if (d->serverName.isEmpty()) {
         d->errorOccurred(ServerNotFoundError,
                          QLatin1String("QLocalSocket::connectToServer"));
-        return false;
+        return;
     }
 
     const QLatin1String prefix("QLocalServer/");
@@ -245,11 +245,10 @@ bool QLocalSocket::open(OpenMode openMode)
     if (!ok) {
         d->errorOccurred(ServerNotFoundError,
                          QLatin1String("QLocalSocket::connectToServer"));
-        return false;
+        return;
     }
     d->tcpSocket->connectToHost(QHostAddress::LocalHost, port, openMode);
     QIODevice::open(openMode);
-    return true;
 }
 
 bool QLocalSocket::setSocketDescriptor(qintptr socketDescriptor,

src/network/socket/qlocalsocket_unix.cpp

@@ -221,14 +221,14 @@ void QLocalSocketPrivate::errorOccurred(QLocalSocket::LocalSocketError error, co
         q->emit stateChanged(state);
 }
 
-bool QLocalSocket::open(OpenMode openMode)
+void QLocalSocket::connectToServer(OpenMode openMode)
 {
     Q_D(QLocalSocket);
     if (state() == ConnectedState || state() == ConnectingState) {
         QString errorString = d->generateErrorString(QLocalSocket::OperationError, QLatin1String("QLocalSocket::connectToserver"));
         setErrorString(errorString);
         emit error(QLocalSocket::OperationError);
-        return false;
+        return;
     }
 
     d->errorString.clear();
@@ -239,14 +239,14 @@ bool QLocalSocket::open(OpenMode openMode)
     if (d->serverName.isEmpty()) {
         d->errorOccurred(ServerNotFoundError,
                 QLatin1String("QLocalSocket::connectToServer"));
-        return false;
+        return;
     }
 
     // create the socket
     if (-1 == (d->connectingSocket = qt_safe_socket(PF_UNIX, SOCK_STREAM, 0))) {
         d->errorOccurred(UnsupportedSocketOperationError,
                         QLatin1String("QLocalSocket::connectToServer"));
-        return false;
+        return;
     }
     // set non blocking so we can try to connect and it won't wait
     int flags = fcntl(d->connectingSocket, F_GETFL, 0);
@@ -254,14 +254,14 @@ bool QLocalSocket::open(OpenMode openMode)
         || -1 == (fcntl(d->connectingSocket, F_SETFL, flags | O_NONBLOCK))) {
         d->errorOccurred(UnknownSocketError,
                 QLatin1String("QLocalSocket::connectToServer"));
-        return false;
+        return;
     }
 
     // _q_connectToSocket does the actual connecting
     d->connectingName = d->serverName;
     d->connectingOpenMode = openMode;
     d->_q_connectToSocket();
-    return true;
+    return;
 }
 
 /*!

src/network/socket/qlocalsocket_win.cpp

@@ -128,13 +128,13 @@ void QLocalSocketPrivate::destroyPipeHandles()
     }
 }
 
-bool QLocalSocket::open(OpenMode openMode)
+void QLocalSocket::connectToServer(OpenMode openMode)
 {
     Q_D(QLocalSocket);
     if (state() == ConnectedState || state() == ConnectingState) {
         setErrorString(tr("Trying to connect while connection is in progress"));
         emit error(QLocalSocket::OperationError);
-        return false;
+        return;
     }
 
     d->error = QLocalSocket::UnknownSocketError;
@@ -147,7 +147,7 @@ bool QLocalSocket::open(OpenMode openMode)
         d->state = UnconnectedState;
         emit error(d->error);
         emit stateChanged(d->state);
-        return false;
+        return;
     }
 
     QString pipePath = QLatin1String("\\\\.\\pipe\\");
@@ -184,7 +184,7 @@ bool QLocalSocket::open(OpenMode openMode)
     if (localSocket == INVALID_HANDLE_VALUE) {
         d->setErrorString(QLatin1String("QLocalSocket::connectToServer"));
         d->fullServerName = QString();
-        return false;
+        return;
     }
 
     // we have a valid handle
@@ -192,7 +192,6 @@ bool QLocalSocket::open(OpenMode openMode)
         d->handle = localSocket;
         emit connected();
     }
-    return true;
 }
 
 // This is reading from the buffer

tests/auto/corelib/io/qprocess/tst_qprocess.cpp

@@ -82,6 +82,7 @@ private slots:
     void constructing();
     void simpleStart();
     void startWithOpen();
+    void startWithOldOpen();
     void execute();
     void startDetached();
     void crashTest();
@@ -296,6 +297,25 @@ void tst_QProcess::startWithOpen()
     QVERIFY(p.waitForFinished(5000));
 }
 
+//-----------------------------------------------------------------------------
+void tst_QProcess::startWithOldOpen()
+{
+    // similar to the above, but we start with start() actually
+    // while open() is overridden to call QIODevice::open().
+    // This tests the BC requirement that "it works with the old implementation"
+    class OverriddenOpen : public QProcess
+    {
+    public:
+        virtual bool open(OpenMode mode) Q_DECL_OVERRIDE
+        { return QIODevice::open(mode); }
+    };
+
+    OverriddenOpen p;
+    p.start("testProcessNormal/testProcessNormal");
+    QVERIFY(p.waitForStarted(5000));
+    QVERIFY(p.waitForFinished(5000));
+}
+
 //-----------------------------------------------------------------------------
 void tst_QProcess::execute()
 {

tests/auto/network/socket/qlocalsocket/tst_qlocalsocket.cpp

@@ -79,6 +79,7 @@ private slots:
     void listenAndConnect();
 
     void connectWithOpen();
+    void connectWithOldOpen();
 
     void sendData_data();
     void sendData();
@@ -475,6 +476,33 @@ void tst_QLocalSocket::connectWithOpen()
     server.close();
 }
 
+void tst_QLocalSocket::connectWithOldOpen()
+{
+    class OverriddenOpen : public LocalSocket
+    {
+    public:
+        virtual bool open(OpenMode mode) Q_DECL_OVERRIDE
+        { return QIODevice::open(mode); }
+    };
+
+    LocalServer server;
+    QCOMPARE(server.listen("tst_qlocalsocket"), true);
+
+    OverriddenOpen socket;
+    socket.connectToServer("tst_qlocalsocket");
+
+    bool timedOut = true;
+    QVERIFY(server.waitForNewConnection(3000, &timedOut));
+
+#if defined(QT_LOCALSOCKET_TCP)
+    QTest::qWait(250);
+#endif
+    QVERIFY(!timedOut);
+
+    socket.close();
+    server.close();
+}
+
 void tst_QLocalSocket::sendData_data()
 {
     listenAndConnect_data();