Merge branch 'master' of ssh://git.fedorahosted.org/git/abrt

Conflicts:
	lib/CommLayer/DBusServerProxy.h

2 files changed, 97 insertions, 105 deletions

diff --git a/src/Hooks/CCpp.cpp b/src/Hooks/CCpp.cpp
index 6a8b1d63..b5b576b8 100644
--- a/src/Hooks/CCpp.cpp
+++ b/src/Hooks/CCpp.cpp
@@ -22,14 +22,6 @@
 
 #include "DebugDump.h"
 #include "ABRTException.h"
-//#include <stdlib.h>
-//#include <string.h>
-//#include <limits.h>
-//#include <stdio.h>
-//#include <sys/types.h>
-//#include <sys/stat.h>
-//#include <unistd.h>
-//#include <time.h>
 #include <syslog.h>
 #include <string>
 
@@ -39,32 +31,17 @@
 
 #define VAR_RUN_PID_FILE         VAR_RUN"/abrt.pid"
 
-static void write_success_log(const char* pid)
+static char* get_executable(pid_t pid)
 {
-    openlog("abrt", 0, LOG_DAEMON);
-    syslog(LOG_WARNING, "CCpp Language Hook: Crashed pid: %s", pid);
-    closelog();
-}
-
-static void write_faliure_log(const char* msg)
-{
-    openlog("abrt", 0, LOG_DAEMON);
-    syslog(LOG_WARNING, "CCpp Language Hook: Exception occur: %s", msg);
-    closelog();
-}
-
-
-char* get_executable(const char* pid)
-{
-    char path[PATH_MAX];
-    char executable[PATH_MAX];
+    char buf[PATH_MAX + 1];
     int len;
 
-    snprintf(path, sizeof(path), "/proc/%s/exe", pid);
-    if ((len = readlink(path, executable, PATH_MAX)) != -1)
+    snprintf(buf, sizeof(buf), "/proc/%u/exe", (int)pid);
+    len = readlink(buf, buf, sizeof(buf)-1);
+    if (len >= 0)
     {
-        executable[len] = '\0';
-        return xstrdup(executable);
+        buf[len] = '\0';
+        return xstrdup(buf);
     }
     return NULL;
 }
@@ -72,49 +49,62 @@ char* get_executable(const char* pid)
 // taken from kernel
 #define COMMAND_LINE_SIZE 2048
 
-char* get_cmdline(const char* pid)
+static char* get_cmdline(pid_t pid)
 {
     char path[PATH_MAX];
     char cmdline[COMMAND_LINE_SIZE];
-    snprintf(path, sizeof(path), "/proc/%s/cmdline", pid);
-    FILE* fp = fopen(path, "r");
-    int ch;
-    int ii = 0;
-    if (fp)
+    snprintf(path, sizeof(path), "/proc/%u/cmdline", (int)pid);
+    int dst = 0;
+
+    int fd = open(path, O_RDONLY);
+    if (fd >= 0)
     {
-        while ((ch = fgetc(fp)) != EOF && ii < COMMAND_LINE_SIZE-1)
+        int len = read(fd, cmdline, sizeof(cmdline) - 1);
+        if (len >= 0)
         {
-            if (ch == 0)
+            int src = 0;
+            while (src < len)
             {
-                cmdline[ii] = ' ';
+                char ch = cmdline[src++];
+                if (ch == '\0')
+                {
+                    cmdline[dst++] = ' ';
+                }
+                /* TODO: maybe just ch >= ' '? */
+                else if (isspace(ch) || (isascii(ch) && !iscntrl(ch)))
+                {
+                    cmdline[dst++] = ch;
+                }
             }
-            else if (isspace(ch) || (isascii(ch) && !iscntrl(ch)))
-            {
-                cmdline[ii] = ch;
-            }
-            ii++;
         }
-        fclose(fp);
+        close(fd);
     }
-    cmdline[ii] = '\0';
+    cmdline[dst] = '\0';
+
     return xstrdup(cmdline);
 }
 
