replace plugin enabling via EnabledPlugins by par-plugin Enabled = yes/no

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

10 files changed, 123 insertions, 138 deletions

diff --git a/lib/Plugins/CCpp.conf b/lib/Plugins/CCpp.conf
index dfdbec8b..153efb95 100644
--- a/lib/Plugins/CCpp.conf
+++ b/lib/Plugins/CCpp.conf
@@ -1,4 +1,5 @@
 # Configuration file for CCpp hook and plugin
+Enabled = yes
 
 # If you also want to dump file named "core"
 # in crashed process' current dir, set to "yes"
diff --git a/lib/Plugins/SQLite3.conf b/lib/Plugins/SQLite3.conf
index 3773c8f2..2053642f 100644
--- a/lib/Plugins/SQLite3.conf
+++ b/lib/Plugins/SQLite3.conf
@@ -1,3 +1,4 @@
 # Configuration file for database plugin SQLite3
+
 # DB path
 DBPath = /var/cache/abrt/abrt-db
diff --git a/src/Daemon/Daemon.cpp b/src/Daemon/Daemon.cpp
index 1a6cadf5..75776119 100644
--- a/src/Daemon/Daemon.cpp
+++ b/src/Daemon/Daemon.cpp
@@ -173,13 +173,6 @@ static int SetUpMW()
     {
         g_setBlackList.insert(*it_b);
     }
-    VERB1 log("Registering plugins");
-    set_string_t::iterator it_p = g_settings_setEnabledPlugins.begin();
-    for (; it_p != g_settings_setEnabledPlugins.end(); it_p++)
-    {
-        if (g_pPluginManager->RegisterPlugin(it_p->c_str()) != 0)
-            return -1;
-    }
     VERB1 log("Adding actions or reporters");
     vector_pair_string_string_t::iterator it_ar = g_settings_vectorActionsAndReporters.begin();
     for (; it_ar != g_settings_vectorActionsAndReporters.end(); it_ar++)
