Bugzilla,Catcut: fix or remove incomprehensible comments; fix error msg

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>

4 files changed, 37 insertions, 86 deletions

diff --git a/lib/Plugins/Bugzilla.cpp b/lib/Plugins/Bugzilla.cpp
index 0197a7bf..fa6b35e4 100644
--- a/lib/Plugins/Bugzilla.cpp
+++ b/lib/Plugins/Bugzilla.cpp
@@ -44,8 +44,7 @@ void ctx::login(const char* login, const char* passwd)
     xmlrpc_env_init(&env);
 
     xmlrpc_value* param = xmlrpc_build_value(&env, "({s:s,s:s})", "login", login, "password", passwd);
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env);
 
     xmlrpc_value* result = NULL;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "User.login", param, &result);
@@ -68,16 +67,13 @@ void ctx::logout()
     xmlrpc_env_init(&env);
 
     xmlrpc_value* param = xmlrpc_build_value(&env, "(s)", "");
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env);
 
     xmlrpc_value* result = NULL;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "User.logout", param, &result);
     xmlrpc_DECREF(param);
-    if (result)
-        xmlrpc_DECREF(result);
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    xmlrpc_DECREF(result);
+    throw_if_xml_fault_occurred(&env);
 }
 
 bool ctx::check_cc_and_reporter(uint32_t bug_id, const char* login)
@@ -85,17 +81,13 @@ bool ctx::check_cc_and_reporter(uint32_t bug_id, const char* login)
     xmlrpc_env env;
     xmlrpc_env_init(&env);
 
-    // fails only when you write query. when it's done it never fails.
     xmlrpc_value* param = xmlrpc_build_value(&env, "(s)", to_string(bug_id).c_str());
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env); // failed to allocate memory
 
     xmlrpc_value* result = NULL;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "bugzilla.getBug", param, &result);
-    // we don't need anymore xml structure for calling xmlrpc query(calls only once)
     xmlrpc_DECREF(param);
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env);
 
     xmlrpc_value* reporter_member = NULL;
     xmlrpc_struct_find_value(&env, result, "reporter", &reporter_member);
@@ -129,7 +121,7 @@ bool ctx::check_cc_and_reporter(uint32_t bug_id, const char* login)
     {
         VERB3 log("Missing member 'reporter'");
         xmlrpc_DECREF(result);
-        throw CABRTException(EXCEP_PLUGIN, _("Missing member 'bugs'"));
+        throw CABRTException(EXCEP_PLUGIN, _("Missing member 'reporter'"));
     }
 
     xmlrpc_value* cc_member = NULL;
@@ -179,7 +171,7 @@ bool ctx::check_cc_and_reporter(uint32_t bug_id, const char* login)
     }
     else
     {
-        VERB3 log("Missing member 'bugs'");
+        VERB3 log("Missing member 'cc'");
         xmlrpc_DECREF(result);
         throw CABRTException(EXCEP_PLUGIN, _("Missing member 'cc'"));
     }
@@ -193,19 +185,14 @@ void ctx::add_plus_one_cc(uint32_t bug_id, const char* login)
     xmlrpc_env env;
     xmlrpc_env_init(&env);
 
-    // fails only when you write query. when it's done it never fails.
     xmlrpc_value* param = xmlrpc_build_value(&env, "({s:i,s:{s:(s)}})", "ids", bug_id, "updates", "add_cc", login); 
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env); // failed to allocate memory
 
     xmlrpc_value* result = NULL;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "Bug.update", param, &result);
-    // we don't need anymore xml structure for calling xmlrpc query(calls only once)
     xmlrpc_DECREF(param);
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
-
     xmlrpc_DECREF(result);
+    throw_if_xml_fault_occurred(&env);
 }
 
 int32_t ctx::check_uuid_in_bugzilla(const char* component, const char* UUID)
@@ -215,17 +202,13 @@ int32_t ctx::check_uuid_in_bugzilla(const char* component, const char* UUID)
 
     std::string query = ssprintf("ALL component:\"%s\" statuswhiteboard:\"%s\"", component, UUID);
 
-    // fails only when you write query. when it's done it never fails.
     xmlrpc_value* param = xmlrpc_build_value(&env, "({s:s})", "quicksearch", query.c_str());
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env); // failed to allocate memory
 
     xmlrpc_value* result = NULL;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "Bug.search", param, &result);
-    // we don't need anymore xml structure for calling xmlrpc query(calls only once)
     xmlrpc_DECREF(param);
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env); // on error result is NULL, no need to DECREF
 
     xmlrpc_value* bugs_member = NULL;
     xmlrpc_struct_find_value(&env, result, "bugs", &bugs_member);
