Hi Urs, Christophe, I'm interested in trying to make KRDC's vnc client support automatic reconnection (using TCP keepalives). I had a quick hack and made some progress, but before trying the real thing, I need to make VncClientThread's "outputErrorMessageString" fully threadsafe.
The attached patch cleanly separates the threading/static callbacks/class members, but does not change any locking or other functionality. The commit log to my local branch reads as follows: ==== commit 545111064c06a0423585263ce9fe1779461756e4 Author: Shaheed Haque <srha...@theiet.org> Date: Sat Oct 26 17:18:47 2013 +0100 Make outputErrorMessageString and m_colorTable non-static members This *looks* intrusive, but actually all it does is: - Make outputErrorMessageString and m_colorTable into non-static member variables. - Do away with the need to reference class members using "t->". - Use the "cl" member variable rather than keep passing it around. === Please let me know what you think. Thanks, Shaheed
diff --git a/vnc/vncclientthread.cpp b/vnc/vncclientthread.cpp index 26321a4..7504919 100644 --- a/vnc/vncclientthread.cpp +++ b/vnc/vncclientthread.cpp @@ -24,13 +24,68 @@ #include "vncclientthread.h" #include <QMutexLocker> +#include <QThreadStorage> #include <QTimer> //for detecting intel AMT KVM vnc server static const QString INTEL_AMT_KVM_STRING= "Intel(r) AMT KVM"; -static QString outputErrorMessageString; +static QThreadStorage<VncClientThread *> instances; -QVector<QRgb> VncClientThread::m_colorTable; +// Dispatch from this static callback context to the member context. +rfbBool VncClientThread::newclientStatic(rfbClient *cl) +{ + VncClientThread *t = (VncClientThread *)rfbClientGetClientData(cl, 0); + Q_ASSERT(t); + + return t->newclient(); +} + +// Dispatch from this static callback context to the member context. +void VncClientThread::updatefbStatic(rfbClient *cl, int x, int y, int w, int h) +{ + VncClientThread *t = (VncClientThread *)rfbClientGetClientData(cl, 0); + Q_ASSERT(t); + + return t->updatefb(x, y, w, h); +} + +// Dispatch from this static callback context to the member context. +void VncClientThread::cuttextStatic(rfbClient *cl, const char *text, int textlen) +{ + VncClientThread *t = (VncClientThread *)rfbClientGetClientData(cl, 0); + Q_ASSERT(t); + + t->cuttext(text, textlen); +} + +// Dispatch from this static callback context to the member context. +char *VncClientThread::passwdHandlerStatic(rfbClient *cl) +{ + VncClientThread *t = (VncClientThread *)rfbClientGetClientData(cl, 0); + Q_ASSERT(t); + + return t->passwdHandler(); +} + +// Dispatch from this static callback context to the member context. +rfbCredential *VncClientThread::credentialHandlerStatic(rfbClient *cl, int credentialType) +{ + VncClientThread *t = (VncClientThread *)rfbClientGetClientData(cl, 0); + Q_ASSERT(t); + + return t->credentialHandler(credentialType); +} + +// Dispatch from this static callback context to the member context. +void VncClientThread::outputHandlerStatic(const char *format, ...) +{ + VncClientThread *t = instances.localData(); + + va_list args; + va_start(args, format); + t->outputHandler(format, args); + va_end(args); +} void VncClientThread::setClientColorDepth(rfbClient* cl, VncClientThread::ColorDepth cd) { @@ -80,27 +135,24 @@ void VncClientThread::setClientColorDepth(rfbClient* cl, VncClientThread::ColorD } } -rfbBool VncClientThread::newclient(rfbClient *cl) +rfbBool VncClientThread::newclient() { - VncClientThread *t = (VncClientThread*)rfbClientGetClientData(cl, 0); - Q_ASSERT(t); - //8bit color hack for Intel(r) AMT KVM "classic vnc" = vnc server built in in Intel Vpro chipsets. if (INTEL_AMT_KVM_STRING == cl->desktopName) { kDebug(5011) << "Intel(R) AMT KVM: switching to 8 bit color depth (workaround, recent libvncserver needed)"; - t->setColorDepth(bpp8); + setColorDepth(bpp8); } - setClientColorDepth(cl, t->colorDepth()); + setClientColorDepth(cl, colorDepth()); const int width = cl->width, height = cl->height, depth = cl->format.bitsPerPixel; const int size = width * height * (depth / 8); - if (t->frameBuffer) - delete [] t->frameBuffer; // do not leak if we get a new framebuffer size - t->frameBuffer = new uint8_t[size]; - cl->frameBuffer = t->frameBuffer; + if (frameBuffer) + delete [] frameBuffer; // do not leak if we get a new framebuffer size + frameBuffer = new uint8_t[size]; + cl->frameBuffer = frameBuffer; memset(cl->frameBuffer, '\0', size); - switch (t->quality()) { + switch (quality()) { case RemoteView::High: cl->appData.encodingsString = "copyrect zlib hextile raw"; cl->appData.compressLevel = 0; @@ -124,15 +176,13 @@ rfbBool VncClientThread::newclient(rfbClient *cl) return true; } -void VncClientThread::updatefb(rfbClient* cl, int x, int y, int w, int h) +void VncClientThread::updatefb(int x, int y, int w, int h) { // kDebug(5011) << "updated client: x: " << x << ", y: " << y << ", w: " << w << ", h: " << h; - VncClientThread *t = (VncClientThread*)rfbClientGetClientData(cl, 0); - Q_ASSERT(t); const int width = cl->width, height = cl->height; QImage img; - switch(t->colorDepth()) { + switch(colorDepth()) { case bpp8: img = QImage(cl->frameBuffer, width, height, QImage::Format_Indexed8); img.setColorTable(m_colorTable); @@ -149,62 +199,53 @@ void VncClientThread::updatefb(rfbClient* cl, int x, int y, int w, int h) kDebug(5011) << "image not loaded"; } - if (t->m_stopped) { + if (m_stopped) { return; // sending data to a stopped thread is not a good idea } - t->setImage(img); + setImage(img); - t->emitUpdated(x, y, w, h); + emitUpdated(x, y, w, h); } -void VncClientThread::cuttext(rfbClient* cl, const char *text, int textlen) +void VncClientThread::cuttext(const char *text, int textlen) { const QString cutText = QString::fromUtf8(text, textlen); kDebug(5011) << cutText; if (!cutText.isEmpty()) { - VncClientThread *t = (VncClientThread*)rfbClientGetClientData(cl, 0); - Q_ASSERT(t); - - t->emitGotCut(cutText); + emitGotCut(cutText); } } -char *VncClientThread::passwdHandler(rfbClient *cl) +char *VncClientThread::passwdHandler() { kDebug(5011) << "password request"; - VncClientThread *t = (VncClientThread*)rfbClientGetClientData(cl, 0); - Q_ASSERT(t); - - t->passwordRequest(); - t->m_passwordError = true; + passwordRequest(); + m_passwordError = true; - return strdup(t->password().toUtf8()); + return strdup(password().toUtf8()); } - -rfbCredential *VncClientThread::credentialHandler(rfbClient *cl, int credentialType) + +rfbCredential *VncClientThread::credentialHandler(int credentialType) { kDebug(5011) << "credential request" << credentialType; - VncClientThread *t = (VncClientThread*)rfbClientGetClientData(cl, 0); - Q_ASSERT(t); - rfbCredential *cred = 0; switch (credentialType) { case rfbCredentialTypeUser: - t->passwordRequest(true); - t->m_passwordError = true; + passwordRequest(true); + m_passwordError = true; cred = new rfbCredential; - cred->userCredential.username = strdup(t->username().toUtf8()); - cred->userCredential.password = strdup(t->password().toUtf8()); + cred->userCredential.username = strdup(username().toUtf8()); + cred->userCredential.password = strdup(password().toUtf8()); break; default: kError(5011) << "credential request failed, unspported credentialType:" << credentialType; - t->outputErrorMessage(i18n("VNC authentication type is not supported.")); + outputErrorMessage(i18n("VNC authentication type is not supported.")); break; } return cred; @@ -372,17 +413,17 @@ void VncClientThread::run() m_passwordError = false; locker.unlock(); - rfbClientLog = outputHandler; - rfbClientErr = outputHandler; + rfbClientLog = outputHandlerStatic; + rfbClientErr = outputHandlerStatic; //24bit color dept in 32 bits per pixel = default. Will change colordepth and bpp later if needed cl = rfbGetClient(8, 3, 4); setClientColorDepth(cl, this->colorDepth()); - cl->MallocFrameBuffer = newclient; + cl->MallocFrameBuffer = newclientStatic; cl->canHandleNewFBSize = true; - cl->GetPassword = passwdHandler; - cl->GetCredential = credentialHandler; - cl->GotFrameBufferUpdate = updatefb; - cl->GotXCutText = cuttext; + cl->GetPassword = passwdHandlerStatic; + cl->GetCredential = credentialHandlerStatic; + cl->GotFrameBufferUpdate = updatefbStatic; + cl->GotXCutText = cuttextStatic; rfbClientSetClientData(cl, 0, this); locker.relock(); diff --git a/vnc/vncclientthread.h b/vnc/vncclientthread.h index ff0a93e..fc45ad7 100644 --- a/vnc/vncclientthread.h +++ b/vnc/vncclientthread.h @@ -148,15 +148,26 @@ protected: void run(); private: - static void setClientColorDepth(rfbClient *cl, ColorDepth cd); + void setClientColorDepth(rfbClient *cl, ColorDepth cd); void setColorDepth(ColorDepth colorDepth); - //these static methods are callback functions for libvncclient - static rfbBool newclient(rfbClient *cl); - static void updatefb(rfbClient *cl, int x, int y, int w, int h); - static void cuttext(rfbClient *cl, const char *text, int textlen); - static char* passwdHandler(rfbClient *cl); - static rfbCredential* credentialHandler(rfbClient *cl, int credentialType); - static void outputHandler(const char *format, ...); + + // These static methods are callback functions for libvncclient. Each + // of them calls back into the corresponding member function via some + // TLS-based logic. + static rfbBool newclientStatic(rfbClient *cl); + static void updatefbStatic(rfbClient *cl, int x, int y, int w, int h); + static void cuttextStatic(rfbClient *cl, const char *text, int textlen); + static char *passwdHandlerStatic(rfbClient *cl); + static rfbCredential *credentialHandlerStatic(rfbClient *cl, int credentialType); + static void outputHandlerStatic(const char *format, ...); + + // Member functions corresponding to the above static methods. + rfbBool newclient(); + void updatefb(int x, int y, int w, int h); + void cuttext(const char *text, int textlen); + char *passwdHandler(); + rfbCredential *credentialHandler(int credentialType); + void outputHandler(const char *format, ...); QImage m_image; rfbClient *cl; @@ -169,7 +180,8 @@ private: ColorDepth m_colorDepth; QQueue<ClientEvent* > m_eventQueue; //color table for 8bit indexed colors - static QVector<QRgb> m_colorTable; + QVector<QRgb> m_colorTable; + QString outputErrorMessageString; volatile bool m_stopped; volatile bool m_passwordError;
>> Visit http://mail.kde.org/mailman/listinfo/kde-devel#unsub to unsubscribe <<