fix memory leaks in bugzilla

4 files changed, 153 insertions, 46 deletions

diff --git a/lib/Plugins/Bugzilla.cpp b/lib/Plugins/Bugzilla.cpp
index 14eb87f..38a6978 100644
--- a/lib/Plugins/Bugzilla.cpp
+++ b/lib/Plugins/Bugzilla.cpp
@@ -39,7 +39,8 @@ 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);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
 
     xmlrpc_value* result = NULL;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "User.login", param, &result);
@@ -62,14 +63,16 @@ void ctx::logout()
     xmlrpc_env_init(&env);
 
     xmlrpc_value* param = xmlrpc_build_value(&env, "(s)", "");
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&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);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
 }
 
 bool ctx::check_cc_and_reporter(uint32_t bug_id, const char* login)
@@ -77,23 +80,36 @@ 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());
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
 
     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);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
 
     xmlrpc_value* reporter_member = NULL;
     xmlrpc_struct_find_value(&env, result, "reporter", &reporter_member);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+    {
+        xmlrpc_DECREF(result);
+        throw_xml_fault(&env);
+    }
 
     if (reporter_member)
     {
         const char* reporter = NULL;
         xmlrpc_read_string(&env, reporter_member, &reporter);
-        throw_if_xml_fault_occurred(&env);
+        if (env.fault_occurred)
+        {
+            xmlrpc_DECREF(result);
+            xmlrpc_DECREF(reporter_member);
+            throw_xml_fault(&env);
+        }
 
         bool eq = (strcmp(reporter, login) == 0);
         free((void*)reporter);
@@ -107,7 +123,11 @@ bool ctx::check_cc_and_reporter(uint32_t bug_id, const char* login)
 
     xmlrpc_value* cc_member = NULL;
     xmlrpc_struct_find_value(&env, result, "cc", &cc_member);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+    {
+        xmlrpc_DECREF(result);
+        throw_xml_fault(&env);
+    }
 
     if (cc_member)
     {
@@ -117,11 +137,22 @@ bool ctx::check_cc_and_reporter(uint32_t bug_id, const char* login)
         {
             xmlrpc_value* item = NULL;
             xmlrpc_array_read_item(&env, cc_member, i, &item); // Correct
-            throw_if_xml_fault_occurred(&env);
+            if (env.fault_occurred)
+            {
+                xmlrpc_DECREF(result);
+                xmlrpc_DECREF(cc_member);
+                throw_xml_fault(&env);
+            }
 
             const char* cc = NULL;
             xmlrpc_read_string(&env, item, &cc);
-            throw_if_xml_fault_occurred(&env);
+            if (env.fault_occurred)
+            {
+                xmlrpc_DECREF(result);
+                xmlrpc_DECREF(cc_member);
+                xmlrpc_DECREF(item);
+                throw_xml_fault(&env);
+            }
 
             bool eq = (strcmp(cc, login) == 0);
             free((void*)cc);
@@ -145,15 +176,19 @@ void ctx::add_plus_one_cc(uint32_t bug_id, const char* login)
     xmlrpc_env env;
     xmlrpc_env_init(&env);
 
-    xmlrpc_value* param = xmlrpc_build_value(&env, "({s:i,s:{s:(s)}})", "ids", bug_id, "updates", "add_cc", login);
-    throw_if_xml_fault_occurred(&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);
 
     xmlrpc_value* result = NULL;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "Bug.update", param, &result);
-    throw_if_xml_fault_occurred(&env);
+    // 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);
-    xmlrpc_DECREF(param);
 }
 
 int32_t ctx::check_uuid_in_bugzilla(const char* component, const char* UUID)
@@ -163,36 +198,61 @@ 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());
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
 
     xmlrpc_value* result = NULL;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "Bug.search", param, &result);
