add experimental saving of /var/log/Xorg*.log for X crashes

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

8 files changed, 102 insertions, 70 deletions

diff --git a/doc/DESIGN b/doc/DESIGN
index 78947238..7298b4c8 100644
--- a/doc/DESIGN
+++ b/doc/DESIGN
@@ -107,14 +107,19 @@ The key dbus calls served by abrt-process are:
 	Development plan
 
 Since current code does not match the planned design, we need to gradually
-change the code to "morph" it into the desired shape. Planned steps:
+change the code to "morph" it into the desired shape.
 
-* make kerneloops plugin into separate daemon (convert it to a hook
-  and get rid of "cron plugins" which are wrong idea since the begining)
-* Make abrtd dbus startable
+Done:
+
+* Make abrtd dbus startable.
+* Add -t TIMEOUT_SEC option to abrtd. {done}
 * Make abrt-gui start abrtd on demand, so that abrt-gui can be started
   even if abrtd does not run at the moment.
-* Add -t TIMEOUT_SEC option to abrtd.
+
+Planned steps:
+
+* make kerneloops plugin into separate daemon (convert it to a hook
+  and get rid of "cron plugins" which are wrong idea since the begining)
 * ???
 * ???
 * ???
diff --git a/doc/PLUGINS-HOWTO b/doc/PLUGINS-HOWTO
index ef0c6ba6..d8467801 100644
--- a/doc/PLUGINS-HOWTO
+++ b/doc/PLUGINS-HOWTO
@@ -18,57 +18,64 @@ The files that were created when the application crashed (such as core dumps)
 are all stored here. In addition, the plugins can write
 their output files there, if any.
 
-Let's discuss the plugin types in detail:
+Plugin types in detail:
+
 
 Action Plugin
 -------------
-You use this type of plugin when you need some action to be
-performed when a crash is encountered.
-you have to override one method:
+This type of plugin is useful when you need some action to be performed
+immediately when a crash is encountered (using ActionsAndReporters = ...
+directive in abrt.conf), or if you need some action to be performed
+periodically ([Cron] section in the same file).
+
+You have to override one method:
+
+virtual void Run(const char *dir, const char *args);
+
+The first argument is a directory that contains the current debug
+dump, or all debug dumps for periodic actions.
+The second argument is a string with arguments (specified in config file).
 
-virtual void Run(const std::string& pActiveDir,
-                 const std::string& pArgs) = 0;
--This method runs the specified action.
- The first argument is a directory name. It can be either the current debug
- dump dir or a directory that contains all debug dumps.
- The second argument is a string with arguments specified for the action.
 
 Analyzer Plugin
 ---------------
 This plugin has to compute the UUID of the crash. Crashes differ, depending on
 where they occur, for example crashes in the kernel differ from crashes in
 userspace binaries, which differ from crashes in python scripts. Therefore,
-you need a plugin for each type of application that you want "abrt" to handle.
+you need a plugin for each type of application that you want abrt to handle.
 
-you have to override these methods:
+You have to override these methods:
 
-virtual std::string GetLocalUUID(const std::string& pDebugDumpPath) = 0;
+virtual std::string GetLocalUUID(const std::string& pDebugDumpPath);
 - This method computes the local UUID of the crash.
 
-virtual std::string GetGlobalUUID(const std::string& pDebugDumpPath) = 0;
+virtual std::string GetGlobalUUID(const std::string& pDebugDumpPath);
 - This method computes the global UUID of the crash.
 
-NOTE:The difference between local and global UUID is that the local UUID
+{something is fishy here}
+
+NOTE: The difference between local and global UUID is that the local UUID
 is specific for the machine architecture on which the crash is encountered.
 When the crash is reported, abrt has to use the "-debuginfo" packages
 to render a global UUID, which should be independent of the specific
 system (the same crash on different architectures/configurations can
 yield different local UUIDs but has to have the same global UUID).
 
-virtual void CreateReport(const std::string& pDebugDumpPath) = 0;
+virtual void CreateReport(const std::string& pDebugDumpPath);
 - This method creates the report about the crash and stores it
   in the crash's directory.
 
+
 Reporter Plugin
 ---------------
 This plugin receives the entire finished crash report and
 posts/reports it somewhere (e.g. logs it, mails it, posts
 it on some web tool...)
 
