On 01/03/2013 04:01 PM, Jakub Filak wrote:
+int g_interactive;
Is it necessary to use a global variable? I'd rather add an extra argument to the two functions using this global variable.
Ok.
+static problem_data_t *load_problem_data_if_not_yet(problem_data_t *problem_data, const char *dump_dir_name) {
- char *usability_description = NULL;
- char *usability_detail = NULL;
- const bool usable_rating = check_problem_rating_usability(config, problem_data,
&usability_description,
&usability_detail);
- if (!usable_rating)
- if (problem_data)
return problem_data;
- struct dump_dir *dd = dd_opendir(dump_dir_name, /*flags:*/ 0);
- if (!dd) {
printf("%s\n", usability_description);
printf("%s\n", usability_detail);
problem_data_free(problem_data);
}return NULL;
I don't see the reason why problem_data variable is freed here.
Because I want to be able to do this in the caller:
problem_data = load_problem_data_if_not_yet(problem_data, dump_dir_name); if (!problem_data) goto ret;
if (config->ec_sending_sensitive_data)
{
/* We assume this event is a reporter */
This assumption is invalid at least for report_Bugzilla event.
report_Bugzilla isn't a reporter?
+static GList *str_to_glist(const char *str, int delim)
This is unnecessary function, see libreport/src/lib/run_event.c list_possible_events_glist()
Thanks, I indeed can use it instead.
+static char *select_event_name(GList *list_options) +{
- if (!list_options)
return NULL;
- /* Get settings */
- load_event_config_data();
- /* Just one? */
- if (!list_options->next)
return xstrdup((char*)list_options->data);
- int errors = 0;
- int plugins = 0;
- if (flags & CLI_REPORT_BATCH)
- int count = 0;
- for (GList *li = list_options; li; li = li->next) {
puts(_("Reporting..."));
errors += run_events(dump_dir_name, report_events, "Reporting");
plugins += g_list_length(report_events);
char *opt = (char*)li->data;
event_config_t *config = get_event_config(opt);
count++;
printf(" %i) %s\n", count, config ? ec_get_screen_name(config) : opt);
Maybe: printf(" %2i) %s\n", count, config ? ec_get_screen_name(config) : opt); for those who have more than 10 events (I am one of them).
Ok.
- list_free_with_free(list_events);
I'd say that list_free_with_free() is obsolete. The g_list_free_full() is the function which should be used here.
g_list_free_full() is not available in older versions of Glib.