On 06/21/2010 01:16 PM, Denys Vlasenko wrote:
On Fri, 2010-06-18 at 16:54 +0200, Nikola Pajkovsky wrote:
> rhbz578397 - abrt should not point to Fedora bugs but create new RHEL bug
> instead. Also when it finds some bugs in fedora it will depend on them
>
> Signed-off-by: Nikola Pajkovsky<npajkovs(a)redhat.com>
> - xmlrpc_int32 new_bug(const map_crash_data_t& pCrashData);
> + xmlrpc_int32 new_bug(const map_crash_data_t& pCrashData, int depon);
"depon" name is not informative enough...
> -xmlrpc_int32 ctx::new_bug(const map_crash_data_t& pCrashData)
> +xmlrpc_int32 ctx::new_bug(const map_crash_data_t& pCrashData, int depon)
> {
> const std::string& package = get_crash_data_item_content(pCrashData,
FILENAME_PACKAGE);
> const std::string& component = get_crash_data_item_content(pCrashData,
FILENAME_COMPONENT);
> @@ -379,7 +418,23 @@ xmlrpc_int32 ctx::new_bug(const map_crash_data_t&
pCrashData)
> char *version = NULL;
> parse_release(release.c_str(),&product,&version);
>
> - xmlrpc_value* result = call("Bug.create",
"({s:s,s:s,s:s,s:s,s:s,s:s,s:s})",
> + xmlrpc_value* result = NULL;
> + if (depon> 0)
> + {
> + 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(),
> + "version", version,
> + "summary", summary.c_str(),
> + "description", description.c_str(),
> + "status_whiteboard",
status_whiteboard.c_str(),
> + "platform", arch.c_str(),
> + "dependson", depon
> + );
> + }
> + else
Aha, it's a *bug number* new bug depends on?
Yes
I recommend renaming: say, depon -> depend_on_bugno.
That
wasn't clever name, I've used yours.
> @@ -449,6 +506,11 @@ int ctx::get_bug_info(struct bug_info* bz, xmlrpc_int32 bug_id)
> if (result)
> {
> // mandatory
> + bz->bug_product = get_bug_product(result);
> + if (bz->bug_product == NULL)
> + return -1;
> +
> + // mandatory
> bz->bug_status = get_bug_status(result);
> if (bz->bug_status == NULL)
> return -1;
"// mandatory" comment is repeated.
Erased all comments "//
mandatory"
> @@ -631,13 +694,16 @@ std::string CReporterBugzilla::Report(const
map_crash_data_t& pCrashData,
> bz_server.login(Login.c_str(), Password.c_str());
>
> update_client(_("Checking for duplicates..."));
> - xmlrpc_value* result = bz_server.call_quicksearch_duphash(component.c_str(),
duphash.c_str());
> +
> + // will be used more times
I do not understand what the above comment tries to say.
Can you remove or rephrase it?
Erased all comments "// will be used more times"
> + xmlrpc_value *result = NULL;
> + result = bz_server.call_quicksearch_duphash(component.c_str(), NULL,
duphash.c_str());
gcc will optimize redundant assignment, but I don't see what's
wrong with simply doing it in one line:
xmlrpc_value *result = bz_server.call_quicksearch_duphash(...)
Fixed.
> if (!result)
> - {
> throw_if_xml_fault_occurred(&bz_server.env);
> - }
>
> - xmlrpc_value* all_bugs = bz_server.get_member("bugs", result);
> + // will be used more times
> + xmlrpc_value *all_bugs = NULL;
> + all_bugs = bz_server.get_member("bugs", result);
> xmlrpc_DECREF(result);
Same
>
> if (!all_bugs)
> @@ -646,7 +712,67 @@ std::string CReporterBugzilla::Report(const
map_crash_data_t& pCrashData,
> throw CABRTException(EXCEP_PLUGIN, _("Missing mandatory member
'bugs'"));
> }
>
> - int all_bugs_size = bz_server.get_array_size(all_bugs);
> + // will be used more times
> + int all_bugs_size = -1;
> + all_bugs_size = bz_server.get_array_size(all_bugs);
> + bug_id = bz_server.get_bug_id(all_bugs);
Same
Apart from these, looks good to me.
Thanks. Pushed into git.