Nikola, thank you for the patch, it seems to work well. I have tested
it both with and without remote_result.
I have simplified and improved the code a bit when reviewing it, and
also fixed the bug causing a termination of reporter-bugzilla when no
duplicate was found. Please consider commiting the following change on
the top of your changes:
diff --git a/src/include/problem_data.h b/src/include/problem_data.h
index 14d1240..9bf46f5 100644
--- a/src/include/problem_data.h
+++ b/src/include/problem_data.h
@@ -78,9 +78,9 @@ static inline struct problem_item
*get_problem_data_item_or_NULL(problem_data_t
{
return (struct problem_item *)g_hash_table_lookup(problem_data, key);
}
-const char *get_problem_item_content_or_NULL(problem_data_t *problem_data,
const char *key);
+char *get_problem_item_content_or_NULL(problem_data_t *problem_data, const
char *key);
/* Aborts if key is not found: */
-const char *get_problem_item_content_or_die(problem_data_t *problem_data,
const char *key);
+char *get_problem_item_content_or_die(problem_data_t *problem_data, const char
*key);
/* Conversions between in-memory and on-disk formats */
diff --git a/src/lib/problem_data.c b/src/lib/problem_data.c
index f864749..2a7835a 100644
--- a/src/lib/problem_data.c
+++ b/src/lib/problem_data.c
@@ -156,7 +156,7 @@ void add_to_problem_data(problem_data_t *problem_data,
add_to_problem_data_ext(problem_data, name, content, CD_FLAG_TXT +
CD_FLAG_ISNOTEDITABLE);
}
-const char *get_problem_item_content_or_die(problem_data_t *problem_data,
const char *key)
+char *get_problem_item_content_or_die(problem_data_t *problem_data, const char
*key)
{
struct problem_item *item = get_problem_data_item_or_NULL(problem_data,
key);
if (!item)
@@ -164,7 +164,7 @@ const char *get_problem_item_content_or_die(problem_data_t
*problem_data, const
return item->content;
}
-const char *get_problem_item_content_or_NULL(problem_data_t *problem_data,
const char *key)
+char *get_problem_item_content_or_NULL(problem_data_t *problem_data, const
char *key)
{
struct problem_item *item = get_problem_data_item_or_NULL(problem_data,
key);
if (!item)
diff --git a/src/plugins/reporter-bugzilla.c b/src/plugins/reporter-bugzilla.c
index 7c7472e..ff06c88 100644
--- a/src/plugins/reporter-bugzilla.c
+++ b/src/plugins/reporter-bugzilla.c
@@ -280,16 +280,12 @@ int main(int argc, char **argv)
parse_release_for_bz(rhbz.b_release, &rhbz.b_product, &version);
free(version);
- const char *filnam_remote_result;
- filnam_remote_result = get_problem_item_content_or_NULL(problem_data,
FILENAME_REMOTE_RESULT);
- if (filnam_remote_result)
+ char *remote_result;
+ remote_result = get_problem_item_content_or_NULL(problem_data,
FILENAME_REMOTE_RESULT);
+ if (remote_result)
{
- char *remote_result = xstrdup(filnam_remote_result);
- if (remote_result[strlen(remote_result) - 1] == '\n')
- remote_result[strlen(remote_result) - 1] = '\0';
-
- char *cmd = strtok(remote_result, " ");
- char *id = strtok(NULL, " ");
+ char *cmd = strtok((char*)remote_result, " \n");
+ char *id = strtok(NULL, " \n");
if (!prefixcmp(cmd, "DUPLICATE"))
{
@@ -313,15 +309,16 @@ int main(int argc, char **argv)
log(_("Checking for duplicates"));
/*
- selinux guy's almost always move filled bug from component
selinux-policy
- to right component.
+ SELinux guys almost always move filed bugs from component
+ selinux-policy to another component.
- bugzilla client is looking for duplicate bug by sending xmlrpc query
+ Bugzilla client is looking for duplicate bug by sending
+ xmlrpc query:
"ALL whiteboard:<hash> product:<product> component:<name>"
- so if bug is moved from component selinux-policy to other, then query
- returns NULL and creates a new bug.
+ So if bug is moved from component selinux-policy to other,
+ then query returns NULL and creates a new bug.
*/
const char *component_substitute = (!strcmp(component,
"selinux-policy")) ? NULL : component;
xmlrpc_value *result = rhbz_search_duphash(client,
component_substitute,
@@ -338,15 +335,11 @@ int main(int argc, char **argv)
VERB3 log("Bugzilla has %i reports with same duphash '%s'",
all_bugs_size, duphash);
- int bug_id = rhbz_bug_id(all_bugs);
- xmlrpc_DECREF(all_bugs);
- bz = rhbz_bug_info(client, bug_id);
-
if (all_bugs_size == 0)
{
/* Create new bug */
log(_("Creating a new bug"));
- bug_id = rhbz_new_bug(client, problem_data, rhbz.b_release);
+ int bug_id = rhbz_new_bug(client, problem_data, rhbz.b_release);
log(_("Adding attachments to bug %i"), bug_id);
char bug_id_str[sizeof(int)*3 + 2];
@@ -362,6 +355,12 @@ int main(int argc, char **argv)
bz->bi_id = bug_id;
goto log_out;
}
+ else
+ {
+ int bug_id = rhbz_bug_id(all_bugs);
+ xmlrpc_DECREF(all_bugs);
+ bz = rhbz_bug_info(client, bug_id);
+ }
}
else
{
Nikola Pajkovsky <[email protected]> writes:
> Deduplication client stores information into <dump-dir> into *remote_result*
> file.
> *Remote_result* file contains only one line in format DUPLICATE <bug-id> for
> now.
>
> Signed-off-by: Nikola Pajkovsky <[email protected]>
> ---
> src/plugins/reporter-bugzilla.c | 119
> +++++++++++++++++++++++++--------------
> src/plugins/rhbz.h | 5 +-
> 2 files changed, 81 insertions(+), 43 deletions(-)
>
> diff --git a/src/plugins/reporter-bugzilla.c b/src/plugins/reporter-bugzilla.c
> index 825c92a..7c7472e 100644
> --- a/src/plugins/reporter-bugzilla.c
> +++ b/src/plugins/reporter-bugzilla.c
> @@ -280,57 +280,92 @@ int main(int argc, char **argv)
> parse_release_for_bz(rhbz.b_release, &rhbz.b_product, &version);
> free(version);
>
> - log(_("Checking for duplicates"));
> + const char *filnam_remote_result;
> + filnam_remote_result = get_problem_item_content_or_NULL(problem_data,
> FILENAME_REMOTE_RESULT);
> + if (filnam_remote_result)
> + {
> + char *remote_result = xstrdup(filnam_remote_result);
> + if (remote_result[strlen(remote_result) - 1] == '\n')
> + remote_result[strlen(remote_result) - 1] = '\0';
> +
> + char *cmd = strtok(remote_result, " ");
> + char *id = strtok(NULL, " ");
> +
> + if (!prefixcmp(cmd, "DUPLICATE"))
> + {
> + int old_errno = errno;
> + errno = 0;
> + char *e;
> + rhbz.b_id = strtoul(id, &e, 10);
> + if (errno || id == e || *e != '\0' || rhbz.b_id > INT_MAX)
> + {
> + /* error / no digits / illegal trailing chars */
> + errno = old_errno;
> + rhbz.b_id = 0;
> + }
> + errno = old_errno; /* Ok. So restore errno. */
> + }
> + }
>
> - /*
> - selinux guy's almost always move filled bug from component
> selinux-policy
> - to right component.
> + struct bug_info *bz = NULL;
> + if (!rhbz.b_id)
> + {
> + log(_("Checking for duplicates"));
> +
> + /*
> + selinux guy's almost always move filled bug from component
> selinux-policy
> + to right component.
>
> - bugzilla client is looking for duplicate bug by sending xmlrpc query
> + bugzilla client is looking for duplicate bug by sending xmlrpc
> query
>
> - "ALL whiteboard:<hash> product:<product> component:<name>"
> + "ALL whiteboard:<hash> product:<product> component:<name>"
>
> - so if bug is moved from component selinux-policy to other, then query
> - returns NULL and creates a new bug.
> - */
> - const char *component_substitute = (!strcmp(component,
> "selinux-policy")) ? NULL : component;
> - xmlrpc_value *result = rhbz_search_duphash(client, component_substitute,
> - rhbz.b_product, duphash);
> + so if bug is moved from component selinux-policy to other, then
> query
> + returns NULL and creates a new bug.
> + */
> + const char *component_substitute = (!strcmp(component,
> "selinux-policy")) ? NULL : component;
> + xmlrpc_value *result = rhbz_search_duphash(client,
> component_substitute,
> + rhbz.b_product, duphash);
>
> - xmlrpc_value *all_bugs = rhbz_get_member("bugs", result);
> - xmlrpc_DECREF(result);
> - if (!all_bugs)
> - error_msg_and_die(_("Missing mandatory member 'bugs'"));
> + xmlrpc_value *all_bugs = rhbz_get_member("bugs", result);
> + xmlrpc_DECREF(result);
> + if (!all_bugs)
> + error_msg_and_die(_("Missing mandatory member 'bugs'"));
>
> - int all_bugs_size = rhbz_array_size(all_bugs);
> - // 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 %i reports with same duphash '%s'",
> - all_bugs_size, duphash);
> + int all_bugs_size = rhbz_array_size(all_bugs);
> + // 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 %i reports with same duphash '%s'",
> + all_bugs_size, duphash);
>
> - int bug_id = rhbz_bug_id(all_bugs);
> - xmlrpc_DECREF(all_bugs);
> - struct bug_info *bz = rhbz_bug_info(client, bug_id);
> + int bug_id = rhbz_bug_id(all_bugs);
> + xmlrpc_DECREF(all_bugs);
> + bz = rhbz_bug_info(client, bug_id);
>
> - if (all_bugs_size == 0)
> + if (all_bugs_size == 0)
> + {
> + /* Create new bug */
> + log(_("Creating a new bug"));
> + bug_id = rhbz_new_bug(client, problem_data, rhbz.b_release);
> +
> + log(_("Adding attachments to bug %i"), bug_id);
> + char bug_id_str[sizeof(int)*3 + 2];
> + sprintf(bug_id_str, "%i", bug_id);
> +
> + int flags = RHBZ_NOMAIL_NOTIFY;
> + if (opts & OPT_b)
> + flags |= RHBZ_ATTACH_BINARY_FILES;
> + rhbz_attach_big_files(client, bug_id_str, problem_data, flags);
> +
> + bz = new_bug_info();
> + bz->bi_status = xstrdup("NEW");
> + bz->bi_id = bug_id;
> + goto log_out;
> + }
> + }
> + else
> {
> - /* Create new bug */
> - log(_("Creating a new bug"));
> - bug_id = rhbz_new_bug(client, problem_data, rhbz.b_release);
> -
> - log(_("Adding attachments to bug %i"), bug_id);
> - char bug_id_str[sizeof(int)*3 + 2];
> - sprintf(bug_id_str, "%i", bug_id);
> -
> - int flags = RHBZ_NOMAIL_NOTIFY;
> - if (opts & OPT_b)
> - flags |= RHBZ_ATTACH_BINARY_FILES;
> - rhbz_attach_big_files(client, bug_id_str, problem_data, flags);
> -
> - bz = new_bug_info();
> - bz->bi_status = xstrdup("NEW");
> - bz->bi_id = bug_id;
> - goto log_out;
> + bz = rhbz_bug_info(client, rhbz.b_id);
> }
>
> log(_("Bug is already reported: %i"), bz->bi_id);
> diff --git a/src/plugins/rhbz.h b/src/plugins/rhbz.h
> index 979cef2..c9023e9 100644
> --- a/src/plugins/rhbz.h
> +++ b/src/plugins/rhbz.h
> @@ -61,6 +61,7 @@ struct bug_info {
> };
>
> struct bugzilla_struct {
> + int b_id;
> const char *b_login;
> const char *b_password;
> const char *b_bugzilla_xmlrpc;
> @@ -71,7 +72,9 @@ struct bugzilla_struct {
> };
>
> #define INIT_BUGZILLA(name) \
> - static struct bugzilla_struct name
> + static struct bugzilla_struct name = { \
> + .b_id = 0, \
> + }
>
> struct bug_info *new_bug_info();
> void free_bug_info(struct bug_info *bz);
