From kde-commits Tue Mar 08 00:17:27 2016 From: Weng Xuetian Date: Tue, 08 Mar 2016 00:17:27 +0000 To: kde-commits Subject: [kinit] src/klauncher: Clean up and refactor the xcb port of klauncher. Message-Id: X-MARC-Message: https://marc.info/?l=kde-commits&m=145739625600662 Git commit d154b4c0d85c1616c512cabdf5bf9919a9a9191d by Weng Xuetian. Committed on 08/03/2016 at 00:15. Pushed by xuetianweng into branch 'master'. Clean up and refactor the xcb port of klauncher. Currently it is possible that connection is free'd twice, and cached connection is not correctly reused in some cases. Try to move all the code with cached connection in the same place and clean up the duplicate logic that spreads in difference places. REVIEW: 127166 M +44 -78 src/klauncher/klauncher.cpp M +10 -1 src/klauncher/klauncher.h http://commits.kde.org/kinit/d154b4c0d85c1616c512cabdf5bf9919a9a9191d diff --git a/src/klauncher/klauncher.cpp b/src/klauncher/klauncher.cpp index 7ea9da9..9a2c962 100644 --- a/src/klauncher/klauncher.cpp +++ b/src/klauncher/klauncher.cpp @@ -116,10 +116,6 @@ KLauncher::KLauncher() #if HAVE_X11 mIsX11 =3D QGuiApplication::platformName() =3D=3D QStringLiteral("xcb"= ); #endif -#if HAVE_XCB - mCached.conn =3D Q_NULLPTR; - mCached.screen =3D 0; -#endif Q_ASSERT(g_klauncher_self =3D=3D NULL); g_klauncher_self =3D this; = @@ -188,11 +184,9 @@ KLauncher::~KLauncher() void KLauncher::close() { #if HAVE_XCB - if (mCached.conn) { + if (mCached) { xcb_disconnect(mCached.conn); - mCached.conn =3D Q_NULLPTR; - mCached.displayName =3D QByteArray(); - mCached.screen =3D 0; + mCached =3D XCBConnection(); } #endif } @@ -509,31 +503,11 @@ KLauncher::requestDone(KLaunchRequest *request) = #if HAVE_XCB if (!request->startup_dpy.isEmpty() && mIsX11) { - xcb_connection_t *conn =3D Q_NULLPTR; - int screen =3D 0; - QByteArray displayName; - if (mCached.conn !=3D Q_NULLPTR && request->startup_dpy !=3D m= Cached.displayName) { - conn =3D mCached.conn; - screen =3D mCached.screen; - displayName =3D mCached.displayName; - } - if (!conn) { - conn =3D xcb_connect(request->startup_dpy.constData(), &sc= reen); - } + XCBConnection conn =3D getXCBConnection(request->startup_dpy); if (conn) { - if (!xcb_connection_has_error(conn)) { - KStartupInfoId id; - id.initId(request->startup_id); - KStartupInfo::sendFinishXcb(conn, screen, id); - if (mCached.conn !=3D conn && mCached.conn !=3D NULL) { - xcb_disconnect(mCached.conn); - } - mCached.conn =3D conn; - mCached.screen =3D screen; - mCached.displayName =3D displayName; - } else { - xcb_disconnect(conn); - } + KStartupInfoId id; + id.initId(request->startup_id); + KStartupInfo::sendFinishXcb(conn.conn, conn.screen, id); } } #endif @@ -850,27 +824,15 @@ KLauncher::send_service_startup_info(KLaunchRequest *= request, KService::Ptr serv dpy_str =3D env.mid(8).toLocal8Bit(); } } - xcb_connection_t *conn =3D Q_NULLPTR; - int screen =3D 0; - QByteArray displayName; - if (!dpy_str.isEmpty() && mCached.conn !=3D Q_NULLPTR && dpy_str !=3D = mCached.displayName) { - conn =3D mCached.conn; - screen =3D mCached.screen; - displayName =3D mCached.displayName; - } - if (!conn) { - conn =3D xcb_connect(dpy_str.constData(), &screen); - } + + XCBConnection conn =3D getXCBConnection(dpy_str); request->startup_id =3D id.id(); - if (conn =3D=3D Q_NULLPTR || xcb_connection_has_error(conn)) { + if (!conn) { cancel_service_startup_info(request, startup_id, envs); - if (conn) { - xcb_disconnect(conn); - } return; } = - request->startup_dpy =3D dpy_str; + request->startup_dpy =3D conn.displayName; = KStartupInfoData data; data.setName(service->name()); @@ -884,13 +846,7 @@ KLauncher::send_service_startup_info(KLaunchRequest *r= equest, KService::Ptr serv } data.setApplicationId(service->entryPath()); // the rest will be sent by kdeinit - KStartupInfo::sendStartupXcb(conn, screen, id, data); - if (mCached.conn !=3D conn && mCached.conn !=3D NULL) { - xcb_disconnect(conn); - } - mCached.conn =3D conn; - mCached.screen =3D screen; - mCached.displayName =3D displayName; + KStartupInfo::sendStartupXcb(conn.conn, conn.screen, id, data); return; #else return; @@ -912,34 +868,13 @@ KLauncher::cancel_service_startup_info(KLaunchRequest= *request, const QByteArray dpy_str =3D env.mid(8); } } - xcb_connection_t *conn =3D Q_NULLPTR; - int screen; - QByteArray displayName; - if (!dpy_str.isEmpty() && mCached.conn !=3D Q_NULLPTR && dpy_str.t= oLocal8Bit() !=3D mCached.displayName) { - conn =3D mCached.conn; - screen =3D mCached.screen; - displayName =3D mCached.displayName; - } + XCBConnection conn =3D getXCBConnection(dpy_str.toLocal8Bit()); if (!conn) { - displayName =3D dpy_str.toLocal8Bit(); - conn =3D xcb_connect(displayName.constData(), &screen); - } - if (conn =3D=3D Q_NULLPTR) { - return; - } - if (xcb_connection_has_error(conn)) { - xcb_disconnect(conn); return; } KStartupInfoId id; id.initId(startup_id); - KStartupInfo::sendFinishXcb(conn, screen, id); - if (mCached.conn !=3D conn && mCached.conn !=3D NULL) { - xcb_disconnect(mCached.conn); - } - mCached.conn =3D conn; - mCached.screen =3D screen; - mCached.displayName =3D displayName; + KStartupInfo::sendFinishXcb(conn.conn, conn.screen, id); } #endif } @@ -1303,3 +1238,34 @@ void KLauncher::terminate_kdeinit() #endif } = +KLauncher::XCBConnection KLauncher::getXCBConnection(const QByteArray &_di= splayName) +{ + const auto displayName =3D !_displayName.isEmpty() ? _displayName : qg= etenv("DISPLAY"); + + // If cached connection is same as request + if (mCached && displayName =3D=3D mCached.displayName) { + // Check error, if so close it, otherwise it's still valid, reuse = the cached one + if (xcb_connection_has_error(mCached.conn)) { + close(); + } else { + return mCached; + } + } + + // At this point, the cached connection can't be reused, make a new co= nnection + XCBConnection conn; + conn.conn =3D xcb_connect(displayName.constData(), &conn.screen); + if (conn) { + // check error first, if so return an empty one + if (xcb_connection_has_error(conn.conn)) { + xcb_disconnect(conn.conn); + return XCBConnection(); + } + + // if it's valid, replace mCached with new connection + conn.displayName =3D displayName; + close(); + mCached =3D conn; + } + return conn; +} diff --git a/src/klauncher/klauncher.h b/src/klauncher/klauncher.h index 31bfaaa..ac5d7ae 100644 --- a/src/klauncher/klauncher.h +++ b/src/klauncher/klauncher.h @@ -252,7 +252,13 @@ protected: bool mIsX11; #endif #if HAVE_XCB - struct { + struct XCBConnection { + XCBConnection() : conn(Q_NULLPTR), screen(0) { } + + operator bool() const { + return conn !=3D Q_NULLPTR; + } + xcb_connection_t *conn; int screen; QByteArray displayName; @@ -263,5 +269,8 @@ protected: protected Q_SLOTS: void slotGotOutput(); void slotFinished(int exitCode, QProcess::ExitStatus exitStatus); + +private: + XCBConnection getXCBConnection(const QByteArray &displyName); }; #endif