diff --git a/src/Daemon/MiddleWare.cpp b/src/Daemon/MiddleWare.cpp
index 25fe2531..3656060a 100644
--- a/src/Daemon/MiddleWare.cpp
+++ b/src/Daemon/MiddleWare.cpp
@@ -204,7 +204,11 @@ static void DebugDumpToCrashReport(const char *pDebugDumpDir, map_crash_report_t
 static std::string GetLocalUUID(const char *pAnalyzer, const char *pDebugDumpDir)
 {
     CAnalyzer* analyzer = g_pPluginManager->GetAnalyzer(pAnalyzer);
-    return analyzer->GetLocalUUID(pDebugDumpDir);
+    if (analyzer)
+    {
+        return analyzer->GetLocalUUID(pDebugDumpDir);
+    }
+    throw CABRTException(EXCEP_PLUGIN, "Error running '%s'", pAnalyzer);
 }
 
 /**
@@ -217,7 +221,11 @@ static std::string GetGlobalUUID(const char *pAnalyzer,
                                        const char *pDebugDumpDir)
 {
     CAnalyzer* analyzer = g_pPluginManager->GetAnalyzer(pAnalyzer);
-    return analyzer->GetGlobalUUID(pDebugDumpDir);
+    if (analyzer)
+    {
+        return analyzer->GetGlobalUUID(pDebugDumpDir);
+    }
+    throw CABRTException(EXCEP_PLUGIN, "Error running '%s'", pAnalyzer);
 }
 
 /**
@@ -232,7 +240,11 @@ static void CreateReport(const char *pAnalyzer,
                 int force)
 {
     CAnalyzer* analyzer = g_pPluginManager->GetAnalyzer(pAnalyzer);
-    analyzer->CreateReport(pDebugDumpDir, force);
+    if (analyzer)
+    {
+        analyzer->CreateReport(pDebugDumpDir, force);
+    }
+    /* else: GetAnalyzer() already complained, no need to handle it here */
 }
 
 mw_result_t CreateCrashReport(const char *pUUID,
@@ -320,9 +332,14 @@ void RunAction(const char *pActionDir,
                             const char *pPluginName,
                             const char *pPluginArgs)
 {
+    CAction* action = g_pPluginManager->GetAction(pPluginName);
+    if (!action)
+    {
+        /* GetAction() already complained */
+        return;
+    }
     try
     {
-        CAction* action = g_pPluginManager->GetAction(pPluginName);
         action->Run(pActionDir, pPluginArgs);
     }
     catch (CABRTException& e)
@@ -770,13 +787,17 @@ static void RunAnalyzerActions(const char *pAnalyzer, const char *pDebugDumpDir)
         for (; it_a != analyzer->second.end(); it_a++)
         {
             const char *plugin_name = it_a->first.c_str();
+            CAction* action = g_pPluginManager->GetAction(plugin_name, /*silent:*/ true);
+            if (!action)
+            {
+                /* GetAction() already complained if no such plugin.
+		 * If plugin exists but isn't an Action, it's not an error.
+		 */
+                continue;
+            }
             try
             {
-                if (g_pPluginManager->GetPluginType(plugin_name) == ACTION)
-                {
-                    CAction* action = g_pPluginManager->GetAction(plugin_name);
-                    action->Run(pDebugDumpDir, it_a->second.c_str());
-                }
+                action->Run(pDebugDumpDir, it_a->second.c_str());
             }
             catch (CABRTException& e)
             {
diff --git a/src/Daemon/PluginManager.cpp b/src/Daemon/PluginManager.cpp
index 663228b5..4aef630c 100644
--- a/src/Daemon/PluginManager.cpp
+++ b/src/Daemon/PluginManager.cpp
@@ -138,7 +138,7 @@ void CPluginManager::LoadPlugins()
             if (!ext || strcmp(ext + 1, PLUGINS_LIB_EXTENSION) != 0)
                 continue;
             *ext = '\0';
-            LoadPlugin(dent->d_name + sizeof(PLUGINS_LIB_PREFIX)-1);
+            LoadPlugin(dent->d_name + sizeof(PLUGINS_LIB_PREFIX)-1, /*enabled_only:*/ true);
         }
         closedir(dir);
     }
@@ -153,16 +153,37 @@ void CPluginManager::UnLoadPlugins()
     }
 }
 
-void CPluginManager::LoadPlugin(const char *pName)
+CPlugin* CPluginManager::LoadPlugin(const char *pName, bool enabled_only)
 {
-    if (m_mapLoadedModules.find(pName) != m_mapLoadedModules.end())
+    map_plugin_t::iterator it_plugin = m_mapPlugins.find(pName);
+    if (it_plugin != m_mapPlugins.end())
     {
-        return;
+        return it_plugin->second; /* ok */
+    }
+
+    const char *conf_name = pName;
+    if (strncmp(pName, "Kerneloops", sizeof("Kerneloops")-1) == 0)
+    {
+        /* Kerneloops{,Scanner,Reporter} share the same .conf file */
+        conf_name = "Kerneloops";
+    }
+    map_plugin_settings_t pluginSettings;
+    std::string conf_fullname = ssprintf(PLUGINS_CONF_DIR"/%s."PLUGINS_CONF_EXTENSION, conf_name);
+    LoadPluginSettings(conf_fullname.c_str(), pluginSettings);
+    VERB3 log("Loaded %s.conf", conf_name);
+
+    if (enabled_only)
+    {
+        map_plugin_settings_t::iterator it = pluginSettings.find("Enabled");
+        if (it == pluginSettings.end() || !string_to_bool(it->second.c_str()))
+        {
+            VERB3 log("Plugin %s: 'Enabled' is not set, not loading it (yet)", pName);
+            return NULL; /* error */
+        }
     }
 
     std::string libPath = ssprintf(PLUGINS_LIB_DIR"/"PLUGINS_LIB_PREFIX"%s."PLUGINS_LIB_EXTENSION, pName);
     CLoadedModule* module = new CLoadedModule(libPath.c_str());
-
     if (module->GetMagicNumber() != PLUGINS_MAGIC_NUMBER
      || module->GetType() < 0
      || module->GetType() > MAX_PLUGIN_TYPE
@@ -172,95 +193,63 @@ void CPluginManager::LoadPlugin(const char *pName)
                 module->GetMagicNumber(), PLUGINS_MAGIC_NUMBER,
                 module->GetType(), MAX_PLUGIN_TYPE);
         delete module;
-        return;
-    }
-
-    log("Plugin %s (%s) succesfully loaded", pName, module->GetVersion());
-    m_mapLoadedModules[pName] = module;
-}
-
-void CPluginManager::UnLoadPlugin(const char *pName)
-{
-    map_loaded_module_t::iterator it_module = m_mapLoadedModules.find(pName);
-    if (it_module != m_mapLoadedModules.end())
-    {
-        UnRegisterPlugin(pName);
-        delete it_module->second;
-        m_mapLoadedModules.erase(it_module);
-        log("Plugin %s successfully unloaded", pName);
+        return NULL; /* error */
     }
-}
+    VERB3 log("Loaded plugin %s v.%s", pName, module->GetVersion());
 
