Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com --- inc/plugin.h | 6 +- lib/plugins/Bugzilla.cpp | 257 ++++++++++++++++++++++++++-------------------- lib/plugins/Bugzilla.h | 12 +- lib/plugins/Logger.cpp | 10 +- lib/utils/make_descr.cpp | 180 ++++++++++++++++++-------------- 5 files changed, 259 insertions(+), 206 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..c8a849b 100644 --- a/lib/plugins/Bugzilla.cpp +++ b/lib/plugins/Bugzilla.cpp @@ -31,6 +31,8 @@ #define XML_RPC_SUFFIX "/xmlrpc.cgi" #define MAX_HOPS 5
+#include "strbuf.h" + /* * 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%22XML_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%22XML_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 no_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(); + no_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; + no_ssl_verify = m_ssl_verify; }
- if ((Login == "") || (Password == "")) + if ((strcmp(login, "") == 0) || (strcmp(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, no_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);
@@ -789,9 +805,7 @@ std::string CReporterBugzilla::Report(const map_crash_data_t& pCrashData, log("Adding attachments to bug %d...", bug_id); int ret = bz_server.add_attachments(to_string(bug_id).c_str(), pCrashData); if (ret == -1) - { throw_if_xml_fault_occurred(&bz_server.env); - }
update_client(_("Logging out...")); bz_server.logout(); @@ -799,7 +813,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,12 +822,11 @@ 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 update_client(_("Bug is already reported: %i"), bug_id); - xmlrpc_int32 original_bug_id = bug_id; if ((strcmp(bz.bug_status, "CLOSED") == 0) && (strcmp(bz.bug_resolution, "DUPLICATE") == 0)) { @@ -850,11 +863,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 +876,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); + throw_xml_fault(&bz_server.env); } + free(full_dsc); } }
@@ -897,7 +914,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 );
@@ -919,58 +936,72 @@ void CReporterBugzilla::SetSettings(const map_plugin_settings_t& pSettings)
map_plugin_settings_t::const_iterator end = pSettings.end(); map_plugin_settings_t::const_iterator it; + 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()); } + it = pSettings.find("NoSSLVerify"); + if (it != end) + { + m_ssl_verify = string_to_bool(it->second.c_str()); + } + VERB3 log("%s:%s:%s:%s:%s", __FILE__, __func__, m_bugzilla_url, m_login, m_password); }
/* Should not be deleted (why?) */ +/* const map_plugin_settings_t& CReporterBugzilla::GetSettings() { +<<<<<<< HEAD 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["NoSSLVerify"] = m_no_ssl_verify ? "yes" : "no"; +>>>>>>> rid of std::string from bugzilla
return m_pSettings; } +*/
PLUGIN_INFO(REPORTER, CReporterBugzilla, 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..348795c 100644 --- a/lib/utils/make_descr.cpp +++ b/lib/utils/make_descr.cpp @@ -29,13 +29,16 @@ # define _(S) (S) #endif
+#include "strbuf.h" + 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); }
Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com --- inc/plugin.h | 1 + lib/plugins/Mailx.cpp | 122 +++++++++++++++------------------------------ lib/plugins/Mailx.h | 12 ++-- lib/utils/make_descr.cpp | 42 ++++++++++++++++ 4 files changed, 90 insertions(+), 87 deletions(-)
diff --git a/inc/plugin.h b/inc/plugin.h index d7772b5..ebad086 100644 --- a/inc/plugin.h +++ b/inc/plugin.h @@ -122,6 +122,7 @@ typedef struct SPluginInfo 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); +char* make_dsc_mailx(const map_crash_data_t& pCrashData);
/** * Loads settings and stores it in second parameter. On success it diff --git a/lib/plugins/Mailx.cpp b/lib/plugins/Mailx.cpp index a94a2c5..2bc68e1 100644 --- a/lib/plugins/Mailx.cpp +++ b/lib/plugins/Mailx.cpp @@ -26,26 +26,34 @@
#define MAILX_COMMAND "/bin/mailx"
-CMailx::CMailx() : - m_sEmailFrom("user@localhost"), - m_sEmailTo("root@localhost"), - m_sSubject("[abrt] full crash report"), - m_bSendBinaryData(false) -{} - -static void exec_and_feed_input(uid_t uid, const char* pText, char **pArgs) +CMailx::CMailx() +{ + m_email_from = xstrdup("user@localhost"); + m_email_to = xstrdup("root@localhost"); + m_subject = xstrdup("[abrt] full crash report"); + m_send_binary_data = false; +} + +CMailx::~CMailx() +{ + free(m_email_from); + free(m_email_to); + free(m_subject); +} + +static void exec_and_feed_input(uid_t uid, const char* text, char **args) { int pipein[2];
pid_t child = fork_execv_on_steroids( EXECFLG_INPUT | EXECFLG_QUIET | EXECFLG_SETGUID, - pArgs, + args, pipein, /*unsetenv_vec:*/ NULL, /*dir:*/ NULL, uid);
- full_write(pipein[1], pText, strlen(pText)); + full_write(pipein[1], text, strlen(text)); close(pipein[1]);
waitpid(child, NULL, 0); /* wait for command completion */ @@ -71,70 +79,29 @@ std::string CMailx::Report(const map_crash_data_t& pCrashData, unsigned arg_size = 0; args = append_str_to_vector(args, arg_size, MAILX_COMMAND);
-//TODO: move email body generation to make_descr.cpp - std::string binaryFiles, commonFiles, additionalFiles, DUPHASHFile; + char *dsc = make_dsc_mailx(pCrashData); + map_crash_data_t::const_iterator it; for (it = pCrashData.begin(); it != pCrashData.end(); it++) { - if (it->second[CD_TYPE] == CD_TXT) - { - if (it->first != CD_DUPHASH - && it->first != FILENAME_ARCHITECTURE - && it->first != FILENAME_KERNEL - && it->first != FILENAME_PACKAGE - ) { - additionalFiles += it->first; - additionalFiles += "\n-----\n"; - additionalFiles += it->second[CD_CONTENT]; - additionalFiles += "\n\n"; - } - else if (it->first == CD_DUPHASH) - { - DUPHASHFile += it->first; - DUPHASHFile += "\n-----\n"; - DUPHASHFile += it->second[CD_CONTENT]; - DUPHASHFile += "\n\n"; - } - else - { - commonFiles += it->first; - commonFiles += "\n-----\n"; - commonFiles += it->second[CD_CONTENT]; - commonFiles += "\n\n"; - } - } - if (it->second[CD_TYPE] == CD_BIN) + if (it->second[CD_TYPE] == CD_BIN && m_send_binary_data) { - binaryFiles += " -a "; - binaryFiles += it->second[CD_CONTENT]; - if (m_bSendBinaryData) - { - args = append_str_to_vector(args, arg_size, "-a"); - args = append_str_to_vector(args, arg_size, it->second[CD_CONTENT].c_str()); - } + args = append_str_to_vector(args, arg_size, "-a"); + args = append_str_to_vector(args, arg_size, it->second[CD_CONTENT].c_str()); } }
- std::string emailBody = "Duplicate check\n"; - emailBody += "=====\n\n"; - emailBody += DUPHASHFile; - emailBody += "\nCommon information\n"; - emailBody += "=====\n\n"; - emailBody += commonFiles; - emailBody += "\nAdditional information\n"; - emailBody += "=====\n\n"; - emailBody += additionalFiles; - emailBody += '\n'; - args = append_str_to_vector(args, arg_size, "-s"); - args = append_str_to_vector(args, arg_size, (pArgs[0] != '\0' ? pArgs : m_sSubject.c_str())); + args = append_str_to_vector(args, arg_size, (pArgs[0] != '\0' ? pArgs : m_subject)); args = append_str_to_vector(args, arg_size, "-r"); - args = append_str_to_vector(args, arg_size, m_sEmailFrom.c_str()); - args = append_str_to_vector(args, arg_size, m_sEmailTo.c_str()); + args = append_str_to_vector(args, arg_size, m_email_from); + args = append_str_to_vector(args, arg_size, m_email_to);
update_client(_("Sending an email...")); - const char *uid_str = get_crash_data_item_content(pCrashData, CD_UID).c_str(); - exec_and_feed_input(xatoi_u(uid_str), emailBody.c_str(), args); + const char *uid_str = get_crash_data_item_content_or_NULL(pCrashData, CD_UID); + exec_and_feed_input(xatoi_u(uid_str), dsc, args); + + free(dsc);
while (*args) { @@ -143,7 +110,7 @@ std::string CMailx::Report(const map_crash_data_t& pCrashData, args -= arg_size; free(args);
- return "Email was sent to: " + m_sEmailTo; + return ssprintf("Email was sent to: %s", m_email_to); }
void CMailx::SetSettings(const map_plugin_settings_t& pSettings) @@ -155,36 +122,29 @@ void CMailx::SetSettings(const map_plugin_settings_t& pSettings) it = pSettings.find("Subject"); if (it != end) { - m_sSubject = it->second; + free(m_subject); + m_subject = xstrdup(it->second.c_str()); } + it = pSettings.find("EmailFrom"); if (it != end) { - m_sEmailFrom = it->second; + free(m_email_from); + m_email_from = xstrdup(it->second.c_str()); } + it = pSettings.find("EmailTo"); if (it != end) { - m_sEmailTo = it->second; + free(m_email_to); + m_email_to = xstrdup(it->second.c_str()); } + it = pSettings.find("SendBinaryData"); if (it != end) - { - m_bSendBinaryData = string_to_bool(it->second.c_str()); - } + m_send_binary_data = string_to_bool(it->second.c_str()); }
-//ok to delete? -//const map_plugin_settings_t& CMailx::GetSettings() -//{ -// m_pSettings["Subject"] = m_sSubject; -// m_pSettings["EmailFrom"] = m_sEmailFrom; -// m_pSettings["EmailTo"] = m_sEmailTo; -// m_pSettings["SendBinaryData"] = m_bSendBinaryData ? "yes" : "no"; -// -// return m_pSettings; -//} - PLUGIN_INFO(REPORTER, CMailx, "Mailx", diff --git a/lib/plugins/Mailx.h b/lib/plugins/Mailx.h index aa870ec..326a637 100644 --- a/lib/plugins/Mailx.h +++ b/lib/plugins/Mailx.h @@ -29,17 +29,17 @@ class CMailx : public CReporter { private: - std::string m_sEmailFrom; - std::string m_sEmailTo; - std::string m_sSubject; - bool m_bSendBinaryData; + char *m_email_from; + char *m_email_to; + char *m_subject; + bool m_send_binary_data;
public: CMailx(); + ~CMailx();
virtual void SetSettings(const map_plugin_settings_t& pSettings); -//ok to delete? -// virtual const map_plugin_settings_t& GetSettings(); + virtual std::string Report(const map_crash_data_t& pCrashData, const map_plugin_settings_t& pSettings, const char *pArgs); diff --git a/lib/utils/make_descr.cpp b/lib/utils/make_descr.cpp index 348795c..f865602 100644 --- a/lib/utils/make_descr.cpp +++ b/lib/utils/make_descr.cpp @@ -88,6 +88,48 @@ static const char *const blacklisted_items[] = { NULL };
+char* make_dsc_mailx(const map_crash_data_t & crash_data) +{ + struct strbuf *buf_dsc = strbuf_new(); + struct strbuf *buf_additional_files = strbuf_new(); + struct strbuf *buf_duphash_file = strbuf_new(); + struct strbuf *buf_common_files = strbuf_new(); + + map_crash_data_t::const_iterator it; + for (it = crash_data.begin(); it != crash_data.end(); it++) + { + if (it->second[CD_TYPE] == CD_TXT) + { + const char *itemname = it->first.c_str(); + if ((strcmp(itemname, CD_DUPHASH) != 0) + && (strcmp(itemname, FILENAME_ARCHITECTURE) != 0) + && (strcmp(itemname, FILENAME_KERNEL) != 0) + && (strcmp(itemname, FILENAME_PACKAGE) != 0) + ) { + strbuf_append_strf(buf_additional_files, "%s\n-----\n%s\n\n", itemname, it->second[CD_CONTENT].c_str()); + } + else if (strcmp(itemname, CD_DUPHASH) == 0) + strbuf_append_strf(buf_duphash_file, "%s\n-----\n%s\n\n", itemname, it->second[CD_CONTENT].c_str()); + else + strbuf_append_strf(buf_common_files, "%s\n-----\n%s\n\n", itemname, it->second[CD_CONTENT].c_str()); + } + } + + char *common_files = strbuf_free_nobuf(buf_common_files); + char *duphash_file = strbuf_free_nobuf(buf_duphash_file); + char *additional_files = strbuf_free_nobuf(buf_additional_files); + + strbuf_append_strf(buf_dsc, "Duplicate check\n=====\n%s\n\n", duphash_file); + strbuf_append_strf(buf_dsc, "Common information\n=====\n%s\n\n", common_files); + strbuf_append_strf(buf_dsc, "Additional information\n=====\n%s\n", additional_files); + + free(common_files); + free(duphash_file); + free(additional_files); + + return strbuf_free_nobuf(buf_dsc); +} + char* make_description_bz(const map_crash_data_t& pCrashData) { struct strbuf *buf_dsc = strbuf_new();
On Thu, 2010-08-12 at 16:28 +0200, Nikola Pajkovsky wrote:
}
- it = pSettings.find("EmailFrom"); if (it != end) {
m_sEmailFrom = it->second;
free(m_email_from);m_email_from = xstrdup(it->second.c_str());- it = pSettings.find("EmailTo"); if (it != end) {
m_sEmailTo = it->second;
free(m_email_to);m_email_to = xstrdup(it->second.c_str());- it = pSettings.find("SendBinaryData"); if (it != end)
- {
m_bSendBinaryData = string_to_bool(it->second.c_str());- }
m_send_binary_data = string_to_bool(it->second.c_str());
I'd avoid spurious changes like these empty line additions, and { } removal also only adds churn.
Otherwise looks good.
On 08/17/2010 02:20 PM, Denys Vlasenko wrote:
On Thu, 2010-08-12 at 16:28 +0200, Nikola Pajkovsky wrote:
}
it = pSettings.find("EmailFrom"); if (it != end) {
m_sEmailFrom = it->second;
free(m_email_from);m_email_from = xstrdup(it->second.c_str()); }it = pSettings.find("EmailTo"); if (it != end) {
m_sEmailTo = it->second;
free(m_email_to);m_email_to = xstrdup(it->second.c_str()); }it = pSettings.find("SendBinaryData"); if (it != end)
- {
m_bSendBinaryData = string_to_bool(it->second.c_str());- }
m_send_binary_data = string_to_bool(it->second.c_str());I'd avoid spurious changes like these empty line additions, and { } removal also only adds churn.
Otherwise looks good.
Ok, no problem.
Signed-off-by: Nikola Pajkovsky npajkovs@redhat.com --- lib/plugins/Logger.cpp | 40 +++++++++++++++++----------------------- lib/plugins/Logger.h | 8 ++++---- 2 files changed, 21 insertions(+), 27 deletions(-)
diff --git a/lib/plugins/Logger.cpp b/lib/plugins/Logger.cpp index a02845f..0c4aad6 100644 --- a/lib/plugins/Logger.cpp +++ b/lib/plugins/Logger.cpp @@ -24,10 +24,16 @@ #include "comm_layer_inner.h" #include "abrt_exception.h"
-CLogger::CLogger() : - m_sLogPath("/var/log/abrt.log"), - m_bAppendLogs(true) -{} +CLogger::CLogger() +{ + m_log_path = xstrdup("/var/log/abrt.log"); + m_append_logs = true; +} + +CLogger::~CLogger() +{ + free(m_log_path); +}
void CLogger::SetSettings(const map_plugin_settings_t& pSettings) { @@ -38,24 +44,14 @@ void CLogger::SetSettings(const map_plugin_settings_t& pSettings) it = pSettings.find("LogPath"); if (it != end) { - m_sLogPath = it->second; + free(m_log_path); + m_log_path = xstrdup(it->second.c_str()); } it = pSettings.find("AppendLogs"); if (it != end) - { - m_bAppendLogs = string_to_bool(it->second.c_str()); - } + m_append_logs = string_to_bool(it->second.c_str()); }
-//ok to delete? -//const map_plugin_settings_t& CLogger::GetSettings() -//{ -// m_pSettings["LogPath"] = m_sLogPath; -// m_pSettings["AppendLogs"] = m_bAppendLogs ? "yes" : "no"; -// -// return m_pSettings; -//} - std::string CLogger::Report(const map_crash_data_t& pCrashData, const map_plugin_settings_t& pSettings, const char *pArgs) @@ -65,10 +61,8 @@ std::string CLogger::Report(const map_crash_data_t& pCrashData, free(dsc);
/* open, not fopen - want to set mode if we create the file, not just open */ - const char *fname = m_sLogPath.c_str(); - int fd = open(fname, - m_bAppendLogs ? O_WRONLY|O_CREAT|O_APPEND : O_WRONLY|O_CREAT|O_TRUNC, - 0600); + const char *fname = m_log_path; + int fd = open(fname, m_append_logs ? O_WRONLY|O_CREAT|O_APPEND : O_WRONLY|O_CREAT|O_TRUNC, 0600); if (fd < 0) throw CABRTException(EXCEP_PLUGIN, "Can't open '%s'", fname);
@@ -78,8 +72,8 @@ std::string CLogger::Report(const map_crash_data_t& pCrashData,
close(fd);
- const char *format = m_bAppendLogs ? _("The report was appended to %s") : _("The report was stored to %s"); - return ssprintf(format, m_sLogPath.c_str()); + const char *format = m_append_logs ? _("The report was appended to %s") : _("The report was stored to %s"); + return ssprintf(format, m_log_path); }
PLUGIN_INFO(REPORTER, diff --git a/lib/plugins/Logger.h b/lib/plugins/Logger.h index aa7def3..d2b8007 100644 --- a/lib/plugins/Logger.h +++ b/lib/plugins/Logger.h @@ -28,14 +28,14 @@ class CLogger : public CReporter { private: - std::string m_sLogPath; - bool m_bAppendLogs; + char *m_log_path; + bool m_append_logs; public: CLogger(); + ~CLogger();
virtual void SetSettings(const map_plugin_settings_t& pSettings); -//ok to delete? -// virtual const map_plugin_settings_t& GetSettings(); + virtual std::string Report(const map_crash_data_t& pCrashData, const map_plugin_settings_t& pSettings, const char *pArgs);
On Thu, 2010-08-12 at 16:28 +0200, Nikola Pajkovsky wrote:
--- a/lib/plugins/Bugzilla.cpp +++ b/lib/plugins/Bugzilla.cpp @@ -31,6 +31,8 @@ #define XML_RPC_SUFFIX "/xmlrpc.cgi" #define MAX_HOPS 5
+#include "strbuf.h"
I usually put all includes together at the top of the module.
- if ((Login == "") || (Password == ""))
- if ((strcmp(login, "") == 0) || (strcmp(password, "") == 0))
strcmp(login, "") == 0 => !login[0] or => login[0] == '\0'
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();no_ssl_verify = string_to_bool(settings["NoSSLVerify"].c_str());
...
- ctx bz_server(BugzillaXMLRPC.c_str(), SSLVerify);
- ctx bz_server(bugzilla_xmlrpc, no_ssl_verify);
Looks like you went back to "no SSL verify" name. We decided that having a negative name is confusing. Is it a mistake?
throw_if_xml_fault_occurred(&bz_server.env);
throw_xml_fault(&bz_server.env);
This change is unrelated, and should be in separate change.
@@ -919,58 +936,72 @@ void CReporterBugzilla::SetSettings(const map_plugin_settings_t& pSettings)
map_plugin_settings_t::const_iterator end = pSettings.end(); map_plugin_settings_t::const_iterator it;
- it = pSettings.find("BugzillaURL"); if (it != end) {
...
}
- it = pSettings.find("Login"); if (it != end) {
...
}
- it = pSettings.find("Password");
These small unrelated changes are better go in separately. Imagine you need to cherry pick a change later.
- VERB3 log("%s:%s:%s:%s:%s", __FILE__, __func__, m_bugzilla_url, m_login, m_password);
Looks like forgotten debugging
/* Should not be deleted (why?) */ +/* const map_plugin_settings_t& CReporterBugzilla::GetSettings() { +<<<<<<< HEAD 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["NoSSLVerify"] = m_no_ssl_verify ? "yes" : "no";
+>>>>>>> rid of std::string from bugzilla
return m_pSettings;} +*/
Looks like this needs fixing...
--- a/lib/utils/make_descr.cpp +++ b/lib/utils/make_descr.cpp @@ -29,13 +29,16 @@ # define _(S) (S) #endif
+#include "strbuf.h"
Here too please move it up to other includes.
- return howToReproduce + comment;
- if (!repro && !comment)
return NULL;
Is caller expecting that the result can be NULL, not merely ""?
On 08/17/2010 02:17 PM, Denys Vlasenko wrote:
On Thu, 2010-08-12 at 16:28 +0200, Nikola Pajkovsky wrote:
--- a/lib/plugins/Bugzilla.cpp +++ b/lib/plugins/Bugzilla.cpp @@ -31,6 +31,8 @@ #define XML_RPC_SUFFIX "/xmlrpc.cgi" #define MAX_HOPS 5
+#include "strbuf.h"
I usually put all includes together at the top of the module.
- if ((Login == "") || (Password == ""))
- if ((strcmp(login, "") == 0) || (strcmp(password, "") == 0))
strcmp(login, "") == 0 => !login[0] or => login[0] == '\0'
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();no_ssl_verify = string_to_bool(settings["NoSSLVerify"].c_str());...
- ctx bz_server(BugzillaXMLRPC.c_str(), SSLVerify);
- ctx bz_server(bugzilla_xmlrpc, no_ssl_verify);
Looks like you went back to "no SSL verify" name. We decided that having a negative name is confusing. Is it a mistake?
It works same as before, but it's badly named
throw_if_xml_fault_occurred(&bz_server.env);
throw_xml_fault(&bz_server.env);This change is unrelated, and should be in separate change.
Will be in next patch.
@@ -919,58 +936,72 @@ void CReporterBugzilla::SetSettings(const map_plugin_settings_t& pSettings)
map_plugin_settings_t::const_iterator end = pSettings.end(); map_plugin_settings_t::const_iterator it;
it = pSettings.find("BugzillaURL"); if (it != end) {...
}
it = pSettings.find("Login"); if (it != end) {...
}
it = pSettings.find("Password");These small unrelated changes are better go in separately. Imagine you need to cherry pick a change later.
Drop changes.
- VERB3 log("%s:%s:%s:%s:%s", __FILE__, __func__, m_bugzilla_url, m_login, m_password);
Looks like forgotten debugging
:)
/* Should not be deleted (why?) */ +/* const map_plugin_settings_t& CReporterBugzilla::GetSettings() { +<<<<<<< HEAD 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["NoSSLVerify"] = m_no_ssl_verify ? "yes" : "no";
+>>>>>>> rid of std::string from bugzilla
return m_pSettings;} +*/
Looks like this needs fixing...
--- a/lib/utils/make_descr.cpp +++ b/lib/utils/make_descr.cpp @@ -29,13 +29,16 @@ # define _(S) (S) #endif
+#include "strbuf.h"
Here too please move it up to other includes.
- return howToReproduce + comment;
- if (!repro&& !comment)
return NULL;Is caller expecting that the result can be NULL, not merely ""?
Yes. NULL means that no comment will be added into bugzilla.
Everything else is fixed.
crash-catcher@lists.fedorahosted.org
-
Denys Vlasenko -
Nikola Pajkovsky