On 07/20/2012 02:22 PM, Jakub Filak wrote:
> * unification of implementaion of report-gtk and report-cli
>
> Signed-off-by: Jakub Filak <[email protected]>
> ---
> src/cli/cli-report.c | 29 ++++++++---------
> src/gui-wizard-gtk/wizard.c | 55 +++++---------------------------
> src/include/event_config.h | 22 +++++++++++++
> src/lib/event_config.c | 73
> +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 117 insertions(+), 62 deletions(-)
>
> diff --git a/src/cli/cli-report.c b/src/cli/cli-report.c
> index 117c6fb..988bfc7 100644
> --- a/src/cli/cli-report.c
> +++ b/src/cli/cli-report.c
> @@ -857,12 +857,7 @@ int report(const char *dump_dir_name, int flags)
> }
> else
> {
> - const char *rating_str =
> problem_data_get_content_or_NULL(problem_data, FILENAME_RATING);
> -//COMPAT, remove after 2.1 release
> - if (!rating_str)
> - rating_str = problem_data_get_content_or_NULL(problem_data,
> "rating");
> -
> - unsigned i, rating = rating_str ? xatou(rating_str) : 4;
> + unsigned i;
> GList *li;
> char wanted_reporters[255];
>
> @@ -890,18 +885,21 @@ int report(const char *dump_dir_name, int flags)
> if (!is_number_in_string(i, wanted_reporters))
> continue;
>
> - if (config && (rating < config->ec_minimal_rating))
> - {
> - puts(_("Reporting disabled because the backtrace is
> unusable"));
> + char *usability_description = NULL;
> + char *usability_detail = NULL;
> + const bool usable_rating =
> check_problem_rating_usability(config, problem_data,
> +
> &usability_description,
> +
> &usability_detail);
>
> - const char *package =
> problem_data_get_content_or_NULL(problem_data, FILENAME_PACKAGE);
> - if (package && package[0])
> - printf(_("Please try to install debuginfo manually using
> the command: \"debuginfo-install %s\" and try again\n"), package);
> + if (!usable_rating)
> + {
> + printf("%s\n", usability_description);
> + printf("%s\n", usability_detail);
>
> - plugins++;
> errors++;
> - continue;
> + goto next_plugin;
> }
> +
> ask_for_missing_settings(reporter_name);
>
> /*
> @@ -914,7 +912,10 @@ int report(const char *dump_dir_name, int flags)
> errors += run_events(dump_dir_name, cur_event, "Reporting");
> g_list_free(cur_event);
>
> +next_plugin:
> plugins++;
> + free(usability_description);
> + free(usability_detail);
> }
> }
>
> diff --git a/src/gui-wizard-gtk/wizard.c b/src/gui-wizard-gtk/wizard.c
> index c93ff42..a32aafa 100644
> --- a/src/gui-wizard-gtk/wizard.c
> +++ b/src/gui-wizard-gtk/wizard.c
> @@ -1644,61 +1644,20 @@ static bool check_minimal_bt_rating(const char
> *event_name)
>
> if (!event_name)
> error_msg_and_die(_("Cannot check backtrace rating because of
> invalid event name"));
> - else if(strncmp("report", event_name, sizeof("report")-1) != 0)
> + else if (prefixcmp(event_name, "report") != 0)
> {
> - VERB1 log("No checks for bactrace rating because event '%s' doesn't
> report.", event_name);
> + VERB2 log("No checks for bactrace rating because event '%s' doesn't
> report.", event_name);
> return acceptable_rating;
> }
> else
> - {
> event_cfg = get_event_config(event_name);
>
> - if (!event_cfg)
> - {
> - VERB1 log("Cannot check backtrace rating because of not defined
> configuration for event '%s'", event_name);
> - return acceptable_rating;
> - }
> - }
> -
> - /*
> - * FIXME: this should be bind to a reporter not to a compoment
> - * but so far only oopses don't have rating, so for now we
> - * skip the "kernel" manually
> - */
> - const char *analyzer = problem_data_get_content_or_NULL(g_cd,
> FILENAME_ANALYZER);
> -//FIXME: say "no" to special casing!
> - if (event_cfg && analyzer && strcmp(analyzer, "Kerneloops") != 0)
> + char *description = NULL;
> + acceptable_rating = check_problem_rating_usability(event_cfg, g_cd,
> &description, NULL);
> + if (description)
> {
> - const char *rating_str = problem_data_get_content_or_NULL(g_cd,
> FILENAME_RATING);
> -//COMPAT, remove after 2.1 release
> - if (!rating_str)
> - rating_str = problem_data_get_content_or_NULL(g_cd, "rating");
> -
> - if (rating_str)
> - {
> - char *endptr;
> - errno = 0;
> - long rating = strtol(rating_str, &endptr, 10);
> - if (errno != 0 || endptr == rating_str || *endptr != '\0')
> - {
> - add_warning(_("Reporting disabled because the rating does
> not contain a number."));
> - acceptable_rating = false;
> - }
> - else
> - {
> - VERB1 log("Checking current rating %ld to required rating
> %ld.", rating, event_cfg->ec_minimal_rating);
> - if (rating == event_cfg->ec_minimal_rating) /* bt is usable,
> but not complete, so show a warning */
> - {
> - add_warning(_("The backtrace is incomplete, please make
> sure you provide the steps to reproduce."));
> - }
> - else if (rating < event_cfg->ec_minimal_rating)
> - {
> - add_warning(_("Reporting disabled because the backtrace
> is unusable."));
> - //FIXME: see CreporterAssistant: 394 for ideas
> - acceptable_rating = false;
> - }
> - }
> - }
> + add_warning(description);
> + free(description);
> }
>
> return acceptable_rating;
> diff --git a/src/include/event_config.h b/src/include/event_config.h
> index 6644b42..15ebf52 100644
> --- a/src/include/event_config.h
> +++ b/src/include/event_config.h
> @@ -21,6 +21,7 @@
>
> #include <stdbool.h>
> #include <glib.h>
> +#include "problem_data.h"
>
> #ifdef __cplusplus
> extern "C" {
> @@ -103,6 +104,27 @@ void unexport_event_config(GList *env_list);
>
> GHashTable *validate_event(const char *event_name);
>
> +/*
> + * Checks usability of problem's backtrace rating against required rating
> level
> + * from event configuration.
> + *
> + * @param cfg an event configuration
> + * @param pd a checked problem data
> + * @param description an output parameter for a description of rating
> + * usability. If the variable holds NULL after function call no description
> is
> + * available. The description can be provided even if backtrace rating is
> + * acceptable. Can be NULL.
> + * @param detail an output parameter for a more details about rating
> usability.
> + * If the variable holds NULL after function call no description is
> available.
> + * The detail can be provided even if backtrace rating is acceptable. Can be
> + * NULL.
> + * @returns true if rating is usable or above usable; otherwise false
> + */
> +bool check_problem_rating_usability(const event_config_t *cfg,
> + problem_data_t *pd,
> + char **description,
> + char **detail);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/src/lib/event_config.c b/src/lib/event_config.c
> index 85c0e80..9bf0d5f 100644
> --- a/src/lib/event_config.c
> +++ b/src/lib/event_config.c
> @@ -367,3 +367,76 @@ GHashTable *validate_event(const char *event_name)
>
> return NULL;
> }
> +
> +/*
> + * This function checks if a value of problem's backtrace rating is
> acceptable
> + * for event according to event's required rating value.
> + *
> + * The solved problem seems to be pretty straightforward but there are some
> + * pitfalls.
> + *
> + * 1. Is rating acceptable if event doesn't have configuration?
> + * 2. Is rating acceptable if problem's data doesn't contains rating value?
> + * 3. Is rating acceptable if rating value is not a number?
> + * 4. What message show to user if there is a concern about usability?
> + */
> +bool check_problem_rating_usability(const event_config_t *cfg,
> + problem_data_t *pd,
> + char **description,
> + char **detail)
> +{
> + char *tmp_desc = NULL;
> + char *tmp_detail = NULL;
> +
> + bool result = false;
> +
> + const char *rating_str = problem_data_get_content_or_NULL(pd,
> FILENAME_RATING);
> + if (!cfg || !rating_str)
> + goto finish;
You can do "if (!cfg) goto finish;" even before getting FILENAME_RATING.
> + const long minimal_rating = cfg->ec_minimal_rating;
> + char *endptr;
> + errno = 0;
> + const long rating = strtol(rating_str, &endptr, 10);
> + if (errno != 0 || endptr == rating_str || *endptr != '\0')
> + {
> + tmp_desc = xasprintf("%s '%s'.", _("Reporting disabled because the
> rating does not contain a number."), rating_str);
> + tmp_detail = xstrdup(_("Please report this problem to ABRT."));
Maybe "Please report this problem to ABRT project developers" ?
> +
> + result = false;
> + }
> + else if (rating == minimal_rating) /* bt is usable, but not complete, so
> show a warning */
What if minimal_rating == highest possible rating?
Will we _always_ say "The backtrace is incomplete" in this case?
> + {
> + tmp_desc = xstrdup(_("The backtrace is incomplete, please make sure
> you provide the steps to reproduce."));
> + tmp_detail = xstrdup(_("The backtrace probably can't help to a
> developer in searching for bug."));
> +
> + result = true;
> + }
> + else if (rating < minimal_rating)
> + {
> + tmp_desc = xstrdup(_("Reporting disabled because the backtrace is
> unusable."));
> +
> + const char *package = problem_data_get_content_or_NULL(pd,
> FILENAME_PACKAGE);
> + if (package && package[0])
> + tmp_detail = xasprintf(_("Please try to install debuginfo
> manually using the command: \"debuginfo-install %s\" and try again."),
> package);
> + else
> + tmp_detail = xstrdup(_("A proppert debuginfo is probably missing
> or the coredump is corrupted."));
s/proppert/proper/
> +
> + result = false;
> + }
> + else /* rating > minimal_rating */
> + result = true;
> +
> +finish:
> + if (description)
> + *description = tmp_desc;
> + else
> + free(tmp_desc);
> +
> + if (detail)
> + *detail = tmp_detail;
> + else
> + free(tmp_detail);
> +
> + return result;
> +}
