simplify C/C++ hook

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

1 files changed, 48 insertions, 75 deletions

diff --git a/src/Hooks/CCpp.cpp b/src/Hooks/CCpp.cpp
index 6a8b1d63..e8c35edd 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(const char* 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/%s/exe", 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,7 +49,7 @@ char* get_executable(const char* pid)
 // taken from kernel
 #define COMMAND_LINE_SIZE 2048
 
-char* get_cmdline(const char* pid)
+static char* get_cmdline(const char* pid)
 {
     char path[PATH_MAX];
     char cmdline[COMMAND_LINE_SIZE];
@@ -100,21 +77,27 @@ char* get_cmdline(const char* pid)
     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,10 +112,9 @@ 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];
@@ -159,25 +141,22 @@ int main(int argc, char** argv)
 
     try
     {
-        FILE* fp;
+        int fd;
         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)
         {
-            free(executable);
-            free(cmdline);
-            throw CABRTException(EXCEP_FATAL, "Can not get proc info.");
+            /* free(executable); - why bother? */
+            /* free(cmdline); */
+            error_msg_and_die("can not get proc info for pid %s", pid);
         }
 
-        snprintf(path, sizeof(path), "%s/ccpp-%ld-%s", dddir, time(NULL), pid);
+        snprintf(path, sizeof(path), "%s/ccpp-%ld-%s", dddir, (long)time(NULL), pid);
         dd.Create(path, uid);
         dd.SaveText(FILENAME_ANALYZER, "CCpp");
         dd.SaveText(FILENAME_EXECUTABLE, executable);
@@ -186,41 +165,35 @@ int main(int argc, char** argv)
 
         snprintf(path + strlen(path), sizeof(path), "/%s", FILENAME_COREDUMP);
 
-        if ((fp = fopen(path, "w")) == NULL)
+        fd = open(path, O_WRONLY | O_CREAT | O_TRUNC);
+        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 %s to %s", 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;
 }