-you have to override this method:
+You have to override this method:
 
 virtual void Report(const crash_report_t& pCrashReport,
-                    const std::string& pArgs) = 0;
+                    const std::string& pArgs);
 -It is self-explanatory, that this method takes the report
  and presents it somewhere to the world.
  The second argument is a string with arguments specified for the reporter.
@@ -82,30 +89,30 @@ is not the same as some crash before. The database can be local, or in
 some centralized location on the network.
 
 you have to override these methods:
-virtual void Connect() = 0;
-virtual void DisConnect() = 0;
+virtual void Connect();
+virtual void DisConnect();
 - connect and disconnect from the database
 
 virtual void Insert(const std::string& pUUID,
                     const std::string& pUID,
                     const std::string& pDebugDumpPath,
-                    const std::string& pTime) = 0;
+                    const std::string& pTime);
 - insert an entry into the database: you use both UID (user ID) and UUID
   (ID of the crash)
 
-virtual void Delete(const std::string& pUUID, const std::string& pUID) = 0;
+virtual void Delete(const std::string& pUUID, const std::string& pUID);
 - delete an entry
 
-virtual void SetReported(const std::string& pUUID, const std::string& pUID) = 0;
+virtual void SetReported(const std::string& pUUID, const std::string& pUID);
 - insert information into the database to say that this bug was already
   reported (so for example the report plugins won't run several times
   for the same bug)
 
-virtual const vector_database_rows_t GetUIDData(const std::string& pUID) = 0;
+virtual const vector_database_rows_t GetUIDData(const std::string& pUID);
 - get database rows for the specified user ID
 
 virtual const database_row_t GetUUIDData(const std::string& pUUID, const
- std::string& pUID) = 0;
+ std::string& pUID);
 - get a database row for the specified user ID and UUID
 
 
@@ -124,7 +131,7 @@ PLUGIN_INFO(type, plugin_class, name, version, description, email, www)
 - "description" is a string with the summary of what the plugin does
 - "email" and "www" are strings with the contact info for the author
 
