From 816f3e001271ed8ab7fdadb6d90aeb2c61362dac Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Wed, 8 Dec 2010 14:51:47 +0100 Subject: removal of C++isms from libabrt, part 1 This patch converts libabrt usage of C++ map to a glib-based container, GHashTable. It is typedef-ed to map_string_h. We can't typedef it to map_string_t, since other parts of ABRT (daemon, cli) still use that name for C++ container. Also, exceptions are removed everywhere. Signed-off-by: Denys Vlasenko --- src/daemon/CommLayerServerDBus.cpp | 23 +---- src/daemon/Daemon.cpp | 106 ++++++++------------- src/daemon/MiddleWare.cpp | 189 ++++++++++++++++--------------------- src/daemon/MiddleWare.h | 2 +- src/daemon/PluginManager.cpp | 49 +++++----- src/daemon/PluginManager.h | 4 - 6 files changed, 152 insertions(+), 221 deletions(-) (limited to 'src/daemon') diff --git a/src/daemon/CommLayerServerDBus.cpp b/src/daemon/CommLayerServerDBus.cpp index 14f132b3..b9b7f20b 100644 --- a/src/daemon/CommLayerServerDBus.cpp +++ b/src/daemon/CommLayerServerDBus.cpp @@ -19,7 +19,6 @@ #include #include "abrtlib.h" #include "abrt_dbus.h" -#include "abrt_exception.h" #include "comm_layer_inner.h" #include "dbus_common.h" #include "MiddleWare.h" @@ -275,20 +274,7 @@ static int handle_Report(DBusMessage* call, DBusMessage* reply) } unix_uid = get_remote_uid(call); - try - { - argout1 = Report(crash_data, events, user_conf_data, unix_uid); - } - catch (CABRTException &e) - { - dbus_message_unref(reply); - reply = dbus_message_new_error(call, DBUS_ERROR_FAILED, e.what()); - if (!reply) - die_out_of_memory(); - send_flush_and_unref(reply); - r = 0; - goto ret; - } + argout1 = Report(crash_data, events, user_conf_data, unix_uid); DBusMessageIter out_iter; dbus_message_iter_init_append(reply, &out_iter); @@ -353,12 +339,13 @@ static int handle_GetPluginSettings(DBusMessage* call, DBusMessage* reply) //long unix_uid = get_remote_uid(call); //VERB1 log("got %s('%s') call from uid %ld", "GetPluginSettings", PluginName, unix_uid); - map_plugin_settings_t plugin_settings; - GetPluginSettings(PluginName, plugin_settings); + map_string_h *plugin_settings = GetPluginSettings(PluginName); DBusMessageIter out_iter; dbus_message_iter_init_append(reply, &out_iter); - store_val(&out_iter, plugin_settings); + store_map_string(&out_iter, plugin_settings); + + free_map_string(plugin_settings); send_flush_and_unref(reply); return 0; diff --git a/src/daemon/Daemon.cpp b/src/daemon/Daemon.cpp index 62bcdc68..f98d8741 100644 --- a/src/daemon/Daemon.cpp +++ b/src/daemon/Daemon.cpp @@ -27,7 +27,6 @@ #include /* ioctl(FIONREAD) */ #include #include "abrtlib.h" -#include "abrt_exception.h" #include "comm_layer_inner.h" #include "Settings.h" #include "CommLayerServerDBus.h" @@ -543,63 +542,49 @@ static gboolean handle_inotify_cb(GIOChannel *gio, GIOCondition condition, gpoin char *fullname = NULL; crash_data_t *crash_data = NULL; - try + fullname = concat_path_file(DEBUG_DUMPS_DIR, name); + mw_result_t res = LoadDebugDump(fullname, &crash_data); + switch (res) { - fullname = concat_path_file(DEBUG_DUMPS_DIR, name); - mw_result_t res = LoadDebugDump(fullname, &crash_data); - switch (res) - { - case MW_OK: - log("New crash %s, processing", fullname); - /* Fall through */ + case MW_OK: + log("New crash %s, processing", fullname); + /* Fall through */ - case MW_OCCURRED: /* dup */ + case MW_OCCURRED: /* dup */ + { + if (res != MW_OK) { - if (res != MW_OK) - { - const char *first = get_crash_item_content_or_NULL(crash_data, CD_DUMPDIR); - log("Deleting crash %s (dup of %s), sending dbus signal", - strrchr(fullname, '/') + 1, - strrchr(first, '/') + 1); - delete_crash_dump_dir(fullname); - } - - const char *uid_str = get_crash_item_content_or_NULL(crash_data, FILENAME_UID); - const char *inform_all = get_crash_item_content_or_NULL(crash_data, FILENAME_INFORMALL); - - if (inform_all && string_to_bool(inform_all)) - uid_str = NULL; - char *crash_id = xasprintf("%s:%s", - get_crash_item_content_or_NULL(crash_data, FILENAME_UID), - get_crash_item_content_or_NULL(crash_data, FILENAME_UUID) - ); - /* Send dbus signal */ - g_pCommLayer->Crash(get_crash_item_content_or_NULL(crash_data, FILENAME_PACKAGE), - crash_id, //TODO: stop passing this param, it is unused - fullname, - uid_str - ); - free(crash_id); - break; - } - case MW_CORRUPTED: - case MW_GPG_ERROR: - default: - log("Corrupted or bad crash %s (res:%d), deleting", fullname, (int)res); + const char *first = get_crash_item_content_or_NULL(crash_data, CD_DUMPDIR); + log("Deleting crash %s (dup of %s), sending dbus signal", + strrchr(fullname, '/') + 1, + strrchr(first, '/') + 1); delete_crash_dump_dir(fullname); - break; + } + + const char *uid_str = get_crash_item_content_or_NULL(crash_data, FILENAME_UID); + const char *inform_all = get_crash_item_content_or_NULL(crash_data, FILENAME_INFORMALL); + + if (inform_all && string_to_bool(inform_all)) + uid_str = NULL; + char *crash_id = xasprintf("%s:%s", + get_crash_item_content_or_NULL(crash_data, FILENAME_UID), + get_crash_item_content_or_NULL(crash_data, FILENAME_UUID) + ); + /* Send dbus signal */ + g_pCommLayer->Crash(get_crash_item_content_or_NULL(crash_data, FILENAME_PACKAGE), + crash_id, //TODO: stop passing this param, it is unused + fullname, + uid_str + ); + free(crash_id); + break; } - } - catch (CABRTException& e) - { - error_msg("%s", e.what()); - } - catch (...) - { - free(fullname); - free(buf); - free_crash_data(crash_data); - throw; + case MW_CORRUPTED: + case MW_GPG_ERROR: + default: + log("Corrupted or bad crash %s (res:%d), deleting", fullname, (int)res); + delete_crash_dump_dir(fullname); + break; } free(fullname); free_crash_data(crash_data); @@ -899,19 +884,8 @@ int main(int argc, char** argv) s_signal_pipe_write = s_signal_pipe[1]; /* Enter the event loop */ - try - { - log("Init complete, entering main loop"); - run_main_loop(pMainloop); - } - catch (CABRTException& e) - { - error_msg("Error: %s", e.what()); - } - catch (std::exception& e) - { - error_msg("Error: %s", e.what()); - } + log("Init complete, entering main loop"); + run_main_loop(pMainloop); cleanup: /* Error or INT/TERM. Clean up, in reverse order. diff --git a/src/daemon/MiddleWare.cpp b/src/daemon/MiddleWare.cpp index 6abf019e..7135a50a 100644 --- a/src/daemon/MiddleWare.cpp +++ b/src/daemon/MiddleWare.cpp @@ -21,7 +21,6 @@ #include "abrtlib.h" #include "Daemon.h" #include "Settings.h" -#include "abrt_exception.h" #include "comm_layer_inner.h" #include "CommLayerServer.h" #include "MiddleWare.h" @@ -127,6 +126,8 @@ static mw_result_t CreateCrashReport(const char *dump_dir_name, if (!dd) return MW_NOENT_ERROR; + struct run_event_state *run_state; + int res; mw_result_t r = MW_OK; if (caller_uid != 0) /* not called by root */ @@ -151,37 +152,24 @@ static mw_result_t CreateCrashReport(const char *dump_dir_name, } dd_close(dd); - try + run_state = new_run_event_state(); + run_state->logging_callback = do_log_and_update_client; + res = run_event(run_state, dump_dir_name, force ? "reanalyze" : "analyze"); + free_run_event_state(run_state); + if (res != 0 && res != -1) /* -1 is "nothing was done", here it is ok */ { - struct run_event_state *run_state = new_run_event_state(); - run_state->logging_callback = do_log_and_update_client; - int res = run_event(run_state, dump_dir_name, force ? "reanalyze" : "analyze"); - free_run_event_state(run_state); - if (res != 0 && res != -1) /* -1 is "nothing was done", here it is ok */ - { - r = MW_PLUGIN_ERROR; - goto ret; - } - - /* Do a load_crash_data_from_crash_dump_dir from (possibly updated) - * crash dump dir - */ - *crash_data = DebugDumpToCrashReport(dump_dir_name); - if (!*crash_data) - { - error_msg("Error loading crash data"); - r = MW_ERROR; - goto ret; - } + r = MW_PLUGIN_ERROR; + goto ret; } - catch (CABRTException& e) + + /* Do a load_crash_data_from_crash_dump_dir from (possibly updated) + * crash dump dir + */ + *crash_data = DebugDumpToCrashReport(dump_dir_name); + if (!*crash_data) { - r = MW_CORRUPTED; - error_msg("%s", e.what()); - if (e.type() == EXCEP_PLUGIN) - { - r = MW_PLUGIN_ERROR; - } + error_msg("Error loading crash data"); + r = MW_ERROR; } ret: @@ -199,14 +187,7 @@ void RunAction(const char *pActionDir, /* GetAction() already complained */ return; } - try - { - action->Run(pActionDir, pPluginArgs, /*force:*/ 0); - } - catch (CABRTException& e) - { - error_msg("Execution of '%s' was not successful: %s", pPluginName, e.what()); - } + action->Run(pActionDir, pPluginArgs, /*force:*/ 0); } struct logging_state { @@ -231,12 +212,14 @@ report_status_t Report(crash_data_t *client_report, const map_map_string_t& settings, long caller_uid) { - // Get ID fields - const char *UID = get_crash_item_content_or_NULL(client_report, FILENAME_UID); + report_status_t ret; const char *dump_dir_name = get_crash_item_content_or_NULL(client_report, CD_DUMPDIR); - if (!UID || !dump_dir_name) + if (!dump_dir_name) { - throw CABRTException(EXCEP_ERROR, "Report(): UID or DUMPDIR is missing in client's report data"); + update_client("Reporting error: %s", "DUMPDIR is missing in client's report data"); + ret[""].push_back("0"); // REPORT_STATUS_IDX_FLAG + ret[""].push_back("DUMPDIR is missing in client's report data"); // REPORT_STATUS_IDX_MSG + return ret; } // Retrieve corresponding stored record @@ -248,14 +231,18 @@ report_status_t Report(crash_data_t *client_report, // Is it allowed for this user to report? if (caller_uid != 0 // not called by root - && strcmp(to_string(caller_uid).c_str(), UID) != 0 + && strcmp(to_string(caller_uid).c_str(), get_crash_item_content_or_die(stored_report, FILENAME_UID)) != 0 ) { const char *inform_all = get_crash_item_content_or_NULL(stored_report, FILENAME_INFORMALL); if (!inform_all || !string_to_bool(inform_all)) { free_crash_data(stored_report); - throw CABRTException(EXCEP_ERROR, "Report(): user with uid %ld can't report crash %s", - caller_uid, dump_dir_name); + char *errmsg = xasprintf("user with uid %ld can't report crash %s", caller_uid, dump_dir_name); + update_client("Reporting error: %s", errmsg); + ret[""].push_back("0"); // REPORT_STATUS_IDX_FLAG + ret[""].push_back(errmsg); // REPORT_STATUS_IDX_MSG + free(errmsg); + return ret; } } @@ -335,7 +322,6 @@ report_status_t Report(crash_data_t *client_report, // Run events bool at_least_one_reporter_succeeded = false; - report_status_t ret; std::string message; struct logging_state l_state; struct run_event_state *run_state = new_run_event_state(); @@ -620,68 +606,61 @@ vector_of_crash_data_t *GetCrashInfos(long caller_uid) DIR *dir = opendir(DEBUG_DUMPS_DIR); if (dir != NULL) { - try + struct dirent *dent; + while ((dent = readdir(dir)) != NULL) { - struct dirent *dent; - while ((dent = readdir(dir)) != NULL) - { - if (dot_or_dotdot(dent->d_name)) - continue; /* skip "." and ".." */ + if (dot_or_dotdot(dent->d_name)) + continue; /* skip "." and ".." */ - char *dump_dir_name = concat_path_file(DEBUG_DUMPS_DIR, dent->d_name); + char *dump_dir_name = concat_path_file(DEBUG_DUMPS_DIR, dent->d_name); - struct stat statbuf; - if (stat(dump_dir_name, &statbuf) != 0 - || !S_ISDIR(statbuf.st_mode) - ) { - goto next; /* not a dir, skip */ - } + struct stat statbuf; + if (stat(dump_dir_name, &statbuf) != 0 + || !S_ISDIR(statbuf.st_mode) + ) { + goto next; /* not a dir, skip */ + } - /* Skip directories which are not for this uid */ - if (caller_uid != 0) /* not called by root? */ - { - char *uid; - char caller_uid_str[sizeof(long) * 3 + 2]; + /* Skip directories which are not for this uid */ + if (caller_uid != 0) /* not called by root? */ + { + char *uid; + char caller_uid_str[sizeof(long) * 3 + 2]; - struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags:*/ 0); - if (!dd) - goto next; + struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags:*/ 0); + if (!dd) + goto next; - sprintf(caller_uid_str, "%ld", caller_uid); - uid = dd_load_text(dd, FILENAME_UID); - if (strcmp(uid, caller_uid_str) != 0) + sprintf(caller_uid_str, "%ld", caller_uid); + uid = dd_load_text(dd, FILENAME_UID); + if (strcmp(uid, caller_uid_str) != 0) + { + char *inform_all = dd_load_text_ext(dd, FILENAME_INFORMALL, DD_FAIL_QUIETLY); + bool for_all = string_to_bool(inform_all); + free(inform_all); + if (!for_all) { - char *inform_all = dd_load_text_ext(dd, FILENAME_INFORMALL, DD_FAIL_QUIETLY); - bool for_all = string_to_bool(inform_all); - free(inform_all); - if (!for_all) - { - dd_close(dd); - goto next; - } + dd_close(dd); + goto next; } - dd_close(dd); } + dd_close(dd); + } + { + crash_data_t *crash_data = FillCrashInfo(dump_dir_name); + if (!crash_data) { - crash_data_t *crash_data = FillCrashInfo(dump_dir_name); - if (!crash_data) - { - error_msg("Dump directory %s doesn't exist or misses crucial files, deleting", dump_dir_name); - delete_crash_dump_dir(dump_dir_name); - } - else - { - g_ptr_array_add(retval, crash_data); - } + error_msg("Dump directory %s doesn't exist or misses crucial files, deleting", dump_dir_name); + delete_crash_dump_dir(dump_dir_name); + } + else + { + g_ptr_array_add(retval, crash_data); } - next: - free(dump_dir_name); } - } - catch (CABRTException& e) - { - error_msg("%s", e.what()); + next: + free(dump_dir_name); } closedir(dir); } @@ -736,18 +715,10 @@ static void* create_report(void* arg) /* Client name is per-thread, need to set it */ set_client_name(thread_data->peer); - try - { - log("Creating report..."); - crash_data_t *crash_data = NULL; - CreateReport(thread_data->crash_id, thread_data->caller_uid, thread_data->force, &crash_data); - g_pCommLayer->JobDone(thread_data->peer); - } - catch (CABRTException& e) - { - error_msg("%s", e.what()); - } - catch (...) {} + log("Creating report..."); + crash_data_t *crash_data = NULL; + CreateReport(thread_data->crash_id, thread_data->caller_uid, thread_data->force, &crash_data); + g_pCommLayer->JobDone(thread_data->peer); set_client_name(NULL); /* free strduped strings */ @@ -874,10 +845,12 @@ void GetPluginsInfo(map_map_string_t &map_of_plugin_info) closedir(dir); } -void GetPluginSettings(const char *plugin_name, map_plugin_settings_t &plugin_settings) +map_string_h *GetPluginSettings(const char *plugin_name) { char *conf_file = xasprintf(PLUGINS_CONF_DIR"/%s.conf", plugin_name); - if (LoadPluginSettings(conf_file, plugin_settings, /*skip w/o value:*/ false)) + map_string_h *settings = new_map_string(); + if (load_conf_file(conf_file, settings, /*skip w/o value:*/ false)) VERB3 log("Loaded %s.conf", plugin_name); free(conf_file); + return settings; } diff --git a/src/daemon/MiddleWare.h b/src/daemon/MiddleWare.h index 55c84c3b..4826d79d 100644 --- a/src/daemon/MiddleWare.h +++ b/src/daemon/MiddleWare.h @@ -91,6 +91,6 @@ void CreateReport(const char* dump_dir_name, long caller_uid, int force, crash_d int DeleteDebugDump(const char *dump_dir_name, long caller_uid); void GetPluginsInfo(map_map_string_t &map_of_plugin_info); -void GetPluginSettings(const char *plugin_name, map_plugin_settings_t &plugin_settings); +map_string_h *GetPluginSettings(const char *plugin_name); #endif /*MIDDLEWARE_H_*/ diff --git a/src/daemon/PluginManager.cpp b/src/daemon/PluginManager.cpp index 665a4625..f8f6b8f8 100644 --- a/src/daemon/PluginManager.cpp +++ b/src/daemon/PluginManager.cpp @@ -20,7 +20,6 @@ */ #include #include "abrtlib.h" -#include "abrt_exception.h" #include "PluginManager.h" using namespace std; @@ -132,18 +131,14 @@ CPlugin* CPluginManager::LoadPlugin(const char *pName, bool enabled_only) /* Kerneloops{,Scanner,Reporter} share the same .conf file */ conf_name = "Kerneloops"; } - map_plugin_settings_t pluginSettings; string conf_fullname = ssprintf(PLUGINS_CONF_DIR"/%s."PLUGINS_CONF_EXTENSION, conf_name); - LoadPluginSettings(conf_fullname.c_str(), pluginSettings); - m_map_plugin_settings[pName] = pluginSettings; - /* If settings are empty, most likely .conf file does not exist. - * Don't mislead the user: */ - VERB3 if (!pluginSettings.empty()) log("Loaded %s.conf", conf_name); + map_string_h *pluginSettings = new_map_string(); + if (load_conf_file(conf_fullname.c_str(), pluginSettings, /*skip key w/o values:*/ true)) + VERB3 log("Loaded %s.conf", conf_name); if (enabled_only) { - map_plugin_settings_t::iterator it = pluginSettings.find("Enabled"); - if (it == pluginSettings.end() || !string_to_bool(it->second.c_str())) + if (!string_to_bool(get_map_string_item_or_empty(pluginSettings, "Enabled"))) { plugin_info["Enabled"] = "no"; string empty; @@ -163,6 +158,7 @@ CPlugin* CPluginManager::LoadPlugin(const char *pName, bool enabled_only) if (!handle) { error_msg("Can't load '%s': %s", libPath.c_str(), dlerror()); + free_map_string(pluginSettings); return NULL; /* error */ } CLoadedModule *module = new CLoadedModule(handle, pName); @@ -175,27 +171,32 @@ CPlugin* CPluginManager::LoadPlugin(const char *pName, bool enabled_only) module->GetMagicNumber(), PLUGINS_MAGIC_NUMBER, module->GetType(), MAX_PLUGIN_TYPE); delete module; + free_map_string(pluginSettings); return NULL; /* error */ } VERB3 log("Loaded plugin %s v.%s", pName, module->GetVersion()); CPlugin *plugin = NULL; - try + plugin = module->PluginNew(); + plugin->Init(); + /* Need to convert pluginSettings from map_string_h container + * to map_string_t, since plugin->SetSettings() needs that type. + * To be removed when remaining uses of map_string_t + * are globally converted to map_string_h. + */ { - plugin = module->PluginNew(); - plugin->Init(); - plugin->SetSettings(pluginSettings); - } - catch (CABRTException& e) - { - error_msg("Can't initialize plugin %s: %s", - pName, - e.what() - ); - delete plugin; - delete module; - return NULL; /* error */ + map_string_t pluginSettings2; + GHashTableIter iter; + char *name; + char *value; + g_hash_table_iter_init(&iter, pluginSettings); + while (g_hash_table_iter_next(&iter, (void**)&name, (void**)&value)) + { + pluginSettings2[name] = value; + } + plugin->SetSettings(pluginSettings2); } + free_map_string(pluginSettings); plugin_info["Enabled"] = "yes"; plugin_info["Type"] = plugin_type_str[module->GetType()]; @@ -252,7 +253,7 @@ plugin_type_t CPluginManager::GetPluginType(const char *pName) CPlugin *plugin = LoadPlugin(pName); if (!plugin) { - throw CABRTException(EXCEP_PLUGIN, "Plugin '%s' is not registered", pName); + return INVALID_PLUGIN_TYPE; } map_loaded_module_t::iterator it_module = m_mapLoadedModules.find(pName); return it_module->second->GetType(); diff --git a/src/daemon/PluginManager.h b/src/daemon/PluginManager.h index c5036fbf..1108b985 100644 --- a/src/daemon/PluginManager.h +++ b/src/daemon/PluginManager.h @@ -48,10 +48,6 @@ class CPluginManager * Registered plugins. A key is a plugin name. */ map_plugin_t m_mapPlugins; - /** - * List of all possible plugins (loaded or not), with some attributes. - */ - map_map_string_t m_map_plugin_settings; public: /** -- cgit