CDSMPlugin::LoadPlugin() fAllowMulti hack breakage

[Duff]

So -- if fAllowMulti is passed to LoadPlugin() in DSMPlugin.cpp, it attempts to copy the DSM to a new file in the same directory before opening it (for what reason I'm not sure). This causes plugin loading to fail for users who don't have write access to this directory -- a Bad Thing, as this flag is used by vncviewer, thus prohibiting unprivileged users from engaging in encrypted VNC client sessions.

If this temporary file does indeed need to be created, it should be made in a temporary directory, not in the same location as the DSM. Any chance of this getting fixed?

There's been more discussion of this on the mailing list. Thanks!

[UltraSam]

Well.

This plugin dll copy is a dirty hack to allow several viewers to be run on the same machine using the same DSM plugin...
I've done this because I had some design problem with DSMPLugin concurrent access.
But it works...

I can't make a copy in another directory because the plugin must be in the viewer's directory.
Sorry...

I need to rework on the multi-threading part of the DSMPlugin architecture to definitly fix this problem that also prevents allowing multiple viewers+DSM connections on server side.

[Duff]

I can't make a copy in another directory because the plugin must be in the viewer's directory.

There's no way around that? I don't know much about dynamic loading on Windows, but using dlopen() on UNIX, one can provide an explicit full path to be used. Poking around on MSDN, it looks like one can use SetDllDirectory to add a new directory to the DLL search path, or (if one prefers to retain compatibility with older versions of Windows) simply manipulate the PATH environment variable. Further, you have the option of calling LoadLibraryEx with LOAD_WITH_ALTERED_SEARCH_PATH and a lpFileName specifying the temporary directory you wish to load the DSM from, thus taking advantage of the alternate DLL search order.

Even if none of that is practical: Exactly what is the issue that copying the DSM to a new file is intended to fix? It might be worth parameterizing the workaround's use if the bug that it fixes isn't as bad as the resultant "can't-use-DSMs-at-all" issue that unprivileged users end up with. (If, for instance, the issue is that only one viewer instance can have a DSM open at a given time... well, one is better than none).

[Duff]

Even if none of that is practical: Exactly what is the issue that copying the DSM to a new file is intended to fix? It might be worth parameterizing the workaround's use if the bug that it fixes isn't as bad as the resultant "can't-use-DSMs-at-all" issue that unprivileged users end up with. (If, for instance, the issue is that only one viewer instance can have a DSM open at a given time... well, one is better than none).

Erk -- pardon my failure to read carefully earlier. (Not to say that I'm not curious about why the preexisting code wouldn't allow multiple viewers to use the same DSM simultaniously -- is it something like Windows not relocating dynamically loaded libraries?).

[UltraSam]

No. It's due to the fact that 2 viewers of the same vncviewer processus (exe) can't use the same DSMplugin dll instance at the same time beacuse of a "bad" design in my DSM architecture workflow...
That's why we need a DSMPlugin dll "file" for each viewer of the running vncviewer.
But of course it works with only one viewer using the plugin as no dll needs to be copied in this case.

When 2 different vncviewers are run on the same machine, there's no problem because each one loads the plugin dll in it's own adress space.

The problem is mainly due to the fact that the "Restore" part of transformed packets is done in 2 steps, 2 calls to the same function of the plugin with different parameters. That's the main mistake but it was hard to do better at the time I implemented the DSM stuff. It is not possible (to my knowledge and understanding) to surround these 2 functions calls with a mutex, as it results in a mutual viewers deadlock.
With no mutex, the 2 viewers mix their packets during "restore" operations causing connections drops, of course.
But I may be completely wrong in my interpretation of the problem. I have to dive into the code and analyse all this again to find a better solution.

[Duff]

But of course it works with only one viewer using the plugin as no dll needs to be copied in this case.

Erm -- unless I have a viewer running I don't know about (and I'm pretty sure I don't), it *doesn't* in fact work with only one viewer, as it attempts the copy anyhow in this situation.

The problem is mainly due to the fact that the "Restore" part of transformed packets is done in 2 steps, 2 calls to the same function of the plugin with different parameters. That's the main mistake but it was hard to do better at the time I implemented the DSM stuff. It is not possible (to my knowledge and understanding) to surround these 2 functions calls with a mutex, as it results in a mutual viewers deadlock.

To me, this sounds like the sort of problem that would be resolved by requiring that the plugins be reenterant. Looking at the RC4 DSM, reentrant it certainly isn't -- there's lots and lots of variables outside the PLUGINSTRUCT, and even so pPlugin is a global... arrgh!

Looking at the plugin API, though, it doesn't look like there's much choice -- the API, as currently defined, doesn't provide an opaque void* which the plugin can use as a handle to store all its state.

So, then, one approach to solving this properly:

• Add an opaque handle to the CDSMPlugin class.
• Pass a pointer to that handle to all plugin methods, such that the plugin's STARTUP method can initialize it (malloc a space for its state and set said pointer to point to it) and the other methods can access it.
• Rewrite all the plugins to store their state behind that pointer (arrrgh!)
• Be happy.

[UltraSam]

Yep.

The solution is to store "in" the plugin a different context (maily transform and restore buffers pointers and datalen values and maybe encryption context depending on the algo used) for each viewer in a vncviewer process that is using it.

Easy to do, actually, but not without breaking backward compatibility at the API level, as at least one more param needs to be passed to the functions (like caller process ID or something or pointer to a context buffer as you suggest).
-> new UltraVNC binaries necessary as well as ported plugins.

Will try to add all this the the TestPlugin sample if I have time.


The other quick fix would be to be able to do several LoadLibrary of the same dll (same name and file) in the same process space, so we wouldn't have to do a renamed copy of the dll... anyone knows if it's possible ?

[scovel]

Sam,

I'd rather take the time to fix it "right" rather than finding a work-around with LoadLibrary. I doubt that will work anyway.

If you get a chance to work on the API I can work on the sample plugin, or the old XOR plugin I used to use for testing. That way we can see if everything is working before I invest the time to fix the bigger plugins.

Sean

[Duff]

Easy to do, actually, but not without breaking backward compatibility at the API level, as at least one more param needs to be passed to the functions (like caller process ID or something or pointer to a context buffer as you suggest).

Just to clarify: The pointer is a void**, initially null, which can be pointed at a struct or a buffer or whatever else the DSM's startup function cares to create (so long as the DSM's shutdown function destroys it and sets the pointer back to null -- if it isn't set back to NULL on shutdown then it's fair game for a free()). This is (as I remember it) the approach taken by (among others) glib and gtk when handling plugins or callbacks.

