[PATCH 1/5] get rid of std::string from bugzilla
by Nikola Pajkovsky
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
13 years, 6 months
backtrace parser and processor for ABRT
by Karel Klic
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
13 years, 6 months
Re: [Crash-catcher] java reports by crash catcher
by Michal Nowak
----- "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
13 years, 6 months
Retrace server
by Karel Klic
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.)
13 years, 6 months
[PATCH 1/5] g_settings_setOpenGPGPublicKeys uses GList
by Nikola Pajkovsky
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
13 years, 6 months
[PATCH 0/3] new parser
by Nikola Pajkovsky
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
13 years, 6 months
Reat Britain at the time when the ruling factions of our count
by Micale Ceasor
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
13 years, 7 months