-int CPluginManager::RegisterPlugin(const char *pName)
-{
-    map_loaded_module_t::iterator it_module = m_mapLoadedModules.find(pName);
-    if (it_module == m_mapLoadedModules.end())
-    {
-        error_msg("Can't initialize plugin %s: no such plugin installed", pName);
-        return -1; /* failure */
-    }
-    if (m_mapPlugins.find(pName) != m_mapPlugins.end())
-    {
-        return 0; /* already registered, success */
-    }
-
-    /* Loaded, but not registered yet */
-    CPlugin* plugin = it_module->second->PluginNew();
-    map_plugin_settings_t pluginSettings;
-
-    const char *conf_name = pName;
-    if (strncmp(pName, "Kerneloops", sizeof("Kerneloops")-1) == 0) {
-        /* Kerneloops{,Scanner,Reporter} share the same .conf file */
-        conf_name = "Kerneloops";
-    }
-    LoadPluginSettings(ssprintf(PLUGINS_CONF_DIR"/%s."PLUGINS_CONF_EXTENSION, conf_name).c_str(), pluginSettings);
-    VERB3 log("Loaded %s.conf", conf_name);
+    CPlugin* plugin = NULL;
     try
     {
+        plugin = module->PluginNew();
         plugin->Init();
         plugin->SetSettings(pluginSettings);
     }
     catch (CABRTException& e)
     {
-        error_msg("Can't initialize plugin %s(%s): %s",
+        error_msg("Can't initialize plugin %s: %s",
                 pName,
-                plugin_type_str[it_module->second->GetType()],
                 e.what()
         );
-        UnLoadPlugin(pName);
-        return -1; /* failure */
+        delete plugin;
+        delete module;
+        return NULL; /* error */
     }
-    m_mapPlugins[pName] = plugin;
-    log("Registered plugin %s(%s)", pName, plugin_type_str[it_module->second->GetType()]);
-    return 0; /* success */
-}
 
-void CPluginManager::RegisterPluginDBUS(const char *pName, const char *pDBUSSender)
-{
-    int polkit_result = polkit_check_authorization(pDBUSSender,
-                           "org.fedoraproject.abrt.change-daemon-settings");
-    if (polkit_result == PolkitYes)
-    {
-        RegisterPlugin(pName);
-    } else
-    {
-        log("User %s not authorized, returned %d", pDBUSSender, polkit_result);
-    }
+    m_mapLoadedModules[pName] = module;
+    m_mapPlugins[pName] = plugin;
+    log("Registered %s plugin '%s'", plugin_type_str[module->GetType()], pName);
+    return plugin; /* ok */
 }
 
-void CPluginManager::UnRegisterPlugin(const char *pName)
+void CPluginManager::UnLoadPlugin(const char *pName)
 {
     map_loaded_module_t::iterator it_module = m_mapLoadedModules.find(pName);
     if (it_module != m_mapLoadedModules.end())
     {
         map_plugin_t::iterator it_plugin = m_mapPlugins.find(pName);
-        if (it_plugin != m_mapPlugins.end())
+        if (it_plugin != m_mapPlugins.end()) /* always true */
         {
             it_plugin->second->DeInit();
             delete it_plugin->second;
             m_mapPlugins.erase(it_plugin);
-            log("UnRegistered plugin %s(%s)", pName, plugin_type_str[it_module->second->GetType()]);
         }
+        delete it_module->second;
+        m_mapLoadedModules.erase(it_module);
+        log("UnRegistered %s plugin %s", plugin_type_str[it_module->second->GetType()], pName);
+    }
+}
+
+void CPluginManager::RegisterPluginDBUS(const char *pName, const char *pDBUSSender)
+{
+    int polkit_result = polkit_check_authorization(pDBUSSender,
+                           "org.fedoraproject.abrt.change-daemon-settings");
+    if (polkit_result == PolkitYes)
+    {
+//TODO: report success/failure
+        LoadPlugin(pName);
+    } else
+    {
+        log("User %s not authorized, returned %d", pDBUSSender, polkit_result);
     }
 }
 
@@ -270,7 +259,7 @@ void CPluginManager::UnRegisterPluginDBUS(const char *pName, const char *pDBUSSe
                            "org.fedoraproject.abrt.change-daemon-settings");
     if (polkit_result == PolkitYes)
     {
-        UnRegisterPlugin(pName);
+        UnLoadPlugin(pName);
     } else
     {
         log("user %s not authorized, returned %d", pDBUSSender, polkit_result);
@@ -280,50 +269,57 @@ void CPluginManager::UnRegisterPluginDBUS(const char *pName, const char *pDBUSSe
 
 CAnalyzer* CPluginManager::GetAnalyzer(const char *pName)
 {
-    map_plugin_t::iterator it_plugin = m_mapPlugins.find(pName);
-    if (it_plugin == m_mapPlugins.end())
+    CPlugin* plugin = LoadPlugin(pName);
+    if (!plugin)
     {
-        throw CABRTException(EXCEP_PLUGIN, "Plugin '%s' is not registered", pName);
+        error_msg("Plugin '%s' is not registered", pName);
+        return NULL;
     }
     if (m_mapLoadedModules[pName]->GetType() != ANALYZER)
     {
-        throw CABRTException(EXCEP_PLUGIN, "Plugin '%s' is not an analyzer plugin", pName);
+        error_msg("Plugin '%s' is not an analyzer plugin", pName);
+        return NULL;
     }
-    return (CAnalyzer*)(it_plugin->second);
+    return (CAnalyzer*)plugin;
 }
 
 CReporter* CPluginManager::GetReporter(const char *pName)
 {
-    map_plugin_t::iterator it_plugin = m_mapPlugins.find(pName);
-    if (it_plugin == m_mapPlugins.end())
+    CPlugin* plugin = LoadPlugin(pName);
+    if (!plugin)
     {
-        throw CABRTException(EXCEP_PLUGIN, "Plugin '%s' is not registered", pName);
+        error_msg("Plugin '%s' is not registered", pName);
+        return NULL;
     }
     if (m_mapLoadedModules[pName]->GetType() != REPORTER)
     {
-        throw CABRTException(EXCEP_PLUGIN, "Plugin '%s' is not a reporter plugin", pName);
+        error_msg("Plugin '%s' is not a reporter plugin", pName);
+        return NULL;
     }
-    return (CReporter*)(it_plugin->second);
+    return (CReporter*)plugin;
 }
 
-CAction* CPluginManager::GetAction(const char *pName)
+CAction* CPluginManager::GetAction(const char *pName, bool silent)
 {
-    map_plugin_t::iterator it_plugin = m_mapPlugins.find(pName);
-    if (it_plugin == m_mapPlugins.end())
+    CPlugin* plugin = LoadPlugin(pName);
+    if (!plugin)
     {
-        throw CABRTException(EXCEP_PLUGIN, "Plugin '%s' is not registered", pName);
+        error_msg("Plugin '%s' is not registered", pName);
+        return NULL;
     }
     if (m_mapLoadedModules[pName]->GetType() != ACTION)
     {
-        throw CABRTException(EXCEP_PLUGIN, "Plugin '%s' is not an action plugin", pName);
+        if (!silent)
+            error_msg("Plugin '%s' is not an action plugin", pName);
+        return NULL;
     }
-    return (CAction*)(it_plugin->second);
+    return (CAction*)plugin;
 }
 
 CDatabase* CPluginManager::GetDatabase(const char *pName)
 {
-    map_plugin_t::iterator it_plugin = m_mapPlugins.find(pName);
-    if (it_plugin == m_mapPlugins.end())
+    CPlugin* plugin = LoadPlugin(pName);
+    if (!plugin)
     {
         throw CABRTException(EXCEP_PLUGIN, "Plugin '%s' is not registered", pName);
     }
@@ -331,17 +327,18 @@ CDatabase* CPluginManager::GetDatabase(const char *pName)
     {
         throw CABRTException(EXCEP_PLUGIN, "Plugin '%s' is not a database plugin", pName);
     }
-    return (CDatabase*)(it_plugin->second);
+    return (CDatabase*)plugin;
 }
 
 plugin_type_t CPluginManager::GetPluginType(const char *pName)
 {
-    map_plugin_t::iterator it_plugin = m_mapPlugins.find(pName);
-    if (it_plugin == m_mapPlugins.end())
+    CPlugin* plugin = LoadPlugin(pName);
+    if (!plugin)
     {
         throw CABRTException(EXCEP_PLUGIN, "Plugin '%s' is not registered", pName);
     }
-    return m_mapLoadedModules[pName]->GetType();
+    map_loaded_module_t::iterator it_module = m_mapLoadedModules.find(pName);
+    return it_module->second->GetType();
 }
 
 vector_map_string_t CPluginManager::GetPluginsInfo()
diff --git a/src/Daemon/PluginManager.h b/src/Daemon/PluginManager.h
index 2d649c5f..65058a50 100644
--- a/src/Daemon/PluginManager.h
+++ b/src/Daemon/PluginManager.h
@@ -74,7 +74,7 @@ class CPluginManager
          * A method, which loads particular plugin.
          * @param pName A plugin name.
          */
-        void LoadPlugin(const char *pName);
+        CPlugin* LoadPlugin(const char *pName, bool enabled_only = false);
         /**
          * A method, which unloads particular plugin.
          * @param pName A plugin name.
@@ -84,18 +84,6 @@ class CPluginManager
          * A method, which registers particular plugin.
          * @param pName A plugin name.
          */
-        int RegisterPlugin(const char *pName);
-        /**
-         * A method, which unregister particular plugin.
-         * @param pName A plugin name.
-         */
-        void UnRegisterPlugin(const char *pName);
-        /**
-         * A method, which registers particular plugin,
-         * called via DBUS
-         * @param pName A plugin name.
-         * @param pDBUSSender DBUS user identification
-         */
         void RegisterPluginDBUS(const char *pName, const char *pDBUSSender);
         /**
          * A method, which unregister particular plugin,
@@ -121,7 +109,7 @@ class CPluginManager
          * @param pName A plugin name.
          * @return An action plugin.
          */
-        CAction* GetAction(const char *pName);
+        CAction* GetAction(const char *pName, bool silent = false);
         /**
          * A method, which returns instance of particular database plugin.
          * @param pName A plugin name.
diff --git a/src/Daemon/Settings.cpp b/src/Daemon/Settings.cpp
index d89bebfd..48658e28 100644
--- a/src/Daemon/Settings.cpp
+++ b/src/Daemon/Settings.cpp
@@ -37,7 +37,6 @@ bool          g_settings_bOpenGPGCheck = false;
 /* one line: "OpenGPGPublicKeys = value1,value2" */
 set_string_t  g_settings_setOpenGPGPublicKeys;
 set_string_t  g_settings_mapBlackList;
-set_string_t  g_settings_setEnabledPlugins;
 std::string   g_settings_sDatabase;
 unsigned int  g_settings_nMaxCrashReportsSize = 1000;
 /* one line: "ActionsAndReporters = aa_first,bb_first(bb_second),cc_first" */
@@ -168,11 +167,6 @@ static void ParseCommon()
     {
         g_settings_sDatabase = it->second;
     }
-    it = s_mapSectionCommon.find("EnabledPlugins");
-    if (it != end)
-    {
-        g_settings_setEnabledPlugins = ParseList(it->second.c_str());
-    }
     it = s_mapSectionCommon.find("MaxCrashReportsSize");
     if (it != end)
     {
@@ -434,7 +428,6 @@ void SaveSettings()
         SaveBool("OpenGPGCheck", g_settings_bOpenGPGCheck, fOut);
         SaveSetString("OpenGPGPublicKeys", g_settings_setOpenGPGPublicKeys, fOut);
         SaveSetString("BlackList", g_settings_mapBlackList, fOut);
-        SaveSetString("EnabledPlugins", g_settings_setEnabledPlugins, fOut);
         fprintf(fOut, "Database = %s\n", g_settings_sDatabase.c_str());
         fprintf(fOut, "MaxCrashReportsSize = %u\n", g_settings_nMaxCrashReportsSize);
         SaveVectorPairStrings("ActionsAndReporters", g_settings_vectorActionsAndReporters, fOut);
diff --git a/src/Daemon/Settings.h b/src/Daemon/Settings.h
index 9ee9370d..3fdb8b9d 100644
--- a/src/Daemon/Settings.h
+++ b/src/Daemon/Settings.h
@@ -9,7 +9,6 @@ typedef map_map_string_t map_abrt_settings_t;
 
 extern set_string_t  g_settings_setOpenGPGPublicKeys;
 extern set_string_t  g_settings_mapBlackList;
-extern set_string_t  g_settings_setEnabledPlugins;
 extern unsigned int  g_settings_nMaxCrashReportsSize;
 extern bool          g_settings_bOpenGPGCheck;
 extern std::string   g_settings_sDatabase;
diff --git a/src/Daemon/abrt.conf b/src/Daemon/abrt.conf
index 276bf251..acf8566f 100644
--- a/src/Daemon/abrt.conf
+++ b/src/Daemon/abrt.conf
@@ -9,11 +9,7 @@ OpenGPGCheck = no
 OpenGPGPublicKeys = /etc/pki/rpm-gpg/RPM-GPG-KEY-fedora
 # Blacklisted packages
 BlackList = nspluginwrapper
-# Enabled plugins.
-# You can disable handling e.g. Python crashes by not listing Python here.
-# There has to be exactly one database plugin enabled.
-EnabledPlugins = SQLite3, CCpp, Logger, Kerneloops, KerneloopsScanner, KerneloopsReporter, Bugzilla, Python, RunApp
-# Database
+# Which database plugin to use
 Database = SQLite3
 # Max size for crash storage [MiB]
 MaxCrashReportsSize = 1000
diff --git a/src/Daemon/abrt.conf.5 b/src/Daemon/abrt.conf.5
index 3b172bcb..7114f5cb 100644
--- a/src/Daemon/abrt.conf.5
+++ b/src/Daemon/abrt.conf.5
@@ -30,10 +30,6 @@ to report them if "EnableOpenGPG = yes".
 .I abrt
 will ignore packages in this list and will not handle their crashes.
 .TP
-.B EnabledPlugins = \fIplugin\fP, \fIplugin\fP ...
-.I abrt
-will only load plugins in this list.
-.TP
 .B Database = \fIdatabasePlugin\fP
 This specifies which database plugin
 .I abrt