Passing in the caller's process ID isn't adequate for reentrancy, because there needs to be somewhere (a list of objects?) where the actual data associated with those processes is stored, and if the data is stored inside a global list maintained by the DSM keyed by PID... well, you've made race conditions *less likely*, but you haven't eliminated them: The list itself is still shared (even if the items in it are process-ID specific) and you can run into race conditions while new PID-based items are being added to or removed from the list unless you put a mutex around list access... and by that point you've added way too much unnecessarily complexity.

If my other question (about free, or ideally Free, development tools usable to build UltraVNC) gets answered, I wouldn't particularly mind building a patch implementing such an API myself. (scovel, should I do this, would the source to your XOR plugin be available?)

[scovel]

The XOR plugin used to be on my website. I can get you the source.

Its basically the "test" plugin with an XOR. The test plugin just copies the input to the output. The XOR plugin adds the step of XORing the buffer first so you can tell if its actually doing anything. Stupid but effective! Its NOT effective encryption though, so I removed it from my website so nobody would try to use it in a production system.

Sean

[Duff]

A patch demonstrating the style of API I prefer is available at:

http://www.ecst.csuchico.edu/~cduffy/Ul ... tect.patch

It's not complete: I don't own Visual Studio, and my patch to reimplement Borland support doesn't extend to DSMs (and, for that matter, I haven't really tested it enough to know whether bugs encountered are on account of issues w/ it or newer changes); and further, the API needs to be extended to have a call for retrieving the plugin's API version. (If it doesn't have that call, it's obviously using the Version 1 plugin API; that way, backwards compatibility or at least user notification of incompatible plugins can be implemented).

It compiles (no promises about running) with both GCC and the Borland compiler, and includes some changes made to support one or the other. Hope it's helpful to someone!

[Duff]

As mentioned elsewhere -- I've acquired part-time access to a copy of MS VC++, and so have been able to get the miscellanious patches I threw together earlier to where they *actually work*. The corner cases still aren't tested carefully, but at least it works.

[syntax="c"]Index: DSMPlugin/DSMPlugin.cpp
===================================================================
RCS file: /cvsroot/ultravnc/ultravnc/DSMPlugin/DSMPlugin.cpp,v
retrieving revision 1.8
diff -u -r1.8 DSMPlugin.cpp
--- DSMPlugin/DSMPlugin.cpp 15 Jan 2005 14:32:51 -0000 1.8
+++ DSMPlugin/DSMPlugin.cpp 16 Oct 2005 02:30:39 -0000
@@ -89,7 +89,7 @@
lpString++;
}
*szToken = '\0' ;
- if (( ! *lpString ) || (! *szToken)) return NULL;
+ if (( ! *lpString ) || (! *szToken)) return FALSE;
return FALSE;
}