-    throw_if_xml_fault_occurred(&env);
+    // 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_value* bugs_member = NULL;
     xmlrpc_struct_find_value(&env, result, "bugs", &bugs_member);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+    {
+        xmlrpc_DECREF(result);
+        throw_xml_fault(&env);
+    }
 
     if (bugs_member)
     {
         // when array size is equal 0 that means no bug reported
         uint32_t array_size = xmlrpc_array_size(&env, bugs_member);
-        throw_if_xml_fault_occurred(&env);
-        if (array_size == 0)
+        if (env.fault_occurred)
         {
+            xmlrpc_DECREF(result);
             xmlrpc_DECREF(bugs_member);
+            throw_xml_fault(&env);
+        }
+        else if (array_size == 0)
+        {
             xmlrpc_DECREF(result);
+            xmlrpc_DECREF(bugs_member);
             return -1;
         }
 
         xmlrpc_value* item = NULL;
         xmlrpc_array_read_item(&env, bugs_member, 0, &item); // Correct
-        throw_if_xml_fault_occurred(&env);
+        if (env.fault_occurred)
+        {
+            xmlrpc_DECREF(result);
+            xmlrpc_DECREF(bugs_member);
+            throw_xml_fault(&env);
+        }
+
         xmlrpc_value* bug = NULL;
         xmlrpc_struct_find_value(&env, item, "bug_id", &bug);
-        throw_if_xml_fault_occurred(&env);
+        if (env.fault_occurred)
+        {
+            xmlrpc_DECREF(result);
+            xmlrpc_DECREF(bugs_member);
+            xmlrpc_DECREF(item);
+            throw_xml_fault(&env);
+        }
 
         if (bug)
         {
@@ -236,6 +296,7 @@ uint32_t ctx::new_bug(const map_crash_report_t& pCrashReport)
     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(),
@@ -245,27 +306,39 @@ uint32_t ctx::new_bug(const map_crash_report_t& pCrashReport)
                                         "status_whiteboard", status_whiteboard.c_str(),
                                         "platform", arch.c_str()
                               );
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
 
     xmlrpc_value* result;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "Bug.create", param, &result);
-    throw_if_xml_fault_occurred(&env);
+    // 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_value* id;
     xmlrpc_struct_find_value(&env, result, "id", &id);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+    {
+        xmlrpc_DECREF(result);
+        throw_xml_fault(&env);
+    }
 
     xmlrpc_int bug_id = -1;
     if (id)
     {
         xmlrpc_read_int(&env, id, &bug_id);
-        throw_if_xml_fault_occurred(&env);
+        if (env.fault_occurred)
+        {
+            xmlrpc_DECREF(result);
+            xmlrpc_DECREF(id);
+            throw_xml_fault(&env);
+        }
         log("New bug id: %i", bug_id);
         update_client(_("New bug id: %i"), bug_id);
     }
 
     xmlrpc_DECREF(result);
-    xmlrpc_DECREF(param);
     xmlrpc_DECREF(id);
     return bug_id;
 }
@@ -287,6 +360,7 @@ void ctx::add_attachments(const char* bug_id_str, const map_crash_report_t& pCra
         if (type == CD_ATT)
         {
             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: " + filename).c_str(),
@@ -295,12 +369,18 @@ void ctx::add_attachments(const char* bug_id_str, const map_crash_report_t& pCra
                                               "data", encoded64
                                       );
             free(encoded64);
-            throw_if_xml_fault_occurred(&env);
+            if (env.fault_occurred)
+                throw_xml_fault(&env);
 
             xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "bugzilla.addAttachment", param, &result);
-            xmlrpc_DECREF(result);
+            // we don't need anymore xml structure for calling xmlrpc query(calls only once)
             xmlrpc_DECREF(param);
-            throw_if_xml_fault_occurred(&env);
+            if (env.fault_occurred)
+            {
+                xmlrpc_DECREF(result);
+                throw_xml_fault(&env);
+            }
+            xmlrpc_DECREF(result);
         }
     }
 }
