get rid of exceptions in CDebugDump class

4 files changed, 175 insertions, 169 deletions

diff --git a/src/daemon/MiddleWare.cpp b/src/daemon/MiddleWare.cpp
index c7ed4df5..bbb485f9 100644
--- a/src/daemon/MiddleWare.cpp
+++ b/src/daemon/MiddleWare.cpp
@@ -167,24 +167,30 @@ static void load_crash_data_from_debug_dump(CDebugDump& dd, map_crash_data_t& da
  * @param pDebugDumpDir A debugdump dir containing all necessary data.
  * @param pCrashData A created crash report.
  */
-static void DebugDumpToCrashReport(const char *pDebugDumpDir, map_crash_data_t& pCrashData)
+static bool DebugDumpToCrashReport(const char *pDebugDumpDir, map_crash_data_t& pCrashData)
 {
     VERB3 log(" DebugDumpToCrashReport('%s')", pDebugDumpDir);
 
     CDebugDump dd;
-    dd.Open(pDebugDumpDir);
-
-    const char *const *v = must_have_files;
-    while (*v)
+    if (dd.Open(pDebugDumpDir))
     {
-        if (!dd.Exist(*v))
+        const char *const *v = must_have_files;
+        while (*v)
         {
-            throw CABRTException(EXCEP_ERROR, "DebugDumpToCrashReport(): important file '%s' is missing", *v);
+            if (!dd.Exist(*v))
+            {
+                throw CABRTException(EXCEP_ERROR, "DebugDumpToCrashReport(): important file '%s' is missing", *v);
+            }
+            v++;
         }
-        v++;
+
+        load_crash_data_from_debug_dump(dd, pCrashData);
+        dd.Close();
+
+        return true;
     }
 
-    load_crash_data_from_debug_dump(dd, pCrashData);
+    return false;
 }
 
 /**
@@ -273,11 +279,14 @@ mw_result_t CreateCrashReport(const char *crash_id,
     mw_result_t r = MW_OK;
     try
     {
+        CDebugDump dd;
+        if (dd.Open(row.m_sDebugDumpDir.c_str()))
         {
-            CDebugDump dd;
-            dd.Open(row.m_sDebugDumpDir.c_str());
             load_crash_data_from_debug_dump(dd, pCrashData);
+            dd.Close();
         }
+        else
+            return MW_ERROR;
 
         std::string analyzer = get_crash_data_item_content(pCrashData, FILENAME_ANALYZER);
         const char* package = get_crash_data_item_content_or_NULL(pCrashData, FILENAME_PACKAGE);
@@ -297,23 +306,22 @@ mw_result_t CreateCrashReport(const char *crash_id,
         VERB3 log(" RunAnalyzerActions('%s','%s','%s',force=%d)", analyzer.c_str(), package_name, row.m_sDebugDumpDir.c_str(), force);
         RunAnalyzerActions(analyzer.c_str(), package_name, row.m_sDebugDumpDir.c_str(), force);
         free(package_name);
-        DebugDumpToCrashReport(row.m_sDebugDumpDir.c_str(), pCrashData);
-        add_to_crash_data_ext(pCrashData, CD_UUID   , CD_SYS, CD_ISNOTEDITABLE, row.m_sUUID.c_str());
-        add_to_crash_data_ext(pCrashData, CD_DUPHASH, CD_TXT, CD_ISNOTEDITABLE, dup_hash.c_str());
+        if (DebugDumpToCrashReport(row.m_sDebugDumpDir.c_str(), pCrashData))
+        {
+            add_to_crash_data_ext(pCrashData, CD_UUID   , CD_SYS, CD_ISNOTEDITABLE, row.m_sUUID.c_str());
+            add_to_crash_data_ext(pCrashData, CD_DUPHASH, CD_TXT, CD_ISNOTEDITABLE, dup_hash.c_str());
+        }
+        else
+        {
+            error_msg("Error loading crash data");
+            return MW_ERROR;
+        }
     }
     catch (CABRTException& e)
     {
         r = MW_CORRUPTED;
         error_msg("%s", e.what());
-        if (e.type() == EXCEP_DD_OPEN)
-        {
-            r = MW_ERROR;
-        }
-        else if (e.type() == EXCEP_DD_LOAD)
-        {
-            r = MW_FILE_ERROR;
-        }
-        else if (e.type() == EXCEP_PLUGIN)
+        if (e.type() == EXCEP_PLUGIN)
         {
             r = MW_PLUGIN_ERROR;
         }
@@ -358,9 +366,13 @@ void RunActionsAndReporters(const char *pDebugDumpDir)
             {
                 CReporter* reporter = g_pPluginManager->GetReporter(plugin_name); /* can't be NULL */
                 map_crash_data_t crashReport;
-                DebugDumpToCrashReport(pDebugDumpDir, crashReport);
-                VERB2 log("%s.Report(...)", plugin_name);
-                reporter->Report(crashReport, plugin_settings, it_ar->second.c_str());
+                if (DebugDumpToCrashReport(pDebugDumpDir, crashReport))
+                {
+                    VERB2 log("%s.Report(...)", plugin_name);
+                    reporter->Report(crashReport, plugin_settings, it_ar->second.c_str());
+                }
+                else
+                    error_msg("Activation of plugin '%s' was not successful: Error converting crash data", plugin_name);
             }
             else if (tp == ACTION)
             {
@@ -421,21 +433,25 @@ report_status_t Report(const map_crash_data_t& client_report,
     if (comment || reproduce || backtrace)
     {
         CDebugDump dd;
-        dd.Open(pDumpDir.c_str());
-        if (comment)
-        {
-            dd.SaveText(FILENAME_COMMENT, comment);
-            add_to_crash_data_ext(stored_report, FILENAME_COMMENT, CD_TXT, CD_ISEDITABLE, comment);
-        }
-        if (reproduce)
+        if (dd.Open(pDumpDir.c_str()))
         {
-            dd.SaveText(FILENAME_REPRODUCE, reproduce);
-            add_to_crash_data_ext(stored_report, FILENAME_REPRODUCE, CD_TXT, CD_ISEDITABLE, reproduce);
-        }
-        if (backtrace)
-        {
-            dd.SaveText(FILENAME_BACKTRACE, backtrace);
-            add_to_crash_data_ext(stored_report, FILENAME_BACKTRACE, CD_TXT, CD_ISEDITABLE, backtrace);
+            if (comment)
+            {
+                dd.SaveText(FILENAME_COMMENT, comment);
+                add_to_crash_data_ext(stored_report, FILENAME_COMMENT, CD_TXT, CD_ISEDITABLE, comment);
+            }
+            if (reproduce)
+            {
+                dd.SaveText(FILENAME_REPRODUCE, reproduce);
+                add_to_crash_data_ext(stored_report, FILENAME_REPRODUCE, CD_TXT, CD_ISEDITABLE, reproduce);
+            }
+            if (backtrace)
+            {
+                dd.SaveText(FILENAME_BACKTRACE, backtrace);
+                add_to_crash_data_ext(stored_report, FILENAME_BACKTRACE, CD_TXT, CD_ISEDITABLE, backtrace);
+            }
+
+            dd.Close();
         }
     }
 
@@ -690,20 +706,19 @@ static mw_result_t SavePackageDescriptionToDebugDump(
             if (g_settings_bProcessUnpackaged || remote)
             {
                 VERB2 log("Crash in unpackaged executable '%s', proceeding without packaging information", pExecutable);
-                try
+
+                CDebugDump dd;
+                if (dd.Open(pDebugDumpDir))
                 {
-                    CDebugDump dd;
-                    dd.Open(pDebugDumpDir);
                     dd.SaveText(FILENAME_PACKAGE, "");
                     dd.SaveText(FILENAME_COMPONENT, "");
                     dd.SaveText(FILENAME_DESCRIPTION, "Crashed executable does not belong to any installed package");
+
+                    dd.Close();
                     return MW_OK;
                 }
-                catch (CABRTException& e)
-                {
-                    error_msg("%s", e.what());
+                else
                     return MW_ERROR;
-                }
             }
             else
             {
@@ -816,10 +831,9 @@ static mw_result_t SavePackageDescriptionToDebugDump(
         }
     }
 
-    try
+    CDebugDump dd;
+    if (dd.Open(pDebugDumpDir))
     {
-        CDebugDump dd;
-        dd.Open(pDebugDumpDir);
         if (rpm_pkg)
         {
             dd.SaveText(FILENAME_PACKAGE, rpm_pkg);
@@ -840,14 +854,11 @@ static mw_result_t SavePackageDescriptionToDebugDump(
 
         if (!remote)
             dd.SaveText(FILENAME_HOSTNAME, host);
-    }
-    catch (CABRTException& e)
-    {
-        error_msg("%s", e.what());
-        return MW_ERROR;
+
+        return MW_OK;
     }
 
-    return MW_OK;
+    return MW_ERROR;
 }
 
 bool analyzer_has_InformAllUsers(const char *analyzer_name)
@@ -1008,10 +1019,10 @@ mw_result_t SaveDebugDump(const char *pDebugDumpDir,
     std::string executable;
     std::string cmdline;
     bool remote = false;
-    try
+
+    CDebugDump dd;
+    if (dd.Open(pDebugDumpDir))
     {
-        CDebugDump dd;
-        dd.Open(pDebugDumpDir);
         dd.LoadText(FILENAME_TIME, time);
         dd.LoadText(CD_UID, UID);
         dd.LoadText(FILENAME_ANALYZER, analyzer);
@@ -1023,12 +1034,11 @@ mw_result_t SaveDebugDump(const char *pDebugDumpDir,
             dd.LoadText(FILENAME_REMOTE, remote_str);
             remote = (remote_str.find('1') != std::string::npos);
         }
+
+        dd.Close();
     }
-    catch (CABRTException& e)
-    {
-        error_msg("%s", e.what());
+    else
         return MW_ERROR;
-    }
 
     /* Convert UID string to number uid_num. The UID string can be modified by user or
        wrongly saved (empty or non-numeric), so xatou() cannot be used here,
@@ -1077,17 +1087,15 @@ mw_result_t FillCrashInfo(const char *crash_id,
     std::string executable;
     std::string description;
     std::string analyzer;
-    try
+
+    CDebugDump dd;
+    if (dd.Open(row.m_sDebugDumpDir.c_str()))
     {
-        CDebugDump dd;
-        dd.Open(row.m_sDebugDumpDir.c_str());
         load_crash_data_from_debug_dump(dd, pCrashData);
+        dd.Close();
     }
-    catch (CABRTException& e)
-    {
-        error_msg("%s", e.what());
+    else
         return MW_ERROR;
-    }
 
     add_to_crash_data(pCrashData, CD_UID              , row.m_sUID.c_str()         );
     add_to_crash_data(pCrashData, CD_UUID             , row.m_sUUID.c_str()        );
diff --git a/src/daemon/dumpsocket.cpp b/src/daemon/dumpsocket.cpp
index 699a0609..21421b81 100644
--- a/src/daemon/dumpsocket.cpp
+++ b/src/daemon/dumpsocket.cpp
@@ -177,12 +177,11 @@ static void create_debug_dump(struct client *client)
        fails if the path is too long. */
 
     CDebugDump dd;
-    try {
-        dd.Create(path, client->uid);
-    } catch (CABRTException &e) {
+    if (!dd.Create(path, client->uid))
+    {
         dd.Delete();
         dd.Close();
-        error_msg_and_die("dumpsocket: Error while creating crash dump %s: %s", path, e.what());
+        error_msg_and_die("dumpsocket: Error while creating crash dump %s", path);
     }
 
     dd.SaveText(FILENAME_ANALYZER, client->analyzer);
diff --git a/src/hooks/abrt-hook-ccpp.cpp b/src/hooks/abrt-hook-ccpp.cpp
index e53007a4..1e469890 100644
--- a/src/hooks/abrt-hook-ccpp.cpp
+++ b/src/hooks/abrt-hook-ccpp.cpp
@@ -416,112 +416,112 @@ int main(int argc, char** argv)
             return 1;
 
         CDebugDump dd;
-        char *cmdline = get_cmdline(pid); /* never NULL */
-        char *reason = xasprintf("Process %s was killed by signal %s (SIG%s)", executable, signal_str, signame ? signame : signal_str);
-        dd.Create(path, uid);
-        dd.SaveText(FILENAME_ANALYZER, "CCpp");
-        dd.SaveText(FILENAME_EXECUTABLE, executable);
-        dd.SaveText(FILENAME_CMDLINE, cmdline);
-        dd.SaveText(FILENAME_REASON, reason);
-        free(cmdline);
-        free(reason);
-
-        if (src_fd_binary > 0)
+        if (dd.Create(path, uid))
         {
-            strcpy(path + path_len, "/"FILENAME_BINARY);
-            int dst_fd_binary = xopen3(path, O_WRONLY | O_CREAT | O_TRUNC, 0600);
-            off_t sz = copyfd_eof(src_fd_binary, dst_fd_binary, COPYFD_SPARSE);
-            if (sz < 0 || fsync(dst_fd_binary) != 0)
+            char *cmdline = get_cmdline(pid); /* never NULL */
+            char *reason = xasprintf("Process %s was killed by signal %s (SIG%s)", executable, signal_str, signame ? signame : signal_str);
+            dd.SaveText(FILENAME_ANALYZER, "CCpp");
+            dd.SaveText(FILENAME_EXECUTABLE, executable);
+            dd.SaveText(FILENAME_CMDLINE, cmdline);
+            dd.SaveText(FILENAME_REASON, reason);
+            free(cmdline);
+            free(reason);
+
+            if (src_fd_binary > 0)
             {
-                unlink(path);
-                error_msg_and_die("error saving binary image to %s", path);
+                strcpy(path + path_len, "/"FILENAME_BINARY);
+                int dst_fd_binary = xopen3(path, O_WRONLY | O_CREAT | O_TRUNC, 0600);
+                off_t sz = copyfd_eof(src_fd_binary, dst_fd_binary, COPYFD_SPARSE);
+                if (sz < 0 || fsync(dst_fd_binary) != 0)
+                {
+                    unlink(path);
+                    error_msg_and_die("error saving binary image to %s", path);
+                }
+                close(dst_fd_binary);
+                close(src_fd_binary);
             }
-            close(dst_fd_binary);
-            close(src_fd_binary);
-        }
 
-        /* We need coredumps to be readable by all, because
-         * when abrt daemon processes coredump,
-         * process producing backtrace is run under the same UID
-         * as the crashed process.
-         * Thus 644, not 600 */
-        strcpy(path + path_len, "/"FILENAME_COREDUMP);
-        int abrt_core_fd = open(path, O_RDWR | O_CREAT | O_TRUNC, 0644);
-        if (abrt_core_fd < 0)
-        {
-            int sv_errno = errno;
-            dd.Delete();
-            dd.Close();
-            if (user_core_fd >= 0)
+            /* We need coredumps to be readable by all, because
+             * when abrt daemon processes coredump,
+             * process producing backtrace is run under the same UID
+             * as the crashed process.
+             * Thus 644, not 600 */
+            strcpy(path + path_len, "/"FILENAME_COREDUMP);
+            int abrt_core_fd = open(path, O_RDWR | O_CREAT | O_TRUNC, 0644);
+            if (abrt_core_fd < 0)
             {
-                xchdir(user_pwd);
-                unlink(core_basename);
+                int sv_errno = errno;
+                dd.Delete();
+                dd.Close();
+                if (user_core_fd >= 0)
+                {
+                    xchdir(user_pwd);
+                    unlink(core_basename);
+                }
+                errno = sv_errno;
+                perror_msg_and_die("can't open '%s'", path);
             }
-            errno = sv_errno;
-            perror_msg_and_die("can't open '%s'", path);
-        }
 
-        /* We write both coredumps at once.
-         * We can't write user coredump first, since it might be truncated
-         * and thus can't be copied and used as abrt coredump;
-         * and if we write abrt coredump first and then copy it as user one,
-         * then we have a race when process exits but coredump does not exist yet:
-         * $ echo -e '#include<signal.h>\nmain(){raise(SIGSEGV);}' | gcc -o test -x c -
-         * $ rm -f core*; ulimit -c unlimited; ./test; ls -l core*
-         * 21631 Segmentation fault (core dumped) ./test
-         * ls: cannot access core*: No such file or directory <=== BAD
-         */
+            /* We write both coredumps at once.
+             * We can't write user coredump first, since it might be truncated
+             * and thus can't be copied and used as abrt coredump;
+             * and if we write abrt coredump first and then copy it as user one,
+             * then we have a race when process exits but coredump does not exist yet:
+             * $ echo -e '#include<signal.h>\nmain(){raise(SIGSEGV);}' | gcc -o test -x c -
+             * $ rm -f core*; ulimit -c unlimited; ./test; ls -l core*
+             * 21631 Segmentation fault (core dumped) ./test
+             * ls: cannot access core*: No such file or directory <=== BAD
+             */
 //TODO: fchown abrt_core_fd to uid:abrt?
 //Currently it is owned by 0:0 but is readable by anyone, so the owner
 //of the crashed binary still can access it, as he has
 //r-x access to the dump dir.
-        off_t core_size = copyfd_sparse(STDIN_FILENO, abrt_core_fd, user_core_fd, ulimit_c);
-        if (core_size < 0 || fsync(abrt_core_fd) != 0)
-        {
-            unlink(path);
-            dd.Delete();
-            dd.Close();
-            if (user_core_fd >= 0)
+            off_t core_size = copyfd_sparse(STDIN_FILENO, abrt_core_fd, user_core_fd, ulimit_c);
+            if (core_size < 0 || fsync(abrt_core_fd) != 0)
+            {
+                unlink(path);
+                dd.Delete();
+                dd.Close();
+                if (user_core_fd >= 0)
+                {
+                    xchdir(user_pwd);
+                    unlink(core_basename);
+                }
+                /* copyfd_sparse logs the error including errno string,
+                 * but it does not log file name */
+                error_msg_and_die("error writing %s", path);
+            }
+            log("saved core dump of pid %lu (%s) to %s (%llu bytes)", (long)pid, executable, path, (long long)core_size);
+            if (user_core_fd >= 0 && core_size >= ulimit_c)
             {
+                /* user coredump is too big, nuke it */
                 xchdir(user_pwd);
                 unlink(core_basename);
             }
-            /* copyfd_sparse logs the error including errno string,
-             * but it does not log file name */
-            error_msg_and_die("error writing %s", path);
-        }
-        log("saved core dump of pid %lu (%s) to %s (%llu bytes)", (long)pid, executable, path, (long long)core_size);
-        if (user_core_fd >= 0 && core_size >= ulimit_c)
-        {
-            /* user coredump is too big, nuke it */
-            xchdir(user_pwd);
-            unlink(core_basename);
-        }
 
-        /* We close dumpdir before we start catering for crash storm case.
-         * Otherwise, delete_debug_dump_dir's from other concurrent
-         * CCpp's won't be able to delete our dump (their delete_debug_dump_dir
-         * will wait for us), and we won't be able to delete their dumps.
-         * Classic deadlock.
-         */
-        dd.Close();
-        path[path_len] = '\0'; /* path now contains only directory name */
-        char *newpath = xstrndup(path, path_len - (sizeof(".new")-1));
-        if (rename(path, newpath) == 0)
-            strcpy(path, newpath);
-        free(newpath);
-
-        /* rhbz#539551: "abrt going crazy when crashing process is respawned" */
-        if (setting_MaxCrashReportsSize > 0)
-        {
-            trim_debug_dumps(setting_MaxCrashReportsSize, path);
-        }
+            /* We close dumpdir before we start catering for crash storm case.
+             * Otherwise, delete_debug_dump_dir's from other concurrent
+             * CCpp's won't be able to delete our dump (their delete_debug_dump_dir
+             * will wait for us), and we won't be able to delete their dumps.
+             * Classic deadlock.
+             */
+            dd.Close();
+            path[path_len] = '\0'; /* path now contains only directory name */
+            char *newpath = xstrndup(path, path_len - (sizeof(".new")-1));
+            if (rename(path, newpath) == 0)
+                strcpy(path, newpath);
+            free(newpath);
+
+            /* rhbz#539551: "abrt going crazy when crashing process is respawned" */
+            if (setting_MaxCrashReportsSize > 0)
+            {
+                trim_debug_dumps(setting_MaxCrashReportsSize, path);
+            }
 
-        return 0;
-    }
-    catch (CABRTException& e)
-    {
-        error_msg_and_die("%s", e.what());
+            return 0;
+        }
+        else
+            xfunc_die();
     }
     catch (std::exception& e)
     {
diff --git a/src/hooks/dumpoops.cpp b/src/hooks/dumpoops.cpp
index a2d2353a..a14fb65b 100644
--- a/src/hooks/dumpoops.cpp
+++ b/src/hooks/dumpoops.cpp
@@ -77,7 +77,7 @@ int main(int argc, char **argv)
 //	const plugin_info_t *plugin_info;
 	CPlugin* (*plugin_newf)(void);
 	int (*scan_syslog_file)(vector_string_t& oopsList, const char *filename, time_t *last_changed_p);
-	void (*save_oops_to_debug_dump)(const vector_string_t& oopsList);
+	int (*save_oops_to_debug_dump)(const vector_string_t& oopsList);
 	void *handle;
 
 	errno = 0;
@@ -110,11 +110,10 @@ int main(int argc, char **argv)
 		}
 		if (opt_d) {
 			log("dumping oopses");
-			try {
-				save_oops_to_debug_dump(oopsList);
-			}
-			catch (CABRTException& e) {
-				fprintf(stderr, "Error: %s\n", e.what());
+			int errors = save_oops_to_debug_dump(oopsList);
+			if (errors > 0)
+			{
+				log("%d errors while dumping oopses", errors);
 				return 1;
 			}
 		}