Crash-catcher September 2010

crash-catcher@lists.fedorahosted.org

44 participants
64 discussions

[PATCH 1/5] get rid of std::string from bugzilla
by Nikola Pajkovsky 19 Oct '10

19 Oct '10

Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com> --- inc/plugin.h | 6 +- lib/plugins/Bugzilla.cpp | 244 ++++++++++++++++++++++++--------------------- lib/plugins/Bugzilla.h | 12 +- lib/plugins/Logger.cpp | 10 +- lib/utils/make_descr.cpp | 180 +++++++++++++++++++--------------- 5 files changed, 245 insertions(+), 207 deletions(-) diff --git a/inc/plugin.h b/inc/plugin.h index 34d5743..d7772b5 100644 --- a/inc/plugin.h +++ b/inc/plugin.h @@ -119,9 +119,9 @@ typedef struct SPluginInfo }; /* helper functions */ -std::string make_description_bz(const map_crash_data_t& pCrashData); -std::string make_description_reproduce_comment(const map_crash_data_t& pCrashData); -std::string make_description_logger(const map_crash_data_t& pCrashData); +char* make_description_bz(const map_crash_data_t& pCrashData); +char* make_description_reproduce_comment(const map_crash_data_t& pCrashData); +char* make_description_logger(const map_crash_data_t& pCrashData); /** * Loads settings and stores it in second parameter. On success it diff --git a/lib/plugins/Bugzilla.cpp b/lib/plugins/Bugzilla.cpp index f3967cd..e494f08 100644 --- a/lib/plugins/Bugzilla.cpp +++ b/lib/plugins/Bugzilla.cpp @@ -24,6 +24,7 @@ #include "debug_dump.h" #include "abrt_exception.h" #include "comm_layer_inner.h" +#include "strbuf.h" #ifdef HAVE_CONFIG_H # include <config.h> #endif @@ -31,6 +32,7 @@ #define XML_RPC_SUFFIX "/xmlrpc.cgi" #define MAX_HOPS 5 + /* * TODO: npajkovs: better deallocation of xmlrpc value * npajkovs: better gathering function which collects all information from bugzilla @@ -389,46 +391,45 @@ int ctx::add_comment(xmlrpc_int32 bug_id, const char* comment, bool is_private) xmlrpc_int32 ctx::new_bug(const map_crash_data_t& pCrashData, int depend_on_bugno) { - const std::string& package = get_crash_data_item_content(pCrashData, FILENAME_PACKAGE); - const std::string& component = get_crash_data_item_content(pCrashData, FILENAME_COMPONENT); - const std::string& release = get_crash_data_item_content(pCrashData, FILENAME_RELEASE); - const std::string& arch = get_crash_data_item_content(pCrashData, FILENAME_ARCHITECTURE); - const std::string& duphash = get_crash_data_item_content(pCrashData, CD_DUPHASH); - const char *reason = get_crash_data_item_content_or_NULL(pCrashData, FILENAME_REASON); - const char *function = get_crash_data_item_content_or_NULL(pCrashData, FILENAME_CRASH_FUNCTION); - - std::string summary = "[abrt] " + package; + const char *package = get_crash_data_item_content_or_NULL(pCrashData, FILENAME_PACKAGE); + const char *component = get_crash_data_item_content_or_NULL(pCrashData, FILENAME_COMPONENT); + const char *release = get_crash_data_item_content_or_NULL(pCrashData, FILENAME_RELEASE); + const char *arch = get_crash_data_item_content_or_NULL(pCrashData, FILENAME_ARCHITECTURE); + const char *duphash = get_crash_data_item_content_or_NULL(pCrashData, CD_DUPHASH); + const char *reason = get_crash_data_item_content_or_NULL(pCrashData, FILENAME_REASON); + const char *function = get_crash_data_item_content_or_NULL(pCrashData, FILENAME_CRASH_FUNCTION); + + struct strbuf *buf_summary = strbuf_new(); + strbuf_append_strf(buf_summary, "[abrt] %s", package); + if (function != NULL && strlen(function) < 30) - { - summary += ": "; - summary += function; - } + strbuf_append_strf(buf_summary, ": %s", function); if (reason != NULL) - { - summary += ": "; - summary += reason; - } - std::string status_whiteboard = "abrt_hash:" + duphash; + strbuf_append_strf(buf_summary, ": %s", reason); + + char *status_whiteboard = xasprintf("abrt_hash:%s", duphash); - std::string description = "abrt version: "VERSION"\n"; - description += make_description_bz(pCrashData); + char *bz_dsc = make_description_bz(pCrashData); + char *full_dsc = xasprintf("abrt version: "VERSION"\n%s", bz_dsc); + free(bz_dsc); char *product = NULL; char *version = NULL; - parse_release(release.c_str(), &product, &version); + parse_release(release, &product, &version); xmlrpc_value* result = NULL; + char *summary = strbuf_free_nobuf(buf_summary); if (depend_on_bugno > -1) { result = call("Bug.create", "({s:s,s:s,s:s,s:s,s:s,s:s,s:s,s:i})", "product", product, - "component", component.c_str(), + "component", component, "version", version, - "summary", summary.c_str(), - "description", description.c_str(), - "status_whiteboard", status_whiteboard.c_str(), - "platform", arch.c_str(), + "summary", summary, + "description", full_dsc, + "status_whiteboard", status_whiteboard, + "platform", arch, "dependson", depend_on_bugno ); } @@ -436,17 +437,19 @@ xmlrpc_int32 ctx::new_bug(const map_crash_data_t& pCrashData, int depend_on_bugn { result = call("Bug.create", "({s:s,s:s,s:s,s:s,s:s,s:s,s:s})", "product", product, - "component", component.c_str(), + "component", component, "version", version, - "summary", summary.c_str(), - "description", description.c_str(), - "status_whiteboard", status_whiteboard.c_str(), - "platform", arch.c_str() + "summary", summary, + "description", full_dsc, + "status_whiteboard", status_whiteboard, + "platform", arch ); - } + free(status_whiteboard); free(product); free(version); + free(summary); + free(full_dsc); if (!result) return -1; @@ -473,21 +476,23 @@ int ctx::add_attachments(const char* bug_id_str, const map_crash_data_t& pCrashD map_crash_data_t::const_iterator it = pCrashData.begin(); for (; it != pCrashData.end(); it++) { - const std::string &itemname = it->first; - const std::string &type = it->second[CD_TYPE]; - const std::string &content = it->second[CD_CONTENT]; + const char *itemname = it->first.c_str(); + const char *type = it->second[CD_TYPE].c_str(); + const char *content = it->second[CD_CONTENT].c_str(); - if (type == CD_TXT - && (content.length() > CD_TEXT_ATT_SIZE || itemname == FILENAME_BACKTRACE) + if ((strcmp(type, CD_TXT) == 0) + && (strlen(content) > CD_TEXT_ATT_SIZE || (strcmp(itemname, FILENAME_BACKTRACE) == 0)) ) { - char *encoded64 = encode_base64(content.c_str(), content.length()); + char *encoded64 = encode_base64(content, strlen(content)); + char *filename = xasprintf("File: %s", itemname); xmlrpc_value* result = call("bugzilla.addAttachment", "(s{s:s,s:s,s:s,s:s})", bug_id_str, - "description", ("File: " + itemname).c_str(), - "filename", itemname.c_str(), + "description", filename, + "filename", itemname, "contenttype", "text/plain", "data", encoded64 ); free(encoded64); + free(filename); if (!result) return -1; @@ -558,9 +563,11 @@ void ctx::login(const char* login, const char* passwd) if (!result) { - std::string errmsg = ssprintf(_("Cannot login. Check Edit->Plugins->Bugzilla and /etc/abrt/plugins/Bugzilla.conf. Server said: %s"), env.fault_string); - error_msg("%s", errmsg.c_str()); // show error in daemon log - throw CABRTException(EXCEP_PLUGIN, "%s", errmsg.c_str()); + char *errmsg = xasprintf("Can't login. Check Edit->Plugins->Bugzilla and /etc/abrt/plugins/Bugzilla.conf. Server said: %s", env.fault_string); + error_msg("%s", errmsg); // show error in daemon log + CABRTException e(EXCEP_PLUGIN, errmsg); + free(errmsg); + throw e; } xmlrpc_DECREF(result); } @@ -636,59 +643,68 @@ static map_plugin_settings_t parse_settings(const map_plugin_settings_t& pSettin return plugin_settings; } -CReporterBugzilla::CReporterBugzilla() : - m_bSSLVerify(true), - m_sBugzillaURL("https://bugzilla.redhat.com"), - m_sBugzillaXMLRPC("https://bugzilla.redhat.com"XML_RPC_SUFFIX), - m_bRatingRequired(true) -{} +CReporterBugzilla::CReporterBugzilla() +{ + m_ssl_verify = true; + m_rating_required = true; + m_login = NULL; + m_password = NULL; + m_bugzilla_url = xstrdup("https://bugzilla.redhat.com"); + m_bugzilla_xmlrpc = xstrdup("https://bugzilla.redhat.com"XML_RPC_SUFFIX); +} CReporterBugzilla::~CReporterBugzilla() -{} +{ + free(m_login); + free(m_password); + free(m_bugzilla_url); + free(m_bugzilla_xmlrpc); +} std::string CReporterBugzilla::Report(const map_crash_data_t& pCrashData, const map_plugin_settings_t& pSettings, const char *pArgs) { xmlrpc_int32 bug_id = -1; - std::string Login; - std::string Password; - std::string BugzillaXMLRPC; - std::string BugzillaURL; - bool SSLVerify; + const char *login = NULL; + const char *password = NULL; + const char *bugzilla_xmlrpc = NULL; + const char *bugzilla_url = NULL; + bool ssl_verify; + map_plugin_settings_t settings = parse_settings(pSettings); /* if parse_settings fails it returns an empty map so we need to use defaults */ if (!settings.empty()) { - Login = settings["Login"]; - Password = settings["Password"]; - BugzillaXMLRPC = settings["BugzillaXMLRPC"]; - BugzillaURL = settings["BugzillaURL"]; - SSLVerify = string_to_bool(settings["SSLVerify"].c_str()); + login = settings["Login"].c_str(); + password = settings["Password"].c_str(); + bugzilla_xmlrpc = settings["BugzillaXMLRPC"].c_str(); + bugzilla_url = settings["BugzillaURL"].c_str(); + ssl_verify = string_to_bool(settings["NoSSLVerify"].c_str()); } else { - Login = m_sLogin; - Password = m_sPassword; - BugzillaXMLRPC = m_sBugzillaXMLRPC; - BugzillaURL = m_sBugzillaURL; - SSLVerify = m_bSSLVerify; + login = m_login; + password = m_password; + bugzilla_xmlrpc = m_bugzilla_xmlrpc; + bugzilla_url = m_bugzilla_url; + ssl_verify = m_ssl_verify; } - if ((Login == "") || (Password == "")) + if (!login[0] || !password[0]) { VERB3 log("Empty login and password"); throw CABRTException(EXCEP_PLUGIN, _("Empty login or password.\nPlease check "PLUGINS_CONF_DIR"/Bugzilla.conf.")); } - const std::string& component = get_crash_data_item_content(pCrashData, FILENAME_COMPONENT); - const std::string& duphash = get_crash_data_item_content(pCrashData, CD_DUPHASH); - const char *release = get_crash_data_item_content_or_NULL(pCrashData, FILENAME_RELEASE); + const char *component = get_crash_data_item_content_or_NULL(pCrashData, FILENAME_COMPONENT); + const char *duphash = get_crash_data_item_content_or_NULL(pCrashData, CD_DUPHASH); + const char *release = get_crash_data_item_content_or_NULL(pCrashData, FILENAME_RELEASE); - ctx bz_server(BugzillaXMLRPC.c_str(), SSLVerify); + ctx bz_server(bugzilla_xmlrpc, ssl_verify); update_client(_("Logging into bugzilla...")); - bz_server.login(Login.c_str(), Password.c_str()); + bz_server.login(login, password); update_client(_("Checking for duplicates...")); @@ -698,9 +714,9 @@ std::string CReporterBugzilla::Report(const map_crash_data_t& pCrashData, xmlrpc_value *result; if (strcmp(product, "Fedora") == 0) - result = bz_server.call_quicksearch_duphash(component.c_str(), product, duphash.c_str()); + result = bz_server.call_quicksearch_duphash(component, product, duphash); else - result = bz_server.call_quicksearch_duphash(component.c_str(), NULL, duphash.c_str()); + result = bz_server.call_quicksearch_duphash(component, NULL, duphash); if (!result) throw_if_xml_fault_occurred(&bz_server.env); @@ -736,7 +752,7 @@ std::string CReporterBugzilla::Report(const map_crash_data_t& pCrashData, { depend_on_bugno = bug_id; bug_info_destroy(&bz); - result = bz_server.call_quicksearch_duphash(component.c_str(), release, duphash.c_str()); + result = bz_server.call_quicksearch_duphash(component, release, duphash); if (!result) throw_if_xml_fault_occurred(&bz_server.env); @@ -799,7 +815,7 @@ std::string CReporterBugzilla::Report(const map_crash_data_t& pCrashData, std::string bug_status = ssprintf( "Status: NEW\n" "%s/show_bug.cgi?id=%u", - BugzillaURL.c_str(), + bugzilla_url, (int)bug_id ); return bug_status; @@ -808,7 +824,7 @@ std::string CReporterBugzilla::Report(const map_crash_data_t& pCrashData, { // When someone clones bug it has same duphash, so we can find more than 1. // Need to be checked if component is same. - VERB3 log("Bugzilla has %u reports with same duphash '%s'", all_bugs_size, duphash.c_str()); + VERB3 log("Bugzilla has %u reports with same duphash '%s'", all_bugs_size, duphash); } // decision based on state @@ -850,11 +866,11 @@ std::string CReporterBugzilla::Report(const map_crash_data_t& pCrashData, if (strcmp(bz.bug_status, "CLOSED") != 0) { int status = 0; - if ((strcmp(bz.bug_reporter, Login.c_str()) != 0) && (am_i_in_cc(&bz, Login.c_str()))) + if ((strcmp(bz.bug_reporter, login) != 0) && (am_i_in_cc(&bz, login))) { - VERB2 log(_("Adding %s to CC list"), Login.c_str()); - update_client(_("Adding %s to CC list"), Login.c_str()); - status = bz_server.add_plus_one_cc(bug_id, Login.c_str()); + VERB2 log(_("Add %s to CC list"), login); + update_client(_("Add %s to CC list"), login); + status = bz_server.add_plus_one_cc(bug_id, login); } if (status == -1) @@ -863,28 +879,32 @@ std::string CReporterBugzilla::Report(const map_crash_data_t& pCrashData, throw_if_xml_fault_occurred(&bz_server.env); } - std::string description = make_description_reproduce_comment(pCrashData); - if (!description.empty()) + char *dsc = make_description_reproduce_comment(pCrashData); + if (dsc) { const char* package = get_crash_data_item_content_or_NULL(pCrashData, FILENAME_PACKAGE); const char* release = get_crash_data_item_content_or_NULL(pCrashData, FILENAME_RELEASE); const char* arch = get_crash_data_item_content_or_NULL(pCrashData, FILENAME_ARCHITECTURE); const char* is_private = get_crash_data_item_content_or_NULL(pCrashData, "is_private"); - description = ssprintf("Package: %s\n" + char *full_dsc = xasprintf("Package: %s\n" "Architecture: %s\n" "OS Release: %s\n" - "%s", package, arch, release, description.c_str() + "%s", package, arch, release, dsc ); update_client(_("Adding new comment to bug %d"), (int)bug_id); + free(dsc); + bool is_priv = is_private && (is_private[0] == '1'); - if (bz_server.add_comment(bug_id, description.c_str(), is_priv) == -1) + if (bz_server.add_comment(bug_id, full_dsc, is_priv) == -1) { + free(full_dsc); bug_info_destroy(&bz); throw_if_xml_fault_occurred(&bz_server.env); } + free(full_dsc); } } @@ -897,7 +917,7 @@ std::string CReporterBugzilla::Report(const map_crash_data_t& pCrashData, bz.bug_status, bz.bug_resolution ? " " : "", bz.bug_resolution ? bz.bug_resolution : "", - BugzillaURL.c_str(), + bugzilla_url, (int)bug_id ); @@ -922,52 +942,48 @@ void CReporterBugzilla::SetSettings(const map_plugin_settings_t& pSettings) it = pSettings.find("BugzillaURL"); if (it != end) { - m_sBugzillaURL = it->second; - //remove the /xmlrpc.cgi part from old settings - //FIXME: can be removed after users are informed about new config format - std::string::size_type pos = m_sBugzillaURL.find(XML_RPC_SUFFIX); - if (pos != std::string::npos) - { - m_sBugzillaURL.erase(pos); - } - //remove the trailing '/' - while (m_sBugzillaURL[m_sBugzillaURL.length() - 1] == '/') - { - m_sBugzillaURL.erase(m_sBugzillaURL.length() - 1); - } - /* - if (*(--m_sBugzillaURL.end()) == '/') - { - m_sBugzillaURL.erase(--m_sBugzillaURL.end()); - } - */ - m_sBugzillaXMLRPC = m_sBugzillaURL + XML_RPC_SUFFIX; + free(m_bugzilla_url); + free(m_bugzilla_xmlrpc); + + m_bugzilla_url = xstrdup(it->second.c_str()); + + int cnt = strlen(m_bugzilla_url); + while (m_bugzilla_url[cnt--] == '/') + m_bugzilla_url[cnt] = '\0'; + + int ret = suffixcmp(m_bugzilla_url, XML_RPC_SUFFIX); + if (ret != 0) + m_bugzilla_xmlrpc = xasprintf("%s%s", m_bugzilla_url, XML_RPC_SUFFIX); + else + m_bugzilla_xmlrpc = xstrdup(m_bugzilla_url); } it = pSettings.find("Login"); if (it != end) { - m_sLogin = it->second; + free(m_login); + m_login = xstrdup(it->second.c_str()); } it = pSettings.find("Password"); if (it != end) { - m_sPassword = it->second; + free(m_password); + m_password = xstrdup(it->second.c_str()); } it = pSettings.find("SSLVerify"); if (it != end) { - m_bSSLVerify = string_to_bool(it->second.c_str()); + m_ssl_verify = string_to_bool(it->second.c_str()); } } /* Should not be deleted (why?) */ const map_plugin_settings_t& CReporterBugzilla::GetSettings() { - m_pSettings["BugzillaURL"] = m_sBugzillaURL; - m_pSettings["Login"] = m_sLogin; - m_pSettings["Password"] = m_sPassword; - m_pSettings["SSLVerify"] = m_bSSLVerify ? "yes" : "no"; - m_pSettings["RatingRequired"] = m_bRatingRequired ? "yes" : "no"; + m_pSettings["BugzillaURL"] = m_bugzilla_url; + m_pSettings["Login"] = (m_login)? m_login: ""; + m_pSettings["Password"] = (m_password)? m_password: ""; + m_pSettings["SSLVerify"] = m_ssl_verify ? "yes" : "no"; + m_pSettings["RatingRequired"] = m_rating_required ? "yes" : "no"; return m_pSettings; } diff --git a/lib/plugins/Bugzilla.h b/lib/plugins/Bugzilla.h index a4c9a01..2b32a70 100644 --- a/lib/plugins/Bugzilla.h +++ b/lib/plugins/Bugzilla.h @@ -25,12 +25,12 @@ class CReporterBugzilla : public CReporter { private: - bool m_bSSLVerify; - std::string m_sBugzillaURL; - std::string m_sBugzillaXMLRPC; - std::string m_sLogin; - std::string m_sPassword; - bool m_bRatingRequired; + bool m_ssl_verify; + char *m_bugzilla_url; + char *m_bugzilla_xmlrpc; + char *m_login; + char *m_password; + bool m_rating_required; public: CReporterBugzilla(); diff --git a/lib/plugins/Logger.cpp b/lib/plugins/Logger.cpp index ece450d..a02845f 100644 --- a/lib/plugins/Logger.cpp +++ b/lib/plugins/Logger.cpp @@ -60,7 +60,9 @@ std::string CLogger::Report(const map_crash_data_t& pCrashData, const map_plugin_settings_t& pSettings, const char *pArgs) { - std::string description = make_description_logger(pCrashData); + char *dsc = make_description_logger(pCrashData); + char *full_dsc = xasprintf("%s\n\n\n", dsc); + free(dsc); /* open, not fopen - want to set mode if we create the file, not just open */ const char *fname = m_sLogPath.c_str(); @@ -71,9 +73,9 @@ std::string CLogger::Report(const map_crash_data_t& pCrashData, throw CABRTException(EXCEP_PLUGIN, "Can't open '%s'", fname); update_client(_("Writing report to '%s'"), fname); - description += "\n\n\n"; - const char *desc = description.c_str(); - full_write(fd, desc, strlen(desc)); + full_write(fd, full_dsc, strlen(full_dsc)); + free(full_dsc); + close(fd); const char *format = m_bAppendLogs ? _("The report was appended to %s") : _("The report was stored to %s"); diff --git a/lib/utils/make_descr.cpp b/lib/utils/make_descr.cpp index 46d9644..93ae292 100644 --- a/lib/utils/make_descr.cpp +++ b/lib/utils/make_descr.cpp @@ -19,6 +19,7 @@ #include "abrtlib.h" #include "crash_types.h" #include "debug_dump.h" /* FILENAME_ARCHITECTURE etc */ +#include "strbuf.h" #ifdef HAVE_CONFIG_H # include "config.h" #endif @@ -29,13 +30,15 @@ # define _(S) (S) #endif + using namespace std; -static void add_content(bool &was_multiline, string& description, const char *header, const char *content) +// caller is responsible for freeing **dsc +static void add_content(bool *was_multiline, char **dsc, const char *header, const char *content) { - /* We separate multiline contents with emply line */ - if (was_multiline) - description += '\n'; + struct strbuf *buf_description = strbuf_new(); + if (*was_multiline) + strbuf_append_char(buf_description, '\n'); while (content[0] == '\n') content++; @@ -45,27 +48,27 @@ static void add_content(bool &was_multiline, string& description, const char *he if (skip_whitespace(content)[0] == '\0') { /* empty, dont report at all */ + *dsc = strbuf_free_nobuf(buf_description); return; } /* one string value, like OS release */ - description += header; - description += ": "; - description += content; - description += '\n'; - was_multiline = 0; + strbuf_append_strf(buf_description, "%s: %s\n", header, content); + *was_multiline = 0; } else { /* multi-string value, like backtrace */ - if (!was_multiline && description.size() != 0) /* if wasn't yet separated */ - description += '\n'; /* do it now */ - description += header; - description += "\n-----\n"; - description += content; + if (!*was_multiline && (buf_description->len != 0)) /* if wasn't yet separated */ + strbuf_append_char(buf_description, '\n'); + + strbuf_append_strf(buf_description, "%s\n-----\n%s", header, content); if (content[strlen(content) - 1] != '\n') - description += '\n'; - was_multiline = 1; + strbuf_append_char(buf_description, '\n'); + + *was_multiline = 1; } + + *dsc = strbuf_free_nobuf(buf_description); } /* Items we don't want to include */ @@ -85,85 +88,91 @@ static const char *const blacklisted_items[] = { NULL }; -string make_description_bz(const map_crash_data_t& pCrashData) +char* make_description_bz(const map_crash_data_t& pCrashData) { - string description; - string long_description; + struct strbuf *buf_dsc = strbuf_new(); + struct strbuf *buf_long_dsc = strbuf_new(); map_crash_data_t::const_iterator it = pCrashData.begin(); for (; it != pCrashData.end(); it++) { - const string& itemname = it->first; - const string& type = it->second[CD_TYPE]; - const string& content = it->second[CD_CONTENT]; - if (type == CD_TXT) + const char *itemname = it->first.c_str(); + const char *type = it->second[CD_TYPE].c_str(); + const char *content = it->second[CD_CONTENT].c_str(); + if (strcmp(type, CD_TXT) == 0) { /* Skip items we are not interested in */ const char *const *bl = blacklisted_items; while (*bl) { - if (itemname == *bl) + if (strcmp(itemname, *bl) == 0) break; bl++; } if (*bl) continue; /* blacklisted */ - if (content == "1.\n2.\n3.\n") + if (strcmp(content, "1.\n2.\n3.\n") == 0) continue; /* user did not change default "How to reproduce" */ - if (content.size() <= CD_TEXT_ATT_SIZE) + if (strlen(content) <= CD_TEXT_ATT_SIZE) { /* Add small (less than few kb) text items inline */ bool was_multiline = 0; - string tmp; - add_content(was_multiline, - tmp, - /* "reproduce: blah" looks ugly, fixing: */ - itemname == FILENAME_REPRODUCE ? "How to reproduce" : itemname.c_str(), - content.c_str() + char *tmp = NULL; + add_content(&was_multiline, + &tmp, + /* "reproduce: blah" looks ugly, fixing: */ + (strcmp(itemname, FILENAME_REPRODUCE) == 0) ? "How to reproduce" : itemname, + content ); if (was_multiline) { /* Not one-liner */ - if (long_description.size() != 0) - long_description += '\n'; - long_description += tmp; + if (buf_long_dsc->len != 0) + strbuf_append_char(buf_long_dsc, '\n'); + + strbuf_append_str(buf_long_dsc, tmp); } else - { - description += tmp; - } + strbuf_append_str(buf_dsc, tmp); + + free(tmp); } else { bool was_multiline = 0; - add_content(was_multiline, description, "Attached file", itemname.c_str()); + char *dsc = NULL; + add_content(&was_multiline, &dsc, "Attached file", itemname); + strbuf_append_str(buf_dsc, dsc); + free(dsc); } } } /* One-liners go first, then multi-line items */ - if (description.size() != 0 && long_description.size() != 0) - { - description += '\n'; - } - description += long_description; + if (buf_dsc->len != 0 && buf_long_dsc->len != 0) + strbuf_append_char(buf_dsc, '\n'); + - return description; + char *long_dsc = strbuf_free_nobuf(buf_long_dsc); + strbuf_append_str(buf_dsc, long_dsc); + free(long_dsc); + + return strbuf_free_nobuf(buf_dsc); } -string make_description_logger(const map_crash_data_t& pCrashData) +char* make_description_logger(const map_crash_data_t& pCrashData) { - string description; - string long_description; + struct strbuf *buf_dsc = strbuf_new(); + struct strbuf *buf_long_dsc = strbuf_new(); map_crash_data_t::const_iterator it = pCrashData.begin(); for (; it != pCrashData.end(); it++) { - const string &filename = it->first; - const string &type = it->second[CD_TYPE]; - const string &content = it->second[CD_CONTENT]; - if (type == CD_TXT - || type == CD_BIN + const char *filename = it->first.c_str(); + const char *type = it->second[CD_TYPE].c_str(); + const char *content = it->second[CD_CONTENT].c_str(); + if ((strcmp(type, CD_TXT) == 0) + || (strcmp(type, CD_BIN) == 0) ) { /* Skip items we are not interested in */ const char *const *bl = blacklisted_items; @@ -175,62 +184,73 @@ string make_description_logger(const map_crash_data_t& pCrashData) } if (*bl) continue; /* blacklisted */ - if (content == "1.\n2.\n3.\n") + if (strcmp(content, "1.\n2.\n3.\n") == 0) continue; /* user did not change default "How to reproduce" */ bool was_multiline = 0; - string tmp; - add_content(was_multiline, tmp, filename.c_str(), content.c_str()); + char *tmp = NULL; + add_content(&was_multiline, &tmp, filename, content); if (was_multiline) { - if (long_description.size() != 0) - long_description += '\n'; - long_description += tmp; + if (buf_long_dsc->len != 0) + strbuf_append_char(buf_long_dsc,'\n'); + + strbuf_append_str(buf_long_dsc, tmp); } else - { - description += tmp; - } + strbuf_append_str(buf_dsc, tmp); } } - if (description.size() != 0 && long_description.size() != 0) - { - description += '\n'; - } - description += long_description; + if (buf_dsc->len != 0 && buf_long_dsc->len != 0) + strbuf_append_char(buf_dsc, '\n'); + + char *long_dsc = strbuf_free_nobuf(buf_long_dsc); + strbuf_append_str(buf_dsc, long_dsc); + free(long_dsc); - return description; + return strbuf_free_nobuf(buf_dsc); } -string make_description_reproduce_comment(const map_crash_data_t& pCrashData) +char* make_description_reproduce_comment(const map_crash_data_t& pCrashData) { + char *repro = NULL; + char *comment = NULL; + map_crash_data_t::const_iterator end = pCrashData.end(); map_crash_data_t::const_iterator it; - string howToReproduce; it = pCrashData.find(FILENAME_REPRODUCE); if (it != end) { if ((it->second[CD_CONTENT].size() > 0) && (it->second[CD_CONTENT] != "1.\n2.\n3.\n")) { - howToReproduce = "\n\nHow to reproduce\n" - "-----\n"; - howToReproduce += it->second[CD_CONTENT]; + repro = xasprintf("\n\nHow to reproduce\n-----\n%s", it->second[CD_CONTENT].c_str()); } } - string comment; + it = pCrashData.find(FILENAME_COMMENT); if (it != end) { if (it->second[CD_CONTENT].size() > 0) - { - comment = "\n\nComment\n" - "-----\n"; - comment += it->second[CD_CONTENT]; - } + comment = xasprintf("\n\nComment\n-----\n%s", it->second[CD_CONTENT].c_str()); } - return howToReproduce + comment; + + if (!repro && !comment) + return NULL; + + struct strbuf *buf_dsc = strbuf_new(); + + if (repro) + strbuf_append_str(buf_dsc, repro); + + if (comment) + strbuf_append_str(buf_dsc, comment); + + free(repro); + free(comment); + + return strbuf_free_nobuf(buf_dsc); } -- 1.7.1

3 11

backtrace parser and processor for ABRT
by Karel Klic 15 Oct '10

15 Oct '10

Hi, I have created the next iteration of backtrace parsing and processing code, and I would like to see it included in the development version of ABRT. What are the improvements compared to the current ABRT's backtrace code? - the backtrace duplication hash algorithm has been significantly improved: it now removes much more irrelevant frames, and unifies and merges frames. For example, glib's functions are sometimes prefixed with IA__, and sometimes they are not (sometimes you see IA__g_logv, sometimes just g_logv), so the IA__ is stripped for the duplication hash. Another example: many backtraces were the same except that one crashed in strcpy_ssse3 and other in strcpy_sse2. So now all glibc's functions which depend on the instruction set available are unified for the hash purposes (both strcpy_sse2 and strcpy_ssse3 are renamed to strcpy) - the backtrace rating algorithm has been improved: it ignores cleanup-after-crash frames even when they miss debug info, and it also fixes bug #592523 - the "crash function name" detection has been improved: many irrelevant frames are skipped during the crash frame detection, so the reported crash function is more often the actual place where the program crashed - new hand written parser: it parses much more backtraces correctly (especially those with C++ frames, applications using boost library), and the code seems to be more readable than the current bison grammar. That is because some parts of the backtrace format are very difficult to express in the rules for the Bison GLR parser while keeping the memory usage and parsing time reasonable - the error messages returned by the hand written parser always include precise location of the error (line:column) and an informative message - the new parser does not crash on "wrong" backtraces; the current bison parser uses stack heavily, and when some line in a backtrace is too long (many kilobytes) bison puts too much stuff on the stack and crashes (I do not know how to fix that without rewriting most of the grammar, and that would probably introduce limits on other places); so the new parser fixes crashes #627698, #627680, #616988, #589962, #588129, #573333 -- The new code has been written as a separate library (see the attached archive), because it became pretty large, containing a binary not used by the rest of ABRT, many tests, and several helper scripts. Within ABRT, it's used only by the CCpp plugin. I am not sure how to integrate it. Should we include it (as a separate project in a subdirectory) to ABRT's git repository? I spend many days hunting bugs in the code, so it is in a good shape now as far as I can tell. Most C functions are covered by unit tests, and I often run the parser on all ABRT-reported backtraces downloaded from Bugzilla (~27000) and it provides good results. So putting btparser into ABRT should not cause much disruption. See the attached patches for the integration code. How to check how it works with ABRT? $ tar xzvf btparser-0.5.tar.gz $ cd btparser-0.5 $ ./configure $ make check # to see the tests :) $ make rpm $ cd i686 # depends on your arch $ sudo yum install --nogpgcheck \ ./btparser-0.5-1.*.rpm \ ./btparser-devel-0.5-1.*.rpm $ cd ../../abrt # to your devel abrt git clone $ git apply 000*.patch $ ./autogen.sh $ ./configure $ make rpm ... Check the btparser-0.5/README and btparser-0.5/lib/*.h. I tried to explain there what it does. Thanks, Karel

5 11

Re: [Crash-catcher] java reports by crash catcher
by Michal Nowak 13 Oct '10

13 Oct '10

----- "Mark Wielaard" <mjw(a)redhat.com> wrote: > Hi, > > It would be nice if crash-catcher could be thought about the > hs_pid*.log > files that a crashed java process creates. That file contains much > more > information that is relevant to the crash than the gdb backtrace that > is > currently collected. If it can see if there is a hs_pid###.log file > (where ### is the process id of the java process that crashed) and > attached that to the bug report it files that would be appreciated. > I am afraid Java exceptions are not supported at the moment (just the python ones) and thus ABRT is not being run in such cases. Do you have some example crash, where ABRT stepped in? Since Java apps exception handling is completely missing, perhaps some of you guys can help? > Thanks, > > Mark Michal > > _______________________________________________ > Crash-catcher mailing list > Crash-catcher(a)lists.fedorahosted.org > https://fedorahosted.org/mailman/listinfo/crash-catcher

3 15

[PATCH] added search box to backtrace rhbz612017
by Jiri Moskovcak 11 Oct '10

11 Oct '10

3 5

Retrace server
by Karel Klic 11 Oct '10

11 Oct '10

Hi, I have written a document that discusses various aspects and a possible "design" of our retrace server. After spending several days thinking about the server, I believe the retrace server will provide a pretty large improvement in the reliability and usefulness of ABRT (both in the intranet installations, and in Fedora). To me, it seems to be worth implementing. It would be great if someone reads it and points to stupid ideas, unclear stuff etc. New thoughts are welcome and very important in this stage. You can post a diff with your changes. Thanks. Karel (We should also look into the debuginfofs idea. The debuginfofs would also improve ABRT experience a lot. And I believe we should go for both the debuginfofs and the retrace server, because both have advantages and disadvantages for various usage cases. I can post about it later, now I need to finish the btparser work.)

7 13

[PATCH 1/5] g_settings_setOpenGPGPublicKeys uses GList
by Nikola Pajkovsky 06 Oct '10

06 Oct '10

Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com> --- src/daemon/Daemon.cpp | 1 + src/daemon/Settings.cpp | 13 +++++++++++-- src/daemon/Settings.h | 5 ++++- src/daemon/abrt-action-save-package-data.cpp | 7 +++---- 4 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/daemon/Daemon.cpp b/src/daemon/Daemon.cpp index 8ab9991..4cecae1 100644 --- a/src/daemon/Daemon.cpp +++ b/src/daemon/Daemon.cpp @@ -1114,6 +1114,7 @@ int main(int argc, char** argv) if (pMainloop) g_main_loop_unref(pMainloop); + settings_free(); /* Exiting */ if (s_sig_caught && s_sig_caught != SIGALRM && s_sig_caught != SIGCHLD) { diff --git a/src/daemon/Settings.cpp b/src/daemon/Settings.cpp index a77e482..a6d59f5 100644 --- a/src/daemon/Settings.cpp +++ b/src/daemon/Settings.cpp @@ -51,7 +51,7 @@ static map_string_t s_mapSectionCron; /* one line: "OpenGPGCheck = value" */ bool g_settings_bOpenGPGCheck = false; /* one line: "OpenGPGPublicKeys = value1,value2" */ -set_string_t g_settings_setOpenGPGPublicKeys; +GList *g_settings_setOpenGPGPublicKeys = NULL; set_string_t g_settings_setBlackListedPkgs; set_string_t g_settings_setBlackListedPaths; std::string g_settings_sDatabase; @@ -348,7 +348,7 @@ static void LoadGPGKeys() if (line[0] == '/') // probably the begining of path, so let's handle it as a key { strchrnul(line, '\n')[0] = '\0'; - g_settings_setOpenGPGPublicKeys.insert(line); + g_settings_setOpenGPGPublicKeys = g_list_append(g_settings_setOpenGPGPublicKeys, xstrdup(line)); } } fclose(fp); @@ -559,3 +559,12 @@ void SetSettings(const map_abrt_settings_t& pSettings, const char *dbus_sender) ParseCron(); } } + +void settings_free() +{ + for (GList *li = g_settings_setOpenGPGPublicKeys; li != NULL; li = g_list_next(li)) + free((char*)li->data); + + g_list_free(g_settings_setOpenGPGPublicKeys); + +} diff --git a/src/daemon/Settings.h b/src/daemon/Settings.h index 3103dbd..2a92020 100644 --- a/src/daemon/Settings.h +++ b/src/daemon/Settings.h @@ -20,12 +20,13 @@ #define SETTINGS_H_ #include "abrt_types.h" +#include <glib.h> typedef map_vector_pair_string_string_t map_analyzer_actions_and_reporters_t; typedef map_vector_pair_string_string_t map_cron_t; typedef map_map_string_t map_abrt_settings_t; -extern set_string_t g_settings_setOpenGPGPublicKeys; +extern GList *g_settings_setOpenGPGPublicKeys; extern set_string_t g_settings_setBlackListedPkgs; extern set_string_t g_settings_setBlackListedPaths; extern unsigned int g_settings_nMaxCrashReportsSize; @@ -42,4 +43,6 @@ void SaveSettings(); void SetSettings(const map_abrt_settings_t& pSettings, const char * dbus_sender); map_abrt_settings_t GetSettings(); +void settings_free(); + #endif diff --git a/src/daemon/abrt-action-save-package-data.cpp b/src/daemon/abrt-action-save-package-data.cpp index bd18bc8..039fa24 100644 --- a/src/daemon/abrt-action-save-package-data.cpp +++ b/src/daemon/abrt-action-save-package-data.cpp @@ -320,11 +320,10 @@ int main(int argc, char **argv) VERB1 log("Initializing rpm library"); rpm_init(); - set_string_t::iterator it_k = g_settings_setOpenGPGPublicKeys.begin(); - for (; it_k != g_settings_setOpenGPGPublicKeys.end(); it_k++) + for (GList *li = g_settings_setOpenGPGPublicKeys; li != NULL; li = g_list_next(li)) { - VERB1 log("Loading GPG key '%s'", it_k->c_str()); - rpm_load_gpgkey(it_k->c_str()); + VERB1 log("Loading GPG key '%s'", (char*)li->data); + rpm_load_gpgkey((char*)li->data); } return SavePackageDescriptionToDebugDump(dump_dir_name); -- 1.7.1

3 16

[PATCH 0/3] new parser
by Nikola Pajkovsky 05 Oct '10

05 Oct '10

A new parser is developed because of we have a lot of standalone applicaions. When bad argument is passed to app, parser will generate a help, it means all app will have same help output. For now is only daemon moved to a new parser to see how to use it. Nikola Pajkovsky (3): xfunc.c: add new funtion xcalloc new args parser Daemon.cpp: remove getopt and use parse_opts instead inc/xfuncs.h | 1 + lib/utils/Makefile.am | 3 +- lib/utils/parse_options.c | 130 +++++++++++++++++++++++++++++++++++++++++++++ lib/utils/parse_options.h | 37 +++++++++++++ lib/utils/xfuncs.c | 8 +++ src/daemon/Daemon.cpp | 65 ++++++++++++----------- 6 files changed, 212 insertions(+), 32 deletions(-) create mode 100644 lib/utils/parse_options.c create mode 100644 lib/utils/parse_options.h

4 22

Derniers jours des prix cassés,Tee-shirts et livraison gratuite
by contact＠tiao.fr 30 Sep '10

30 Sep '10

Pour être sûr de recevoir tous nos emails, ajoutez-nous à votre carnet d'adresses. Si ce mail ne s'affiche pas correctement, suivez ce lien ( http://www.emkg.net/D1/QVO-fzZ/QqhxAiwf6/QmEkwaIZG_t.aspx )

1 0

La solution repas des entreprises où il fait bon vivre
by Dent Blanche 30 Sep '10

30 Sep '10

1 0

Reat Britain at the time when the ruling factions of our count
by Micale Ceasor 29 Sep '10

29 Sep '10

name, as a part of a larger work, the restoration of his country to its earliest and noblest tradition, which alone gave permanence or worth to its existence as a nation. CHAPTER II THE GROWTH OF THE AMERICAN NATION 1. _The Formation of a National Government_. It is of course impossible to understand the life of a politician in another cou

1 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

Crash-catcher September 2010