diff --git a/lib/Plugins/Catcut.cpp b/lib/Plugins/Catcut.cpp
index 52efc57..27b868d 100644
--- a/lib/Plugins/Catcut.cpp
+++ b/lib/Plugins/Catcut.cpp
@@ -167,11 +167,14 @@ struct_find_int(xmlrpc_env* env, xmlrpc_value* result,
 {
     xmlrpc_value* an_xmlrpc_value;
     xmlrpc_struct_find_value(env, result, fieldName, &an_xmlrpc_value);
-    throw_if_xml_fault_occurred(env);
+    if (env->fault_occurred)
+        throw_xml_fault(env);
+
     if (an_xmlrpc_value)
     {
         xmlrpc_read_int(env, an_xmlrpc_value, &value);
-        throw_if_xml_fault_occurred(env);
+        if (env->fault_occurred)
+            throw_xml_fault(env);
         xmlrpc_DECREF(an_xmlrpc_value);
         return true;
     }
@@ -184,12 +187,14 @@ struct_find_string(xmlrpc_env* env, xmlrpc_value* result,
 {
     xmlrpc_value* an_xmlrpc_value;
     xmlrpc_struct_find_value(env, result, fieldName, &an_xmlrpc_value);
-    throw_if_xml_fault_occurred(env);
+    if (env->fault_occurred)
+        throw_xml_fault(env);
     if (an_xmlrpc_value)
     {
         const char* value_s;
         xmlrpc_read_string(env, an_xmlrpc_value, &value_s);
-        throw_if_xml_fault_occurred(env);
+        if (env->fault_occurred)
+            throw_xml_fault(env);
         value = value_s;
         xmlrpc_DECREF(an_xmlrpc_value);
         free((void*)value_s);
@@ -228,20 +233,24 @@ ctx::login(const char* login, const char* passwd)
     xmlrpc_env_init(&env);
 
     xmlrpc_value* param = xmlrpc_build_value(&env, "(ss)", login, passwd);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
 
     xmlrpc_value* result;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "Catcut.auth", param, &result);
     xmlrpc_DECREF(param);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
 
     xmlrpc_value *cookie_xml;
     const char *cookie;
     string cookie_str;
     xmlrpc_struct_find_value(&env, result, "cookie", &cookie_xml);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
     xmlrpc_read_string(&env, cookie_xml, &cookie);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
     cookie_str = cookie;
     /* xmlrpc_read_string returns *malloc'ed ptr*.
      * doc is not very clear on it, but I looked in xmlrpc sources. */
@@ -284,20 +293,24 @@ ctx::new_bug(const char *auth_cookie, const map_crash_report_t& pCrashReport)
                 "status_whiteboard", status_whiteboard.c_str(),
                 "platform", arch.c_str()
                 );
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
 
     xmlrpc_value *result;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "Catcut.createTicket", param, &result);
     xmlrpc_DECREF(param);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
 
     xmlrpc_value *bug_id_xml;
     const char *bug_id;
     string bug_id_str;
     xmlrpc_struct_find_value(&env, result, "ticket", &bug_id_xml);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
     xmlrpc_read_string(&env, bug_id_xml, &bug_id);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
     bug_id_str = bug_id;
     log("New bug id: %s", bug_id);
     update_client(_("New bug id: %s"), bug_id);
@@ -321,12 +334,14 @@ ctx::request_upload(const char* auth_cookie, const char* pTicketName,
                 pTicketName,
                 fileName,
                 description);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
 
     xmlrpc_value* result = NULL;
     xmlrpc_client_call2(&env, m_pClient, m_pServer_info, "Catcut.requestUpload", param, &result);
     xmlrpc_DECREF(param);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&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 ae1d098..e3a4648 100644
--- a/lib/Utils/abrt_xmlrpc.cpp
+++ b/lib/Utils/abrt_xmlrpc.cpp
@@ -15,6 +15,16 @@ 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);
+    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());
+}
+#else
 void throw_if_xml_fault_occurred(xmlrpc_env *env)
 {
     if (env->fault_occurred)
@@ -26,6 +36,7 @@ void throw_if_xml_fault_occurred(xmlrpc_env *env)
         throw CABRTException(EXCEP_PLUGIN, errmsg.c_str());
     }
 }
+#endif
 
 void abrt_xmlrpc_conn::new_xmlrpc_client(const char* url, bool no_ssl_verify)
 {
@@ -60,15 +71,16 @@ void abrt_xmlrpc_conn::new_xmlrpc_client(const char* url, bool no_ssl_verify)
                         PACKAGE_NAME, VERSION,
                         &clientParms, XMLRPC_CPSIZE(transportparm_size),
                         &m_pClient);
-    throw_if_xml_fault_occurred(&env);
+    if (env.fault_occurred)
+        throw_xml_fault(&env);
 
     m_pServer_info = xmlrpc_server_info_new(&env, url);
     if (env.fault_occurred)
     {
         xmlrpc_client_destroy(m_pClient);
         m_pClient = NULL;
+        throw_xml_fault(&env);
     }
-    throw_if_xml_fault_occurred(&env);
 }
 
 void abrt_xmlrpc_conn::destroy_xmlrpc_client()
diff --git a/lib/Utils/abrt_xmlrpc.h b/lib/Utils/abrt_xmlrpc.h
index 352e80c..a8bd2cc 100644
--- a/lib/Utils/abrt_xmlrpc.h
+++ b/lib/Utils/abrt_xmlrpc.h
@@ -24,7 +24,7 @@ struct abrt_xmlrpc_conn {
 };
 
 /* Utility functions */
-void throw_if_xml_fault_occurred(xmlrpc_env *env);
+void throw_xml_fault(xmlrpc_env *env);
 CURL* xcurl_easy_init();
 
 #endif