move logger reporting to a separate program (abrt-action-print)

Usage: abrt-action-print [v] -d DIR

Print information about the crash to standard output

    -v, --verbose         be verbose
    -d DIR                Crash dump directory

As you can see, it simply writes crash dump information
to stdout, not to a file.

Logger plugin spawns a child abrt-action-print -d DIR
and pipes its output to log file.

Signed-off-by: Denys Vlasenko <dvlasenk@redhat.com>

2 files changed, 73 insertions, 29 deletions

diff --git a/lib/plugins/Logger.cpp b/lib/plugins/Logger.cpp
index e3ba9660..2985208e 100644
--- a/lib/plugins/Logger.cpp
+++ b/lib/plugins/Logger.cpp
@@ -1,5 +1,5 @@
 /*
-    Logger.cpp - it simple writes report to specific file
+    Logger.cpp - it simply writes report to a specified file
 
     Copyright (C) 2009  Zdenek Prikryl (zprikryl@redhat.com)
     Copyright (C) 2009  RedHat inc.
@@ -19,60 +19,107 @@
     51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 */
 #include "abrtlib.h"
-#include "Logger.h"
 #include "comm_layer_inner.h"
 #include "abrt_exception.h"
+#include "Logger.h"
+
+using namespace std;
 
 CLogger::CLogger()
 {
-    m_log_path = xstrdup("/var/log/abrt.log");
-    m_append_logs = true;
+    m_pSettings["LogPath"] = "/var/log/abrt.log";
+    m_pSettings["AppendLogs"] = "yes";
 }
 
 CLogger::~CLogger()
 {
-    free(m_log_path);
 }
 
 void CLogger::SetSettings(const map_plugin_settings_t& pSettings)
 {
+    /* Can't simply do this:
+
     m_pSettings = pSettings;
 
-    map_plugin_settings_t::const_iterator end = pSettings.end();
-    map_plugin_settings_t::const_iterator it;
-    it = pSettings.find("LogPath");
-    if (it != end)
+     * - it will erase keys which aren't present in pSettings.
+     * Example: if Bugzilla.conf doesn't have "Login = foo",
+     * then there's no pSettings["Login"] and m_pSettings = pSettings
+     * will nuke default m_pSettings["Login"] = "",
+     * making GUI think that we have no "Login" key at all
+     * and thus never overriding it - even if it *has* an override!
+     */
+
+    map_plugin_settings_t::iterator it = m_pSettings.begin();
+    while (it != m_pSettings.end())
     {
-        free(m_log_path);
-        m_log_path = xstrdup(it->second.c_str());
+        map_plugin_settings_t::const_iterator override = pSettings.find(it->first);
+        if (override != pSettings.end())
+        {
+            VERB3 log(" logger settings[%s]='%s'", it->first.c_str(), it->second.c_str());
+            it->second = override->second;
+        }
+        it++;
     }
-    it = pSettings.find("AppendLogs");
-    if (it != end)
-        m_append_logs = string_to_bool(it->second.c_str());
 }
 
-std::string CLogger::Report(const map_crash_data_t& pCrashData,
+string CLogger::Report(const map_crash_data_t& crash_data,
                 const map_plugin_settings_t& pSettings,
                 const char *pArgs)
 {
-    char *dsc = make_description_logger(pCrashData);
-    char *full_dsc = xasprintf("%s\n\n\n", dsc);
-    free(dsc);
+    const char *log_path = "/var/log/abrt.log";
+    bool append_logs = true;
+
+    map_plugin_settings_t::const_iterator end = pSettings.end();
+    map_plugin_settings_t::const_iterator it;
+    it = pSettings.find("LogPath");
+    if (it != end)
+        log_path = it->second.c_str();
+    it = pSettings.find("AppendLogs");
+    if (it != end)
+        append_logs = string_to_bool(it->second.c_str());
 
     /* open, not fopen - want to set mode if we create the file, not just open */
-    const char *fname = m_log_path;
-    int fd = open(fname, m_append_logs ? O_WRONLY|O_CREAT|O_APPEND : O_WRONLY|O_CREAT|O_TRUNC, 0600);
+    int fd = open(log_path, append_logs ? O_WRONLY|O_CREAT|O_APPEND : O_WRONLY|O_CREAT|O_TRUNC, 0600);
     if (fd < 0)
-        throw CABRTException(EXCEP_PLUGIN, "Can't open '%s'", fname);
+        throw CABRTException(EXCEP_PLUGIN, "Can't open '%s'", log_path);
+
+    update_client(_("Writing report to '%s'"), log_path);
+
+    /* abrt-action-print -d DIR NULL */
+    char *argv[4];
+    char **pp = argv;
+    *pp++ = (char*)"abrt-action-print";
+    *pp++ = (char*)"-d";
+    *pp++ = (char*)get_crash_data_item_content_or_NULL(crash_data, CD_DUMPDIR);
+    *pp = NULL;
+    int pipefds[2];
+    pid_t pid = fork_execv_on_steroids(EXECFLG_OUTPUT, // + EXECFLG_ERR2OUT,
+                argv,
+                pipefds,
+                /* unsetenv_vec: */ NULL,
+                /* dir: */ NULL,
+                /* uid(unused): */ 0
+    );
 
-    update_client(_("Writing report to '%s'"), fname);
-    full_write_str(fd, full_dsc);
-    free(full_dsc);
+    /* Consume log from stdout, write it to log file */
+    FILE *fp = fdopen(pipefds[0], "r");
+    if (!fp)
+        die_out_of_memory();
+    char buf[512];
+    while (fgets(buf, sizeof(buf), fp))
+    {
+	full_write_str(fd, buf);
+    }
+    fclose(fp); /* this also closes pipefds[0] */
+    /* wait for child to actually exit, and prevent leaving a zombie behind */
+    waitpid(pid, NULL, 0);
 
+    /* Add separating empty lines, then close log file */
+    full_write_str(fd, "\n\n\n");
     close(fd);
 
-    const char *format = m_append_logs ? _("The report was appended to %s") : _("The report was stored to %s");
-    return ssprintf(format, m_log_path);
+    const char *format = append_logs ? _("The report was appended to %s") : _("The report was stored to %s");
+    return ssprintf(format, log_path);
 }
 
 PLUGIN_INFO(REPORTER,
diff --git a/lib/plugins/Logger.h b/lib/plugins/Logger.h
index d2b80075..afe1b7c2 100644
--- a/lib/plugins/Logger.h
+++ b/lib/plugins/Logger.h
@@ -27,9 +27,6 @@
 
 class CLogger : public CReporter
 {
-    private:
-        char *m_log_path;
-        bool m_append_logs;
     public:
         CLogger();
         ~CLogger();