@@ -102,6 +102,7 @@
{
m_fLoaded = false;
m_fEnabled = false;
+ m_fInitialized = false;
m_lPassLen = 0;

m_pTransBuffer = NULL;
@@ -125,6 +126,7 @@
TRANSFORMBUFFER m_PFreeBuffer = NULL;
RESET m_PReset = NULL;

+ context = NULL;
}

//
@@ -135,6 +137,11 @@
// TODO: Log events
if (IsLoaded())
UnloadPlugin();
+#if 0
+ // TODO: if context isn't NULL, throw a warning that the plugin author screwed up.
+ if (context != NULL)
+ delete *context;
+#endif
}


@@ -187,10 +194,10 @@
//
bool CDSMPlugin::InitPlugin(void)
{
- // TODO: Log events
- int nRes = (*m_PStartup)();
- if (nRes < 0) return false;
- else return true;
+ if(m_fInitialized) return true;
+ if(((*m_PStartup)(&context, m_hPDll)) < 0) return false;
+ m_fInitialized = true;
+ return true;
}

//
@@ -198,8 +205,11 @@
//
bool CDSMPlugin::ResetPlugin(void)
{
+ if(!m_fInitialized) {
+ InitPlugin();
+ }
// TODO: Log events
- int nRes = (*m_PReset)();
+ int nRes = (*m_PReset)(&context);
if (nRes < 0) return false;
else return true;
}
@@ -210,8 +220,11 @@
//
bool CDSMPlugin::SetPluginParams(HWND hWnd, char* szParams)
{
+ if(!m_fInitialized) {
+ InitPlugin();
+ }
// TODO: Log events
- int nRes = (*m_PSetParams)(hWnd, szParams);
+ int nRes = (*m_PSetParams)(&context, hWnd, szParams);
if (nRes > 0) return true; else return false;

}
@@ -222,8 +235,10 @@
//
char* CDSMPlugin::GetPluginParams(void)
{
- //
- return (*m_PGetParams)();
+ if(!m_fInitialized) {
+ InitPlugin();
+ }
+ return (*m_PGetParams)(&context);

}