-for example:
+For example:
 PLUGIN_INFO(REPORTER,
             CMailx,
             "Mailx",
diff --git a/lib/Plugins/RunApp.cpp b/lib/Plugins/RunApp.cpp
index 3be7bdf4..a2aa6987 100644
--- a/lib/Plugins/RunApp.cpp
+++ b/lib/Plugins/RunApp.cpp
@@ -21,7 +21,6 @@
 
 
 #include "RunApp.h"
-#include <stdio.h>
 #include "DebugDump.h"
 #include "ABRTException.h"
 #include "CommLayerInner.h"
@@ -30,20 +29,39 @@
 #define COMMAND     0
 #define FILENAME    1
 
+using namespace std;
+
 void CActionRunApp::Run(const char *pActionDir, const char *pArgs)
 {
-    update_client(_("Executing RunApp plugin..."));
+    /* Don't update_client() - actions run at crash time */
+    log("RunApp('%s','%s')", pActionDir, pArgs);
 
-    std::string output;
     vector_string_t args;
-
     parse_args(pArgs, args, '"');
 
-    FILE *fp = popen(args[COMMAND].c_str(), "r");
+    const char *cmd = args[COMMAND].c_str();
+    if (!cmd[0])
+    {
+        return;
+    }
+
+//FIXME: need to be able to escape " in .conf
+    /* Chdir to the dump dir. Command can analyze component and such.
+     * Example:
+     * test x"`cat component`" = x"xorg-x11-apps" && cp /var/log/Xorg.0.log .
+     */
+//Can do it using chdir() in child if we'd open-code popen
+    string cd_and_cmd = ssprintf("cd '%s'; %s", pActionDir, cmd);
+    FILE *fp = popen(cd_and_cmd.c_str(), "r");
     if (fp == NULL)
     {
-        throw CABRTException(EXCEP_PLUGIN, "Can't execute " + args[COMMAND]);
+        /* Happens only on resource starvation (fork fails or out-of-mem) */
+        return;
     }
+
+//FIXME: RunApp("gzip -9 </var/log/Xorg.0.log", "Xorg.0.log.gz") fails
+//since we mangle NULs.
+    string output;
     char line[1024];
     while (fgets(line, 1024, fp) != NULL)
     {
@@ -51,7 +69,7 @@ void CActionRunApp::Run(const char *pActionDir, const char *pArgs)
     }
     pclose(fp);
 
-    if (args.size() > 1)
+    if (args.size() > FILENAME)
     {
         CDebugDump dd;
         dd.Open(pActionDir);
diff --git a/lib/Utils/DebugDump.cpp b/lib/Utils/DebugDump.cpp
index ba11d965..2883d01f 100644
--- a/lib/Utils/DebugDump.cpp
+++ b/lib/Utils/DebugDump.cpp
@@ -74,7 +74,7 @@ void CDebugDump::Open(const char *pDir)
 
 bool CDebugDump::Exist(const char* pPath)
 {
-    std::string fullPath = m_sDebugDumpDir + "/" + pPath;
+    std::string fullPath = concat_path_file(m_sDebugDumpDir.c_str(), pPath);
     return ExistFileDir(fullPath.c_str());
 }
 
@@ -433,7 +433,7 @@ void CDebugDump::LoadText(const char* pName, std::string& pData)
     {
         throw CABRTException(EXCEP_DD_OPEN, "CDebugDump::LoadText(): DebugDump is not opened.");
     }
-    std::string fullPath = m_sDebugDumpDir + '/' + pName;
+    std::string fullPath = concat_path_file(m_sDebugDumpDir.c_str(), pName);
     LoadTextFile(fullPath.c_str(), pData);
 }
 
@@ -443,7 +443,7 @@ void CDebugDump::SaveText(const char* pName, const char* pData)
     {
         throw CABRTException(EXCEP_DD_OPEN, "CDebugDump::SaveText(): DebugDump is not opened.");
     }
-    std::string fullPath = m_sDebugDumpDir + "/" + pName;
+    std::string fullPath = concat_path_file(m_sDebugDumpDir.c_str(), pName);
     SaveBinaryFile(fullPath.c_str(), pData, strlen(pData));
 }
 void CDebugDump::SaveBinary(const char* pName, const char* pData, unsigned pSize)
@@ -452,7 +452,7 @@ void CDebugDump::SaveBinary(const char* pName, const char* pData, unsigned pSize
     {
         throw CABRTException(EXCEP_DD_OPEN, "CDebugDump::SaveBinary(): DebugDump is not opened.");
     }
-    std::string fullPath = m_sDebugDumpDir + "/" + pName;
+    std::string fullPath = concat_path_file(m_sDebugDumpDir.c_str(), pName);
     SaveBinaryFile(fullPath.c_str(), pData, pSize);
 }
 
diff --git a/src/Daemon/MiddleWare.cpp b/src/Daemon/MiddleWare.cpp
index 0bc358e1..dc5a295a 100644
--- a/src/Daemon/MiddleWare.cpp
+++ b/src/Daemon/MiddleWare.cpp
@@ -278,17 +278,21 @@ void RunActionsAndReporters(const char *pDebugDumpDir)
     {
         try
         {
-            if (g_pPluginManager->GetPluginType(it_ar->first) == REPORTER)
+            VERB3 log("RunActionsAndReporters: checking %s", it_ar->first.c_str());
+            plugin_type_t tp = g_pPluginManager->GetPluginType(it_ar->first);
+            if (tp == REPORTER)
             {
                 CReporter* reporter = g_pPluginManager->GetReporter(it_ar->first);
 
                 map_crash_report_t crashReport;
                 DebugDumpToCrashReport(pDebugDumpDir, crashReport);
+                VERB2 log("%s.Report(...)", it_ar->first.c_str());
                 reporter->Report(crashReport, plugin_settings, it_ar->second);
             }
-            else if (g_pPluginManager->GetPluginType(it_ar->first) == ACTION)
+            else if (tp == ACTION)
             {
                 CAction* action = g_pPluginManager->GetAction(it_ar->first);
+                VERB2 log("%s.Run('%s','%s')", it_ar->first.c_str(), pDebugDumpDir, it_ar->second.c_str());
                 action->Run(pDebugDumpDir, it_ar->second.c_str());
             }
         }
@@ -375,18 +379,22 @@ report_status_t Report(const map_crash_report_t& pCrashReport,
         }
     }
 
-    // analyzer with package name (CCpp:xrog-x11-app) has higher priority
+    // analyzer with package name (CCpp:xorg-x11-app) has higher priority
     std::string key = analyzer + ":" + packageName;
+    map_analyzer_actions_and_reporters_t::iterator end = s_mapAnalyzerActionsAndReporters.end();
     map_analyzer_actions_and_reporters_t::iterator keyPtr = s_mapAnalyzerActionsAndReporters.find(key);
-    if (keyPtr == s_mapAnalyzerActionsAndReporters.end())
+    if (keyPtr == end)
     {
         // if there is no such settings, then try default analyzer
         keyPtr = s_mapAnalyzerActionsAndReporters.find(analyzer);
+        key = analyzer;
     }
 
     std::string message;
-    if (keyPtr != s_mapAnalyzerActionsAndReporters.end())
+    if (keyPtr != end)
     {
+        VERB2 log("Found AnalyzerActionsAndReporters for '%s'", key.c_str());
+
         vector_pair_string_string_t::iterator it_r = keyPtr->second.begin();
         for (; it_r != keyPtr->second.end(); it_r++)
         {
@@ -433,7 +441,7 @@ report_status_t Report(const map_crash_report_t& pCrashReport,
             {
                 ret[pluginName].push_back("0");
                 ret[pluginName].push_back(e.what());
-                update_client("Reporting via  %s' was not successful: %s", pluginName.c_str(), e.what());
+                update_client("Reporting via '%s' was not successful: %s", pluginName.c_str(), e.what());
             }
         }
     }
@@ -764,7 +772,7 @@ vector_pair_string_string_t GetUUIDsOfCrash(const char *pUID)
     database->DisConnect();
 
     vector_pair_string_string_t UUIDsUIDs;
-    int ii;
+    unsigned ii;
     for (ii = 0; ii < rows.size(); ii++)
     {
         UUIDsUIDs.push_back(make_pair(rows[ii].m_sUUID, rows[ii].m_sUID));
@@ -783,5 +791,6 @@ void AddAnalyzerActionOrReporter(const char *pAnalyzer,
 void AddActionOrReporter(const char *pActionOrReporter,
                                       const char *pArgs)
 {
+    VERB3 log("AddActionOrReporter('%s','%s')", pActionOrReporter, pArgs);
     s_vectorActionsAndReporters.push_back(make_pair(std::string(pActionOrReporter), std::string(pArgs)));
 }
diff --git a/src/Daemon/PluginManager.cpp b/src/Daemon/PluginManager.cpp
index 5b871515..21a66adf 100644
--- a/src/Daemon/PluginManager.cpp
+++ b/src/Daemon/PluginManager.cpp
@@ -278,13 +278,11 @@ CAnalyzer* CPluginManager::GetAnalyzer(const std::string& pName)
     map_plugins_t::iterator plugin = m_mapPlugins.find(pName);
     if (plugin == m_mapPlugins.end())
     {
-        throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetAnalyzer():"
-                                            "Analyzer plugin: '"+pName+"' is not registered.");
+        throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not registered");
     }
     if (m_mapABRTPlugins[pName]->GetType() != ANALYZER)
     {
-        throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetAnalyzer():"
-                                            "Plugin: '"+pName+"' is not analyzer plugin.");
+        throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not an analyzer plugin");
     }
     return (CAnalyzer*)(plugin->second);
 }
@@ -294,13 +292,11 @@ CReporter* CPluginManager::GetReporter(const std::string& pName)
     map_plugins_t::iterator plugin = m_mapPlugins.find(pName);
     if (plugin == m_mapPlugins.end())
     {
-        throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetReporter():"
-                                           "Reporter plugin: '"+pName+"' is not registered.");
+        throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not registered");
     }
     if (m_mapABRTPlugins[pName]->GetType() != REPORTER)
     {
-        throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetReporter():"
-                                            "Plugin: '"+pName+"' is not reporter plugin.");
+        throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not a reporter plugin");
     }
     return (CReporter*)(plugin->second);
 }
