From: VMware, Inc <> Date: Tue, 17 Nov 2009 21:44:40 +0000 (-0800) Subject: Fix a few issues with app provider registration in vmtoolsd. X-Git-Tag: 2009.11.16-210370~30 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=40eef16860939e30def815b198c47d7bebe70fbf;p=thirdparty%2Fopen-vm-tools.git Fix a few issues with app provider registration in vmtoolsd. . create a fake provider for the TOOLS_APP_PROVIDER type so that when registering a provider there is no NULL dereference. . correctly detect duplicate providers by looking at the list of existing providers. . if a provider is not found when registering an app, print a log message and continue trying other app regs. . add a provider reg to the test plugin to make sure the above works. Signed-off-by: Marcelo Vanzin --- diff --git a/open-vm-tools/services/vmtoolsd/pluginMgr.c b/open-vm-tools/services/vmtoolsd/pluginMgr.c index bfd503a89..e06d468ec 100644 --- a/open-vm-tools/services/vmtoolsd/pluginMgr.c +++ b/open-vm-tools/services/vmtoolsd/pluginMgr.c @@ -149,6 +149,11 @@ ToolsCoreRegisterApp(ToolsServiceState *state, ToolsAppProviderReg *preg, gpointer reg) { + if (type == TOOLS_APP_PROVIDER) { + /* We should already have registered all providers. */ + return; + } + if (preg == NULL) { g_warning("Plugin %s wants to register app of type %d but no " "provider was found.\n", plugin->name, type); @@ -202,14 +207,21 @@ ToolsCoreRegisterProvider(ToolsServiceState *state, gpointer reg) { if (type == TOOLS_APP_PROVIDER) { + guint k; ToolsAppProvider *prov = reg; ToolsAppProviderReg newreg = { prov, TOOLS_PROVIDER_IDLE }; - /* Assert that no two providers choose the same app type. */ - ASSERT(preg == NULL); - ASSERT(prov->name != NULL); ASSERT(prov->registerApp != NULL); + + /* Assert that no two providers choose the same app type. */ + for (k = 0; k < state->providers->len; k++) { + ToolsAppProviderReg *existing = &g_array_index(state->providers, + ToolsAppProviderReg, + k); + ASSERT(prov->regType != existing->prov->regType); + } + g_array_append_val(state->providers, newreg); } } @@ -251,7 +263,6 @@ ToolsCoreForEachPlugin(ToolsServiceState *state, for (j = 0; j < regs->len; j++) { guint k; - guint provIdx = -1; ToolsAppReg *reg = &g_array_index(regs, ToolsAppReg, j); ToolsAppProviderReg *preg = NULL; @@ -262,11 +273,16 @@ ToolsCoreForEachPlugin(ToolsServiceState *state, k); if (tmp->prov->regType == reg->type) { preg = tmp; - provIdx = k; break; } } + if (preg == NULL) { + g_message("Cannot find provider for app type %d, plugin %s may not work.\n", + reg->type, plugin->data->name); + continue; + } + for (k = 0; k < reg->data->len; k++) { gpointer appdata = ®->data->data[preg->prov->regSize * k]; appRegCb(state, plugin->data, reg->type, preg, appdata); @@ -578,8 +594,8 @@ ToolsCore_RegisterPlugins(ToolsServiceState *state) } /* - * Create two "fake" app providers for the functionality provided by - * vmtoolsd (GuestRPC channel, glib signals). + * Create "fake" app providers for the functionality provided by + * vmtoolsd (GuestRPC channel, glib signals, custom app providers). */ state->providers = g_array_new(FALSE, TRUE, sizeof (ToolsAppProviderReg)); @@ -607,6 +623,18 @@ ToolsCore_RegisterPlugins(ToolsServiceState *state) fakeReg.state = TOOLS_PROVIDER_ACTIVE; g_array_append_val(state->providers, fakeReg); + fakeProv = g_malloc0(sizeof *fakeProv); + fakeProv->regType = TOOLS_APP_PROVIDER; + fakeProv->regSize = sizeof (ToolsAppProvider); + fakeProv->name = "App Provider"; + fakeProv->registerApp = NULL; + fakeProv->dumpState = NULL; + + fakeReg.prov = fakeProv; + fakeReg.state = TOOLS_PROVIDER_ACTIVE; + g_array_append_val(state->providers, fakeReg); + + /* * First app providers need to be identified, so that we know that they're * available for use by plugins who need them. @@ -673,7 +701,8 @@ ToolsCore_UnloadPlugins(ToolsServiceState *state) } if (preg->prov->regType == TOOLS_APP_GUESTRPC || - preg->prov->regType == TOOLS_APP_SIGNALS) { + preg->prov->regType == TOOLS_APP_SIGNALS || + preg->prov->regType == TOOLS_APP_PROVIDER) { g_free(preg->prov); } } diff --git a/open-vm-tools/tests/testPlugin/testPlugin.c b/open-vm-tools/tests/testPlugin/testPlugin.c index 2c9eeea6c..aedc5e7c9 100644 --- a/open-vm-tools/tests/testPlugin/testPlugin.c +++ b/open-vm-tools/tests/testPlugin/testPlugin.c @@ -34,6 +34,12 @@ #include "vmtools.h" #include "guestrpc/ghiGetBinaryHandlers.h" +#define TEST_APP_PROVIDER "TestProvider" +#define TEST_APP_NAME "TestProviderApp1" + +typedef struct TestApp { + const char *name; +} TestApp; /** * Handles a "test.rpcin.msg1" RPC message. The incoming data should be an @@ -266,6 +272,25 @@ TestPluginSetOption(gpointer src, } +/** + * Prints out the registration data for the test provider. + * + * @param[in] ctx Unused. + * @param[in] prov Unused. + * @param[in] reg Registration data (should be a string). + */ + +static void +TestProviderRegisterApp(ToolsAppCtx *ctx, + ToolsAppProvider *prov, + gpointer reg) +{ + TestApp *app = reg; + g_debug("%s: registration data is '%s'\n", __FUNCTION__, app->name); + ASSERT(strcmp(TEST_APP_NAME, app->name) == 0); +} + + /** * Plugin entry point. Returns the registration data. This is called once when * the plugin is loaded into the service process. @@ -293,6 +318,9 @@ ToolsOnLoad(ToolsAppCtx *ctx) { "test.rpcin.msg3", TestPluginRpc3, NULL, NULL, xdr_TestPluginData, 0 } }; + ToolsAppProvider provs[] = { + { TEST_APP_PROVIDER, 42, sizeof (char *), NULL, TestProviderRegisterApp, NULL, NULL } + }; ToolsPluginSignalCb sigs[] = { { TOOLS_CORE_SIG_RESET, TestPluginReset, ®Data }, { TOOLS_CORE_SIG_SHUTDOWN, TestPluginShutdown, ®Data }, @@ -303,9 +331,14 @@ ToolsOnLoad(ToolsAppCtx *ctx) { TOOLS_CORE_SIG_PRESHUTDOWN, TestPluginPreShutdownChange, ®Data }, #endif }; + TestApp tapp[] = { + { TEST_APP_NAME } + }; ToolsAppReg regs[] = { { TOOLS_APP_GUESTRPC, VMTools_WrapArray(rpcs, sizeof *rpcs, ARRAYSIZE(rpcs)) }, - { TOOLS_APP_SIGNALS, VMTools_WrapArray(sigs, sizeof *sigs, ARRAYSIZE(sigs)) } + { TOOLS_APP_PROVIDER, VMTools_WrapArray(provs, sizeof *provs, ARRAYSIZE(provs)) }, + { TOOLS_APP_SIGNALS, VMTools_WrapArray(sigs, sizeof *sigs, ARRAYSIZE(sigs)) }, + { 42, VMTools_WrapArray(tapp, sizeof *tapp, ARRAYSIZE(tapp)) }, }; g_signal_new("test-signal",