@@ -237,7 +220,7 @@ int32_t ctx::check_uuid_in_bugzilla(const char* component, const char* UUID)
 
     if (bugs_member)
     {
-        // when array size is equal 0 that means no bug reported
+        // when array size is 0 that means no bug reported
         uint32_t array_size = xmlrpc_array_size(&env, bugs_member);
         if (env.fault_occurred)
         {
@@ -325,7 +308,6 @@ uint32_t ctx::new_bug(const map_crash_data_t& pCrashData)
     std::string version;
     parse_release(release.c_str(), product, version);
 
-    // fails only when you write query. when it's done it never fails.
     xmlrpc_value* param = xmlrpc_build_value(&env, "({s:s,s:s,s:s,s:s,s:s,s:s,s:s})",
                                         "product", product.c_str(),
                                         "component", component.c_str(),
@@ -335,17 +317,14 @@ uint32_t ctx::new_bug(const map_crash_data_t& pCrashData)
                                         "status_whiteboard", status_whiteboard.c_str(),
                                         "platform", arch.c_str()
                               );
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env); // failed to allocate memory
 
-    xmlrpc_value* result;
+    xmlrpc_value* result = NULL;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "Bug.create", param, &result);
-    // we don't need anymore xml structure for calling xmlrpc query(calls only once)
     xmlrpc_DECREF(param);
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env);
 
-    xmlrpc_value* id;
+    xmlrpc_value* id = NULL;
     xmlrpc_struct_find_value(&env, result, "id", &id);
     if (env.fault_occurred)
     {
@@ -377,8 +356,6 @@ void ctx::add_attachments(const char* bug_id_str, const map_crash_data_t& pCrash
     xmlrpc_env env;
     xmlrpc_env_init(&env);
 
-    xmlrpc_value* result = NULL;
-
     map_crash_data_t::const_iterator it = pCrashData.begin();
     for (; it != pCrashData.end(); it++)
     {
@@ -390,7 +367,6 @@ void ctx::add_attachments(const char* bug_id_str, const map_crash_data_t& pCrash
          && (content.length() > CD_TEXT_ATT_SIZE || itemname == FILENAME_BACKTRACE)
         ) {
             char *encoded64 = encode_base64(content.c_str(), content.length());
-            // fails only when you write query. when it's done it never fails.
             xmlrpc_value* param = xmlrpc_build_value(&env, "(s{s:s,s:s,s:s,s:s})",
                                               bug_id_str,
                                               "description", ("File: " + itemname).c_str(),
@@ -399,18 +375,13 @@ void ctx::add_attachments(const char* bug_id_str, const map_crash_data_t& pCrash
                                               "data", encoded64
                                       );
             free(encoded64);
-            if (env.fault_occurred)
-                throw_xml_fault(&env);
+            throw_if_xml_fault_occurred(&env); // failed to allocate memory
 
+            xmlrpc_value* result = NULL;
             xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "bugzilla.addAttachment", param, &result);
-            // we don't need anymore xml structure for calling xmlrpc query(calls only once)
             xmlrpc_DECREF(param);
-            if (env.fault_occurred)
-            {
-                xmlrpc_DECREF(result);
-                throw_xml_fault(&env);
-            }
             xmlrpc_DECREF(result);
+            throw_if_xml_fault_occurred(&env);
         }
     }
 }
diff --git a/lib/Plugins/Catcut.cpp b/lib/Plugins/Catcut.cpp
index 3580a3b4..badca4b4 100644
--- a/lib/Plugins/Catcut.cpp
+++ b/lib/Plugins/Catcut.cpp
@@ -167,14 +167,12 @@ struct_find_int(xmlrpc_env* env, xmlrpc_value* result,
 {
     xmlrpc_value* an_xmlrpc_value;
     xmlrpc_struct_find_value(env, result, fieldName, &an_xmlrpc_value);
-    if (env->fault_occurred)
-        throw_xml_fault(env);
+    throw_if_xml_fault_occurred(env);
 
     if (an_xmlrpc_value)
     {
         xmlrpc_read_int(env, an_xmlrpc_value, &value);
-        if (env->fault_occurred)
-            throw_xml_fault(env);
+        throw_if_xml_fault_occurred(env);
         xmlrpc_DECREF(an_xmlrpc_value);
         return true;
     }
@@ -187,14 +185,12 @@ struct_find_string(xmlrpc_env* env, xmlrpc_value* result,
 {
     xmlrpc_value* an_xmlrpc_value;
     xmlrpc_struct_find_value(env, result, fieldName, &an_xmlrpc_value);
-    if (env->fault_occurred)
-        throw_xml_fault(env);
+    throw_if_xml_fault_occurred(env);
     if (an_xmlrpc_value)
     {
         const char* value_s;
         xmlrpc_read_string(env, an_xmlrpc_value, &value_s);
-        if (env->fault_occurred)
-            throw_xml_fault(env);
+        throw_if_xml_fault_occurred(env);
         value = value_s;
         xmlrpc_DECREF(an_xmlrpc_value);
         free((void*)value_s);
@@ -233,24 +229,20 @@ ctx::login(const char* login, const char* passwd)
     xmlrpc_env_init(&env);
 
     xmlrpc_value* param = xmlrpc_build_value(&env, "(ss)", login, passwd);
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env);
 
-    xmlrpc_value* result;
+    xmlrpc_value* result = NULL;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "Catcut.auth", param, &result);
     xmlrpc_DECREF(param);
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env);
 
     xmlrpc_value *cookie_xml;
     const char *cookie;
     string cookie_str;
     xmlrpc_struct_find_value(&env, result, "cookie", &cookie_xml);
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env);
     xmlrpc_read_string(&env, cookie_xml, &cookie);
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env);
     cookie_str = cookie;
     /* xmlrpc_read_string returns *malloc'ed ptr*.
      * doc is not very clear on it, but I looked in xmlrpc sources. */
@@ -293,24 +285,20 @@ ctx::new_bug(const char *auth_cookie, const map_crash_data_t& pCrashData)
                 "status_whiteboard", status_whiteboard.c_str(),
                 "platform", arch.c_str()
                 );
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env);
 
     xmlrpc_value *result;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "Catcut.createTicket", param, &result);
     xmlrpc_DECREF(param);
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env);
 
     xmlrpc_value *bug_id_xml;
     const char *bug_id;
     string bug_id_str;
     xmlrpc_struct_find_value(&env, result, "ticket", &bug_id_xml);
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env);
     xmlrpc_read_string(&env, bug_id_xml, &bug_id);
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env);
     bug_id_str = bug_id;
     log("New bug id: %s", bug_id);
     update_client(_("New bug id: %s"), bug_id);