@@ -310,13 +306,11 @@ CAction* CPluginManager::GetAction(const std::string& pName)
     map_plugins_t::iterator plugin = m_mapPlugins.find(pName);
     if (plugin == m_mapPlugins.end())
     {
-        throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetAction():"
-                                           "Action plugin: '"+pName+"' is not registered.");
+        throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not registered");
     }
     if (m_mapABRTPlugins[pName]->GetType() != ACTION)
     {
-        throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetAction():"
-                                            "Plugin: '"+pName+"' is not action plugin.");
+        throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not an action plugin");
     }
     return (CAction*)(plugin->second);
 }
@@ -326,13 +320,11 @@ CDatabase* CPluginManager::GetDatabase(const std::string& pName)
     map_plugins_t::iterator plugin = m_mapPlugins.find(pName);
     if (plugin == m_mapPlugins.end())
     {
-        throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetDatabase():"
-                                           "Database plugin: '"+pName+"' is not registered.");
+        throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not registered");
     }
     if (m_mapABRTPlugins[pName]->GetType() != DATABASE)
     {
-        throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetDatabase():"
-                                            "Plugin: '"+pName+"' is not database plugin.");
+        throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not a database plugin");
     }
     return (CDatabase*)(plugin->second);
 }
@@ -342,8 +334,7 @@ plugin_type_t CPluginManager::GetPluginType(const std::string& pName)
     map_plugins_t::iterator plugin = m_mapPlugins.find(pName);
     if (plugin == m_mapPlugins.end())
     {
-        throw CABRTException(EXCEP_PLUGIN, "CPluginManager::GetPluginType():"
-                                           "Plugin: '"+pName+"' is not registered.");
+        throw CABRTException(EXCEP_PLUGIN, "Plugin '"+pName+"' is not registered");
     }
     return m_mapABRTPlugins[pName]->GetType();
 }