-#define PID_MAX                 16
-
-int daemon_is_ok()
+static int daemon_is_ok()
 {
-    char pid[PID_MAX];
+    char pid[sizeof(pid_t)*3 + 2];
     char path[PATH_MAX];
     struct stat buff;
-    FILE* fp = fopen(VAR_RUN_PID_FILE, "r");
-    if (fp == NULL)
+    int fd = open(VAR_RUN_PID_FILE, O_RDONLY);
+    if (fd < 0)
     {
         return 0;
     }
-    fgets(pid, sizeof(pid), fp);
-    fclose(fp);
+    int len = read(fd, pid, sizeof(pid)-1);
+    close(fd);
+    if (len <= 0)
+        return 0;
+    pid[len] = '\0';
     *strchrnul(pid, '\n') = '\0';
+    /* paranoia: we don't want to check /proc//stat or /proc///stat */
+    if (pid[0] == '\0' || pid[0] == '/')
+        return 0;
+
+    /* TODO: maybe readlink and check that it is "xxx/abrt"? */
     snprintf(path, sizeof(path), "/proc/%s/stat", pid);
     if (stat(path, &buff) == -1)
     {
@@ -129,27 +119,33 @@ int main(int argc, char** argv)
     const char* program_name = argv[0];
     if (argc < 4)
     {
-        fprintf(stderr, "Usage: %s: <dddir> <pid> <signal> <uid>\n",
-                program_name);
-        return -1;
+        error_msg_and_die("Usage: %s: <dddir> <pid> <signal> <uid>", program_name);
     }
+    openlog("abrt", 0, LOG_DAEMON);
     logmode = LOGMODE_SYSLOG;
 
     const char* dddir = argv[1];
-    const char* pid = argv[2];
-    const char* signal = argv[3];
-    const char* uid = argv[4];
-
-    if (strcmp(signal, "3") != 0 &&     // SIGQUIT
-        strcmp(signal, "4") != 0 &&     // SIGILL
-        strcmp(signal, "6") != 0 &&     // SIGABRT
-        strcmp(signal, "8") != 0 &&     // SIGFPE
-        strcmp(signal, "11") != 0)      // SIGSEGV
-    {
+    pid_t pid = atoi(argv[2]);
+    const char* signal_str = argv[3];
+    int signal = atoi(argv[3]);
+    uid_t uid = atoi(argv[4]);
+
+    if (signal != SIGQUIT
+     && signal != SIGILL
+     && signal != SIGABRT
+     && signal != SIGFPE
+     && signal != SIGSEGV
+    ) {
+        /* not an error, exit silently */
         return 0;
     }
+    if (pid <= 0 || uid < 0)
+    {
+        error_msg_and_die("pid '%s' or uid '%s' are bogus", argv[2], argv[4]);
+    }
     if (!daemon_is_ok())
     {
+        /* not an error, exit with exitcode 0 */
         log("abrt daemon is not running. If it crashed, "
             "/proc/sys/kernel/core_pattern contains a stale value, "
             "consider resetting it to 'core'"
@@ -159,68 +155,64 @@ int main(int argc, char** argv)
 
     try
     {
-        FILE* fp;
-        CDebugDump dd;
-        int byte;
-        char path[PATH_MAX];
-        char* executable = NULL;
-        char* cmdline = NULL;
-
+        char* executable;
+        char* cmdline;
         executable = get_executable(pid);
         cmdline = get_cmdline(pid);
-
-        if (executable == NULL ||
-            cmdline == NULL)
+        if (executable == NULL || cmdline == NULL)
+        {
+            error_msg_and_die("can not get proc info for pid %u", (int)pid);
+        }
+        if (strstr(executable, "/abrt"))
         {
-            free(executable);
-            free(cmdline);
-            throw CABRTException(EXCEP_FATAL, "Can not get proc info.");
+            /* free(executable); - why bother? */
+            /* free(cmdline); */
+            error_msg_and_die("pid %u is '%s', not dumping it to avoid abrt recursion",
+                            (int)pid, executable);
         }
 
-        snprintf(path, sizeof(path), "%s/ccpp-%ld-%s", dddir, time(NULL), pid);
+        char path[PATH_MAX];
+        snprintf(path, sizeof(path), "%s/ccpp-%ld-%u", dddir, (long)time(NULL), (int)pid);
+
+        CDebugDump dd;
         dd.Create(path, uid);
         dd.SaveText(FILENAME_ANALYZER, "CCpp");
         dd.SaveText(FILENAME_EXECUTABLE, executable);
         dd.SaveText(FILENAME_CMDLINE, cmdline);
-        dd.SaveText(FILENAME_REASON, std::string("Process was terminated by signal ") + signal);
+        dd.SaveText(FILENAME_REASON, std::string("Process was terminated by signal ") + signal_str);
 
         snprintf(path + strlen(path), sizeof(path), "/%s", FILENAME_COREDUMP);
 
-        if ((fp = fopen(path, "w")) == NULL)
+        int fd;
+        fd = open(path, O_WRONLY | O_CREAT | O_TRUNC, 0600);
+        if (fd < 0)
         {
             dd.Delete();
             dd.Close();
-            throw CABRTException(EXCEP_FATAL, std::string("Can not open the file ") + path);
+            perror_msg_and_die("can't open '%s'", path);
         }
-        // TODO: rewrite this
-        while ((byte = getc(stdin)) != EOF)
+        if (copyfd_eof(STDIN_FILENO, fd) < 0)
         {
-            if (putc(byte, fp) == EOF)
-            {
-                fclose(fp);
-                dd.Delete();
-                dd.Close();
-                throw CABRTException(EXCEP_FATAL, "Can not write to the file %s.");
-            }
+            /* close(fd); - why bother? */
+            dd.Delete();
+            dd.Close();
+            /* copyfd_eof logs the error including errno string,
+             * but it does not log file name */
+            error_msg_and_die("error saving coredump to %s", path);
         }
-
-        free(executable);
-        free(cmdline);
-        fclose(fp);
+        /* close(fd); - why bother? */
+        /* free(executable); */
+        /* free(cmdline); */
         dd.Close();
-        write_success_log(pid);
+        log("saved core dump of pid %u to %s", (int)pid, path);
     }
     catch (CABRTException& e)
     {
-        fprintf(stderr, "%s: %s\n", program_name, e.what().c_str());
-        write_faliure_log(e.what().c_str());
-        return -2;
+        error_msg_and_die("%s", e.what().c_str());
     }
     catch (std::exception& e)
     {
-        fprintf(stderr, "%s: %s\n", program_name, e.what());
-        write_faliure_log(e.what());
-        return -2;
+        error_msg_and_die("%s", e.what());
     }
     return 0;
 }
diff --git a/src/Hooks/abrt_exception_handler.py.in b/src/Hooks/abrt_exception_handler.py.in
index 148573d9..f4ebbb56 100644
--- a/src/Hooks/abrt_exception_handler.py.in
+++ b/src/Hooks/abrt_exception_handler.py.in
@@ -200,7 +200,7 @@ def handleMyException((etype, value, tb)):
     progname - the name of the application
     version  - the version of the application
     """
-    
+
     # restore original exception handler
     sys.excepthook = sys.__excepthook__  # pylint: disable-msg=E1101