@@ -334,14 +322,12 @@ ctx::request_upload(const char* auth_cookie, const char* pTicketName,
                 pTicketName,
                 fileName,
                 description);
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env);
 
     xmlrpc_value* result = NULL;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "Catcut.requestUpload", param, &result);
     xmlrpc_DECREF(param);
-    if (env.fault_occurred)
-        throw_xml_fault(&env);
+    throw_if_xml_fault_occurred(&env);
 
     string URL;
     bool has_URL = struct_find_string(&env, result, "uri", URL);
diff --git a/lib/Utils/abrt_xmlrpc.cpp b/lib/Utils/abrt_xmlrpc.cpp
index e3a4648a..c2d716e1 100644
--- a/lib/Utils/abrt_xmlrpc.cpp
+++ b/lib/Utils/abrt_xmlrpc.cpp
@@ -15,7 +15,6 @@ CURL* xcurl_easy_init()
     return curl;
 }
 
-#if 1
 void throw_xml_fault(xmlrpc_env *env)
 {
     std::string errmsg = ssprintf("XML-RPC Fault: %s(%d)", env->fault_string, env->fault_code);
@@ -24,19 +23,13 @@ void throw_xml_fault(xmlrpc_env *env)
     error_msg("%s", errmsg.c_str()); // show error in daemon log
     throw CABRTException(EXCEP_PLUGIN, errmsg.c_str());
 }
-#else
 void throw_if_xml_fault_occurred(xmlrpc_env *env)
 {
     if (env->fault_occurred)
     {
-        std::string errmsg = ssprintf("XML-RPC Fault: %s(%d)", env->fault_string, env->fault_code);
-        xmlrpc_env_clean(env); // this is needed ONLY if fault_occurred
-        xmlrpc_env_init(env); // just in case user catches ex and _continues_ to use env
-        error_msg("%s", errmsg.c_str()); // show error in daemon log
-        throw CABRTException(EXCEP_PLUGIN, errmsg.c_str());
+        throw_xml_fault(env);
     }
 }
-#endif
 
 void abrt_xmlrpc_conn::new_xmlrpc_client(const char* url, bool no_ssl_verify)
 {
diff --git a/lib/Utils/abrt_xmlrpc.h b/lib/Utils/abrt_xmlrpc.h
index a8bd2cca..a0e9ca34 100644
--- a/lib/Utils/abrt_xmlrpc.h
+++ b/lib/Utils/abrt_xmlrpc.h
@@ -25,6 +25,7 @@ struct abrt_xmlrpc_conn {
 
 /* Utility functions */
 void throw_xml_fault(xmlrpc_env *env);
+void throw_if_xml_fault_occurred(xmlrpc_env *env);
 CURL* xcurl_easy_init();
 
 #endif