diff --git a/src/Daemon/Settings.cpp b/src/Daemon/Settings.cpp
index cefd35a4..7f419ec5 100644
--- a/src/Daemon/Settings.cpp
+++ b/src/Daemon/Settings.cpp
@@ -228,6 +228,7 @@ static void ParseAnalyzerActionsAndReporters()
         set_string_t::iterator it_keys = keys.begin();
         for (; it_keys != keys.end(); it_keys++)
         {
+            VERB2 log("AnalyzerActionsAndReporters['%s']=...", it_keys->c_str());
             g_settings_mapAnalyzerActionsAndReporters[*it_keys] = actionsAndReporters;
         }
     }
diff --git a/src/Daemon/abrt.conf b/src/Daemon/abrt.conf
index e9845e58..f24ec4a8 100644
--- a/src/Daemon/abrt.conf
+++ b/src/Daemon/abrt.conf
@@ -10,20 +10,21 @@ OpenGPGPublicKeys = /etc/pki/rpm-gpg/RPM-GPG-KEY-fedora
 # Blacklisted packages
 BlackList = nspluginwrapper
 # Enabled plugins. There has to be exactly one database plugin
-EnabledPlugins = SQLite3, CCpp, Logger, Kerneloops, KerneloopsScanner, KerneloopsReporter, Bugzilla, Python
+EnabledPlugins = SQLite3, CCpp, Logger, Kerneloops, KerneloopsScanner, KerneloopsReporter, Bugzilla, Python, RunApp
 # Database
 Database = SQLite3
 # Max size for crash storage [MiB]
 MaxCrashReportsSize = 1000
 # Vector of actions and reporters which are activated immediately after a crash occurs
-# ActionsAndReporters = Mailx("[abrt] new crash was detected")
+#ActionsAndReporters = Mailx("[abrt] new crash was detected")
+# TODO: teach parser to escape \". Should be: x"`cat component`" = x"xorg-x11-server-Xorg"
+ActionsAndReporters = RunApp("test x`cat component` = xxorg-x11-server-Xorg && cp /var/log/Xorg*.log .")
 
 # Reporters association with analyzers
 [ AnalyzerActionsAndReporters ]
 Kerneloops = KerneloopsReporter
 CCpp = Bugzilla, Logger
 Python = Bugzilla, Logger
-# CCpp : xorg-x11-apps = RunApp("date", "RunApp")
 
 # Repeated calling of Action plugins
 [ Cron ]