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 <mvanzin@vmware.com>

diff --git a/open-vm-tools/services/vmtoolsd/pluginMgr.c b/open-vm-tools/services/vmtoolsd/pluginMgr.c
index bfd503a89045114e3dd81102ee31ec11de9e0048..e06d468ec94309b405ef15c46436051db914422a 100644 (file)
--- 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 = &reg->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 2c9eeea6c106f132d31b6fa94e1dfe42d563c1ce..aedc5e7c9d711a82fd043ad3b272fd3a69ff146e 100644 (file)
--- 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, &regData },
       { TOOLS_CORE_SIG_SHUTDOWN, TestPluginShutdown, &regData },
@@ -303,9 +331,14 @@ ToolsOnLoad(ToolsAppCtx *ctx)
       { TOOLS_CORE_SIG_PRESHUTDOWN, TestPluginPreShutdownChange, &regData },
 #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",