@@ -280,35 +295,12 @@
//
bool CDSMPlugin::LoadPlugin(char* szPlugin, bool fAllowMulti)
{
- // sf@2003 - Multi dll trick
- // I really don't like doing this kind of dirty workaround but I have no time to do
- // better for now. Used only by a listening viewer.
- // Create a numbered temporary copy of the original plugin dll and load it (viewer only, for now)
- if (fAllowMulti)
- {
- bool fDllCopyCreated = false;
- int i = 1;
- char szDllCopyName[196];
- while (!fDllCopyCreated)
- {
- strcpy(szDllCopyName, szPlugin);
- szDllCopyName[strlen(szPlugin) - 4] = '\0'; //remove the ".dsm" extension
- sprintf(szDllCopyName, "%s-tmp.d%d", szDllCopyName, i++);
- fDllCopyCreated = (bool) CopyFile(szPlugin, szDllCopyName, false);
- // Note: Let's be really dirty; Overwrite if it's possible only (dll not loaded).
- // This way if for some reason (abnormal process termination) the dll wasn't previously
- // normally deleted we overwrite/clean it with the new one at the same time.
- if (i > 99) break; // Just in case...
- }
- strcpy(m_szDllName, szDllCopyName);
- m_hPDll = LoadLibrary(m_szDllName);
- }
- else // Use the original plugin dll
- {
- ZeroMemory(m_szDllName, strlen(m_szDllName));
- m_hPDll = LoadLibrary(szPlugin);
+ if(m_fInitialized) {
+ UnloadPlugin();
}

+ m_hPDll = LoadLibrary(szPlugin);
+
if (m_hPDll == NULL) return false;

m_PDescription = (DESCRIPTION) GetProcAddress(m_hPDll, "Description");
@@ -325,7 +317,6 @@
|| m_PTransformBuffer == NULL || m_PRestoreBuffer == NULL || m_PFreeBuffer == NULL)
{
FreeLibrary(m_hPDll);
- if (*m_szDllName) DeleteFile(m_szDllName);
return false;
}

@@ -342,20 +333,17 @@
{
// TODO: Log events
// Force the DSMplugin to free the buffers it allocated
- if (m_pTransBuffer != NULL) (*m_PFreeBuffer)(m_pTransBuffer);
- if (m_pRestBuffer != NULL) (*m_PFreeBuffer)(m_pRestBuffer);
+ if (m_pTransBuffer != NULL) (*m_PFreeBuffer)(&context, m_pTransBuffer);
+ if (m_pRestBuffer != NULL) (*m_PFreeBuffer)(&context, m_pRestBuffer);

m_pTransBuffer = NULL;
m_pRestBuffer = NULL;

SetLoaded(false);

- if ((*m_PShutdown)())
+ if ((*m_PShutdown)(&context))
{
- bool fFreed = false;
- fFreed = FreeLibrary(m_hPDll);
- if (*m_szDllName) DeleteFile(m_szDllName);
- return fFreed;
+ return FreeLibrary(m_hPDll) ? true : false;
}
else
return false;
@@ -373,7 +361,7 @@
// FixME: possible pb with this mutex in WinVNC
omni_mutex_lock l(m_TransMutex);

- m_pTransBuffer = (*m_PTransformBuffer)(pDataBuffer, nDataLen, pnTransformedDataLen);
+ m_pTransBuffer = (*m_PTransformBuffer)(&context, pDataBuffer, nDataLen, pnTransformedDataLen);

return m_pTransBuffer;
}
@@ -385,13 +373,13 @@
BYTE* CDSMPlugin::RestoreBufferStep1(BYTE* pRestoredDataBuffer, int nDataLen, int* pnRestoredDataLen)
{
//m_RestMutex.lock();
- m_pRestBuffer = (*m_PRestoreBuffer)(pRestoredDataBuffer, nDataLen, pnRestoredDataLen);
+ m_pRestBuffer = (*m_PRestoreBuffer)(&context, pRestoredDataBuffer, nDataLen, pnRestoredDataLen);
return m_pRestBuffer;
}

BYTE* CDSMPlugin::RestoreBufferStep2(BYTE* pRestoredDataBuffer, int nDataLen, int* pnRestoredDataLen)
{
- m_pRestBuffer = (*m_PRestoreBuffer)(pRestoredDataBuffer, nDataLen, pnRestoredDataLen);
+ m_pRestBuffer = (*m_PRestoreBuffer)(&context, pRestoredDataBuffer, nDataLen, pnRestoredDataLen);
//m_RestMutex.unlock();
return NULL;
}
Index: DSMPlugin/DSMPlugin.h
===================================================================
RCS file: /cvsroot/ultravnc/ultravnc/DSMPlugin/DSMPlugin.h,v
retrieving revision 1.8
diff -u -r1.8 DSMPlugin.h
--- DSMPlugin/DSMPlugin.h 17 Feb 2005 21:48:17 -0000 1.8
+++ DSMPlugin/DSMPlugin.h 16 Oct 2005 02:30:17 -0000
@@ -43,14 +43,14 @@

// A plugin dll must export the following functions (with same convention)
typedef char* (__cdecl *DESCRIPTION)(void);
-typedef int (__cdecl *STARTUP)(void);
-typedef int (__cdecl *SHUTDOWN)(void);
-typedef int (__cdecl *SETPARAMS)(HWND, char*);
-typedef char* (__cdecl *GETPARAMS)(void);
-typedef BYTE* (__cdecl *TRANSFORMBUFFER)(BYTE*, int, int*);
-typedef BYTE* (__cdecl *RESTOREBUFFER)(BYTE*, int, int*);
-typedef void (__cdecl *FREEBUFFER)(BYTE*);
-typedef int (__cdecl *RESET)(void);
+typedef int (__cdecl *STARTUP)(void**, HINSTANCE);
+typedef int (__cdecl *SHUTDOWN)(void**);
+typedef int (__cdecl *SETPARAMS)(void**,HWND, char*);
+typedef char* (__cdecl *GETPARAMS)(void**);
+typedef BYTE* (__cdecl *TRANSFORMBUFFER)(void**,BYTE*, int, int*);
+typedef BYTE* (__cdecl *RESTOREBUFFER)(void**,BYTE*, int, int*);
+typedef void (__cdecl *FREEBUFFER)(void**,BYTE*);
+typedef int (__cdecl *RESET)(void**);


//
@@ -88,7 +88,10 @@
omni_mutex m_RestMutex;

private:
+ void *context;
+
bool m_fLoaded;
+ bool m_fInitialized;
bool m_fEnabled;

char szPassword[64];
@@ -100,7 +103,6 @@
char m_szPluginFileName[128]; // No path, just the filename and possible comment

HMODULE m_hPDll;
- char m_szDllName[196];

// Plugin's functions pointers when loaded
DESCRIPTION m_PDescription;
Index: DSMPlugin/TestPlugin/TestPlugin.cpp
===================================================================
RCS file: /cvsroot/ultravnc/ultravnc/DSMPlugin/TestPlugin/TestPlugin.cpp,v
retrieving revision 1.16
diff -u -r1.16 TestPlugin.cpp
--- DSMPlugin/TestPlugin/TestPlugin.cpp 16 Apr 2005 12:00:55 -0000 1.16
+++ DSMPlugin/TestPlugin/TestPlugin.cpp 16 Oct 2005 01:56:50 -0000
@@ -46,30 +46,29 @@
//
//////////////////////////////////////////////////////////////////////////////

+#ifndef TESTPLUGIN_EXPORTS
+#define TESTPLUGIN_EXPORTS
+#endif
#include "TestPlugin.h"

-HINSTANCE hInstance;
-PLUGINSTRUCT* pPlugin = NULL; // struct (or class instance) that handles all Plugin params.
- // Given as an example.
-
-// Internal Plugin vars, depending on what it does
-char szExternalKey[255]; // To store the password/key transmitted via SetParams() by UltraVNC apps
-char szLoaderType[32]; // To store the type of application that has loaded the plugin
-BYTE* pLocalTransBuffer = NULL; // Local Transformation buffer (freed on VNC demand)
-BYTE* pLocalRestBuffer = NULL; // Local Restoration buffer (freed on VNC demand)
-int nLocalTransBufferSize = 0;
-int nLocalRestBufferSize = 0;
-//
-
+typedef struct {
+ HINSTANCE hInstance;
+ char szExternalKey[255]; // To store the password/key transmitted via SetParams() by UltraVNC apps
+ char szLoaderType[32]; // To store the type of application that has loaded the plugin
+ BYTE* pLocalTransBuffer; // Local Transformation buffer (freed on VNC demand)
+ BYTE* pLocalRestBuffer; // Local Restoration buffer (freed on VNC demand)
+ int nLocalTransBufferSize;
+ int nLocalRestBufferSize;
+} pluginState;

// Plugin Description
// Please use the following format (with ',' (comma) as separator)
// Name-Description,Author,Date,Version,FileName-Comment
// For the version, we recommend the following format: x.y.z
// The other fields (Name-Description, Author, Date, FileName-Comment) are format free (don't use ',' in them, of course)
-#define PLUGIN_DESCRIPTION "TestPlugin,Sam,Nov 21 2002,1.0.0,TestPlugin.dsm"
-
+#define PLUGIN_DESCRIPTION "TestPlugin,Duff,Oct 01 2005,2.0.0,TestPlugin.dsm"

+extern "C" {

// ----------------------------------------------------------------------
//
@@ -95,10 +94,16 @@
// Initialize the plugin and all its internals
// Return -1 if error
//
-TESTPLUGIN_API int Startup(void)
+TESTPLUGIN_API int Startup(void **context, HINSTANCE *hInstance)
{
+ *context = (void*)(new pluginState);
+ ((pluginState*)(*context))->pLocalTransBuffer = NULL;
+ ((pluginState*)(*context))->nLocalTransBufferSize = 0;
+ ((pluginState*)(*context))->pLocalRestBuffer = NULL;
+ ((pluginState*)(*context))->nLocalRestBufferSize = 0;
+ ((pluginState*)(*context))->hInstance = *hInstance;
// Init everything
- memset(szExternalKey, 0, sizeof(szExternalKey));
+ memset(((pluginState*)(*context))->szExternalKey, 0, sizeof(((pluginState*)(*context))->szExternalKey));
// Create threads if any
return 1;
}
@@ -108,10 +113,12 @@
// Stop and Clean up the plugin
// Return -1 if error
//
-TESTPLUGIN_API int Shutdown(void)
+TESTPLUGIN_API int Shutdown(void **context)
{
// Terminate Threads if any
// Cleanup everything
+ delete ((pluginState*)(*context));
+ *context = NULL;
return 1;
}

@@ -120,11 +127,11 @@
// Stop and Clean up the plugin
// Return -1 if error
//
-TESTPLUGIN_API int Reset(void)
+TESTPLUGIN_API int Reset(void **context)
{
// Reset the plugin (buffers, keys, whatever that
// requires to be reset between 2 connections
-
+ memset(((pluginState*)(*context))->szExternalKey, 0, sizeof(((pluginState*)(*context))->szExternalKey));
return 1;
}

@@ -163,25 +170,25 @@
// (this info can be used for application/environnement dependent
// operations (config saving...))
//
-TESTPLUGIN_API int SetParams(HWND hVNC, char* szParams)
+TESTPLUGIN_API int SetParams(void **context, HWND hVNC, char* szParams)
{
// CASE 1
// Get the environnement (szLoaderType) value that is always sent from
// VNC viewer or server
- MyStrToken(szLoaderType, szParams, 2, ',');
+ MyStrToken(((pluginState*)(*context))->szLoaderType, szParams, 2, ',');

// If hVNC != 0, display for instance the Plugin Config Dialog box
if (hVNC)
{
// Display the Plugin Config dialog box
- DoDialog();
+ DoDialog(context);
}

// CASE 2:
// Use szParams to setup the Plugin.
// In this example Plugin, the externalkey is not used but we store it anyway.for demo.
// (it corresponds to the VNC password as we require it in the GetParams() function below)
- MyStrToken(szExternalKey, szParams, 1, ',');
+ MyStrToken(((pluginState*)(*context))->szExternalKey, szParams, 1, ',');

return 1;
}
@@ -196,13 +203,13 @@
// Thus this function is called once before the SetParams() function is called
// - Return "VNCPasswordNeeded" if VNC password must be transmitted by the UltraVNC app
// - Return any other Plugin parameters value otherwise (not used in WinVNC & vncviewer for now)
-TESTPLUGIN_API char* GetParams(void)
+TESTPLUGIN_API char* GetParams(void **context)
{
- if (strlen(szExternalKey) > 0)
- return szExternalKey; // Return the already stored externalkey params
+ if (strlen(((pluginState*)(*context))->szExternalKey) > 0)
+ return "IveGotAllINeedThanks";
+ // return strlen(((pluginState*)(*context))->szExternalKey); // Return the already stored externalkey params // err... returning an int for a char*?
else
return "VNCPasswordNeeded";
- // return "IveGotAllINeedThanks";
}

//
@@ -212,9 +219,9 @@
// buffer containing the resulting data.
// The length of the resulting data is given by pnTransformedDataLen
//
-TESTPLUGIN_API BYTE* TransformBuffer(BYTE* pDataBuffer, int nDataLen, int* pnTransformedDataLen)
+TESTPLUGIN_API BYTE* TransformBuffer(void **context, BYTE* pDataBuffer, int nDataLen, int* pnTransformedDataLen)
{
- BYTE* pTransBuffer = CheckLocalTransBufferSize(GiveTransDataLen(nDataLen));
+ BYTE* pTransBuffer = CheckLocalTransBufferSize(context, GiveTransDataLen(nDataLen));
if (pTransBuffer == NULL)
{
*pnTransformedDataLen = -1;
@@ -259,7 +266,7 @@
// - Calls RestoreBuffer again to actually restore data into the given destination buffer.
// This way the copies of data buffers are reduced to the minimum.
//
-TESTPLUGIN_API BYTE* RestoreBuffer(BYTE* pRestoredDataBuffer, int nDataLen, int* pnRestoredDataLen)
+TESTPLUGIN_API BYTE* RestoreBuffer(void **context, BYTE* pRestoredDataBuffer, int nDataLen, int* pnRestoredDataLen)
{
// If given buffer is NULL, allocate necessary space here and return the pointer.
// Additionaly, calculate the resulting length based on nDataLen and return it at the same time.
@@ -268,7 +275,7 @@
// Give the size of the transformed data buffer, based on the original data length
*pnRestoredDataLen = GiveTransDataLen(nDataLen);
// Ensure the pLocalRestBuffer that receive transformed data is big enought
- BYTE* pBuffer = CheckLocalRestBufferSize(*pnRestoredDataLen);
+ BYTE* pBuffer = CheckLocalRestBufferSize(context, *pnRestoredDataLen);
return pBuffer; // Actually pBuffer = pLocalRestBuffer
}

@@ -282,13 +289,13 @@
int i;
for (i = 0; i < nDataLen; i++)
{
- pRestoredDataBuffer = pLocalRestBuffer;
+ pRestoredDataBuffer = ((pluginState*)(*context))->pLocalRestBuffer;
}

// return the resulting data length
*pnRestoredDataLen = GiveRestDataLen(nDataLen);

- return pLocalRestBuffer;
+ return ((pluginState*)(*context))->pLocalRestBuffer;
}


@@ -297,7 +304,7 @@
// in TransformBuffer and RestoreBuffer, using the method adapted
// to the used allocation method.
//
-TESTPLUGIN_API void FreeBuffer(BYTE* pBuffer)
+TESTPLUGIN_API void FreeBuffer(void **context, BYTE* pBuffer)
{
if (pBuffer != NULL)
free(pBuffer);
@@ -310,7 +317,7 @@
// End of functions that must be exported
// -----------------------------------------------------------------

-
+}

// -----------------------------------------------------------------
// Plugin internal Config Dialog Box Sample
@@ -350,10 +357,10 @@
//
// Display the Plugin Config Dialog box
//
-int DoDialog(void)
+int DoDialog(void** context)
{
- return DialogBoxParam(hInstance, MAKEINTRESOURCE(IDD_CONFIG_DIALOG),
- NULL, (DLGPROC) ConfigDlgProc, (LONG) pPlugin);
+ return DialogBoxParam(((pluginState*)(*context))->hInstance, MAKEINTRESOURCE(IDD_CONFIG_DIALOG),
+ NULL, (DLGPROC) ConfigDlgProc, (LONG) (*context));
}


@@ -362,7 +369,7 @@
//
BOOL CALLBACK ConfigDlgProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam )
{
- PLUGINSTRUCT* _this = (PLUGINSTRUCT*) GetWindowLong(hwnd, GWL_USERDATA);
+ pluginState *_this = (pluginState*) GetWindowLong(hwnd, GWL_USERDATA);

switch (uMsg)
{
@@ -460,24 +467,24 @@
// Allocate more space for the local transformation buffer if necessary
// and returns the pointer to this buffer
//
-BYTE* CheckLocalTransBufferSize(int nBufferSize)
+BYTE* CheckLocalTransBufferSize(void** context, int nBufferSize)
{
- if (nLocalTransBufferSize >= nBufferSize) return pLocalTransBuffer;
+ if (((pluginState*)(*context))->nLocalTransBufferSize >= nBufferSize) return ((pluginState*)(*context))->pLocalTransBuffer;

BYTE *pNewBuffer = (BYTE *) malloc (nBufferSize + 256);
if (pNewBuffer == NULL)
{
return NULL;
}
- if (pLocalTransBuffer != NULL)
- free(pLocalTransBuffer);
+ if (((pluginState*)(*context))->pLocalTransBuffer != NULL)
+ free(((pluginState*)(*context))->pLocalTransBuffer);

- pLocalTransBuffer = pNewBuffer;
- nLocalTransBufferSize = nBufferSize + 256;
+ ((pluginState*)(*context))->pLocalTransBuffer = pNewBuffer;
+ ((pluginState*)(*context))->nLocalTransBufferSize = nBufferSize + 256;

- memset(pLocalTransBuffer, 0, nLocalTransBufferSize);
+ memset(((pluginState*)(*context))->pLocalTransBuffer, 0, ((pluginState*)(*context))->nLocalTransBufferSize);

- return pLocalTransBuffer;
+ return ((pluginState*)(*context))->pLocalTransBuffer;
}


@@ -485,24 +492,24 @@
// Allocate more space for the local restoration buffer if necessary
// and returns the pointer to this buffer
//
-BYTE* CheckLocalRestBufferSize(int nBufferSize)
+BYTE* CheckLocalRestBufferSize(void **context, int nBufferSize)
{
- if (nLocalRestBufferSize >= nBufferSize) return pLocalRestBuffer;
+ if (((pluginState*)(*context))->nLocalRestBufferSize >= nBufferSize) return ((pluginState*)(*context))->pLocalRestBuffer;

BYTE *pNewBuffer = (BYTE *) malloc (nBufferSize + 256);
if (pNewBuffer == NULL)
{
return NULL;
}
- if (pLocalRestBuffer != NULL)
- free(pLocalRestBuffer);
+ if (((pluginState*)(*context))->pLocalRestBuffer != NULL)
+ free(((pluginState*)(*context))->pLocalRestBuffer);

- pLocalRestBuffer = pNewBuffer;
- nLocalRestBufferSize = nBufferSize + 256;
+ ((pluginState*)(*context))->pLocalRestBuffer = pNewBuffer;
+ ((pluginState*)(*context))->nLocalRestBufferSize = nBufferSize + 256;

- memset(pLocalRestBuffer, 0, nLocalRestBufferSize);
+ memset(((pluginState*)(*context))->pLocalRestBuffer, 0, ((pluginState*)(*context))->nLocalRestBufferSize);

- return pLocalRestBuffer;
+ return ((pluginState*)(*context))->pLocalRestBuffer;
}


@@ -515,15 +522,5 @@
LPVOID lpReserved
)
{
- switch (dwReason)
- {
- case DLL_PROCESS_ATTACH:
- hInstance = hInst;
- case DLL_THREAD_ATTACH:
- case DLL_THREAD_DETACH:
- case DLL_PROCESS_DETACH:
- break;
- }
return TRUE;
}
-
Index: DSMPlugin/TestPlugin/TestPlugin.h
===================================================================
RCS file: /cvsroot/ultravnc/ultravnc/DSMPlugin/TestPlugin/TestPlugin.h,v
retrieving revision 1.10
diff -u -r1.10 TestPlugin.h
--- DSMPlugin/TestPlugin/TestPlugin.h 15 Apr 2005 20:27:15 -0000 1.10
+++ DSMPlugin/TestPlugin/TestPlugin.h 16 Oct 2005 01:28:55 -0000
@@ -58,6 +58,7 @@
#include "resource.h"

// Plugin struct example - Of course some classes can be used instead...
+#if 0
typedef struct
{
char szDescription[256];
@@ -66,6 +67,7 @@
int nInternalParam2; // And so on.. add what you need.

} PLUGINSTRUCT;
+#endif


//
@@ -74,11 +76,11 @@
BOOL MyStrToken(LPSTR szToken, LPSTR lpString, int nTokenNum, char cSep);
int GiveTransDataLen(int nDataLen);
int GiveRestDataLen(int nDataLen);
-BYTE* CheckLocalTransBufferSize(int nBufferSize);
-BYTE* CheckLocalRestBufferSize(int nBufferSize);
+BYTE* CheckLocalTransBufferSize(void **context, int nBufferSize);
+BYTE* CheckLocalRestBufferSize(void **context, int nBufferSize);
// Config procs if necessary (the plugin is allowed to have no config dialog at all)
BOOL CALLBACK ConfigDlgProc(HWND hwnd, UINT uMsg, WPARAM wParam, LPARAM lParam );
-int DoDialog(void);
+int DoDialog(void **context);


//
@@ -88,12 +90,12 @@
extern "C"
{
TESTPLUGIN_API char* Description(void);
-TESTPLUGIN_API int Startup(void);
-TESTPLUGIN_API int Shutdown(void);
-TESTPLUGIN_API int Reset(void);
-TESTPLUGIN_API int SetParams(HWND hVNC, char* szParams);
-TESTPLUGIN_API char* GetParams(void);
-TESTPLUGIN_API BYTE* TransformBuffer(BYTE* pDataBuffer, int nDataLen, int* pnTransformedDataLen);
-TESTPLUGIN_API BYTE* RestoreBuffer(BYTE* pTransBuffer, int nTransDataLen, int* pnRestoredDataLen);
-TESTPLUGIN_API void FreeBuffer(BYTE* pBuffer);
+TESTPLUGIN_API int Startup(void** context, HINSTANCE *hInstance);
+TESTPLUGIN_API int Shutdown(void** context);
+TESTPLUGIN_API int Reset(void** context);
+TESTPLUGIN_API int SetParams(void** context, HWND hVNC, char* szParams);
+TESTPLUGIN_API char* GetParams(void** context);
+TESTPLUGIN_API BYTE* TransformBuffer(void** context, BYTE* pDataBuffer, int nDataLen, int* pnTransformedDataLen);
+TESTPLUGIN_API BYTE* RestoreBuffer(void** context, BYTE* pTransBuffer, int nTransDataLen, int* pnRestoredDataLen);
+TESTPLUGIN_API void FreeBuffer(void** context, BYTE* pBuffer);
}[/syntax]

[UltraSam]

Thanks Charles,

Sorry. I was away and really busy for 3 weeks
Now I'm going to study all what you've done.

Thanks again for helping making UltraVNC better

[scovel]

Sam, Charles,

I'll be taking a look too. Sounds great!

Sean

[Duff]

One simple thing that needs to be added still: A check to ensure that the API version of the plugin being loaded is matched for the app.

It's easy enough to do: Require that plugins export something like "int getApiVersion()". Anything that doesn't export it is obviously the old pre-refactor code; anything that does returns an API version number that can be checked to make sure it matches what we're trying to support. That way we can pop up a nice dialog box and tell the user when they're trying to load a plugin that was written for an older version of UltraVNC. (In theory, simultanious support for multiple APIs is feasible as well -- though you won't see me implementing it).