On Tuesday 26 of June 2012 17:09:48 you wrote:
On 06/26/2012 10:37 AM, Jakub Filak wrote:
Signed-off-by: Jakub Filakjfilak@redhat.com
libreport.spec.in | 1 + src/include/Makefile.am | 3 +- src/include/run_event_list.h | 209 ++++++++++++++ src/lib/Makefile.am | 3 +- src/lib/run_event_list.c | 618 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 832 insertions(+), 2 deletions(-) create mode 100644 src/include/run_event_list.h create mode 100644 src/lib/run_event_list.c
diff --git a/libreport.spec.in b/libreport.spec.in index 3f730e3..ea51431 100644 --- a/libreport.spec.in +++ b/libreport.spec.in
@@ -297,6 +297,7 @@ gtk-update-icon-cache
%{_datadir}/icons/hicolor&>/dev/null || :
%{_includedir}/libreport/report.h %{_includedir}/libreport/run_event.h %{_includedir}/libreport/event_usability.h
+%{_includedir}/libreport/run_event_list.h
# Private api headers: %{_includedir}/libreport/internal_abrt_dbus.h %{_includedir}/libreport/internal_libreport.h
diff --git a/src/include/Makefile.am b/src/include/Makefile.am index 7aa5f78..bca63dd 100644 --- a/src/include/Makefile.am +++ b/src/include/Makefile.am @@ -10,4 +10,5 @@ libreport_include_HEADERS = \
\ internal_libreport.h \ internal_abrt_dbus.h \
- event_usability.h
- event_usability.h \
- run_event_list.h
diff --git a/src/include/run_event_list.h b/src/include/run_event_list.h new file mode 100644 index 0000000..f5fa11f --- /dev/null +++ b/src/include/run_event_list.h @@ -0,0 +1,209 @@ +/*
- Copyright (C) 2012 ABRT team.
- Copyright (C) 2012 RedHat inc.
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
along + with this program; if not, write to the Free Software Foundation, Inc., + 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +*/ +#ifndef LIBREPORT_RUN_EVENT_LIST_H_ +#define LIBREPORT_RUN_EVENT_LIST_H_
+#include<glib.h> +#include "run_event.h" +#include "event_usability.h"
+/* Forward declartion */ +struct event_list_process;
+/*
- Return value from event list process signal.
- Signal handler returns these values in order to adjust process
- flow according to current state.
- */
+enum elp_signal_ret +{
- ELPSR_CONTINUE, ///< A process should continue with next step.
- ELPSR_NEXT_EVENT, ///< A process should skip all remaining steps a
continue with next event. + ELPSR_FINISH, ///< A process should skip all remaining events and finis itself. +};
What is elp[sr]?
event list process signal return
+/*
- Event list process signal function type
This does not make sense to me.
pull approach, all functions have the same declaration and read required data from a passed process pointer
- @param process A process that emits a signal.
- @return A value that adjusts process flow.
- */
+typedef enum elp_signal_ret (* elp_signal)(struct event_list_process *process); + +/*
- All signals used by event list process
- */
+struct elp_signals_impl +{
- /*
* Next event was successfully selected and process is about to
process it + */
- elp_signal next_event_selected;
- /*
* Currently selected event requires review.
*/
- elp_signal review_data;
- /*
* Currently selected event is about to be run.
*/
- elp_signal run_event;
- /*
* Currently selected event has finished.
*/
- elp_signal event_done;
- /*
* No more events to be processed
*/
- elp_signal finished;
- /*
* Event has some problem with configuration
*/
- elp_signal configuration_issue;
+};
This looks like a TON of added complexity for no apparent reason.
I can use only one signal functions with two arguments. 1. process 2. type of signal But it forces me to define a new enum with type of signals. And on the client side it requires more complex logic.
For starters, what problem are you solving?
(report-gtk|report-cli) -e report_uReport -e analyze_LocalGDB -e report_Bugzilla
I tried to unify the implementation.
Each event from list must be reviewed if it is required. After review the event must be run. And finally a result of run is sent to back to caller.
+/*
- Types of event run result
- */
+enum elp_event_run_status +{
- ELP_ERS_NONE, ///< No run was performed
- ELP_ERS_SUCCESSFUL, ///< Event finished successfully
- ELP_ERS_FAILED, ///< Event failed
- ELP_ERS_NOT_FOUND, ///< No processing was found
+};
ELP ERS?
:) event list process event run status
+/*
- Structure holds an configuration issue info
- */
+struct elp_configuration_issue +{
- const char *event_name; ///< Event with problems
- enum event_usability_status usability; ///< Event usability
- const GList *issues; ///< All known configuration
issues for an event +};
+/*
- Initializes event list process signal functions.
- @param impl A not NULL pointer to signals
- */
+void event_list_process_impl_init(struct elp_signals_impl *impl);
I am totally lost by now. What are we doing here?
convenient way how to initialize a structure with signals I can use memset() instead of this function
+/*
- Creates an initializes an event list process
- @param events A list of events to be processed, do not take an
ownership + * @param signals A signal implementation, do not take an ownership + * @param run_impl A call backs for event run, do not take an ownership + * @param run_obs An observer for event run, do not take an ownership + * @param dump_dir_name A dump dir name, do not take an ownership + * @param signal_args A custom data available by elp_get_signal_args() + * @return An initialized event list process, never returns NULL
- */
+struct event_list_process *new_event_list_process(const GList *events,
const struct
elp_signals_impl *impl, + const struct run_event_impl *run_impl, + const struct run_event_observer *run_obs, + const char *dump_dir_name, + void *signal_args); + +/*
- Frees all used memory by an event list process
- @param process A pointer to process
- */
+void free_event_list_process(struct event_list_process *process);
+/*
- Gets an currently selected event.
- @param process A not NULL pointer to process
- @return A pointer to event name if some event was selected, otherwise
returns NULL + */ +const char *elp_get_current_event(const struct event_list_process *process); + +/*
- Gets an currently selected event.
- @param process A not NULL pointer to process
- @return A data passed in new_event_list_process()
- */
+void *elp_get_signal_args(const struct event_list_process *process);
+/*
- Gets the last event run result.
- @param process A not NULL pointer to process
- @return the last event run result.
- */
+enum elp_event_run_status elp_get_last_event_status(const struct event_list_process *process); + +/*
- Gets the last configuraton issue. This function must be called
- only from handler of configuration_issue signal.
- @param process A not NULL pointer to process
- @return the last configuration issue.
- */
+const struct elp_configuration_issue *elp_get_last_configuration_issue(const struct event_list_process *process); + +/*
- Sets a post run callback for run_event_state.
- The method is provided only for backward compatibility
- @param process A not NULL pointer to process
- @param post_run_callback A pointer to function
- @param post_run_param A custom data passed to callback
- */
+void elp_set_post_run_callback(struct event_list_process *process, res_post_run_callback post_run_callback, void *post_run_param); + +/*
- Sets a logging callback for run_event_state.
- The method is provided only for backward compatibility
- @param process A not NULL pointer to process
- @param loggin_callback A pointer to function
- @param loggin_param A custom data passed to callback
- */
+void elp_set_logging_callback(struct event_list_process *process, res_logging_callback logging_callback, void *logging_param); + +/*
- Forces a process to perform next step.
- @param process A not NULL pointer to process
- @return 0 if last step was performed; otherwise returns non 0
- */
+int elp_next_step(struct event_list_process *process);
+/*
- Kills an event command process
- @param process A not NULL pointer to process
- */
+void elp_kill_run(struct event_list_process *process);
+#endif /* LIBREPORT_RUN_EVENT_LIST_H_ */
diff --git a/src/lib/Makefile.am b/src/lib/Makefile.am index cd1f164..fd406ce 100644 --- a/src/lib/Makefile.am +++ b/src/lib/Makefile.am @@ -48,7 +48,8 @@ libreport_la_SOURCES = \
user_settings.c \ client.c \ utf8.c \
- event_usability.c
event_usability.c \
run_event_list.c
libreport_la_CPPFLAGS = \
-Wall -Wwrite-strings -Werror \
diff --git a/src/lib/run_event_list.c b/src/lib/run_event_list.c new file mode 100644 index 0000000..559772d --- /dev/null +++ b/src/lib/run_event_list.c @@ -0,0 +1,618 @@ +/*
- Copyright (C) 2012 ABRT Team
- Copyright (C) 2012 RedHat inc.
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
along + with this program; if not, write to the Free Software Foundation, Inc., + 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +*/ +#include<pthread.h> +#include<glob.h> +#include<regex.h> +#include "run_event.h" +#include "run_event_list.h" +#include "event_usability.h" +#include "internal_libreport.h"
+enum elp_process_states +{
- __MIN_PROC_STATE__ = 0,
__names are reserved for compiler / libc.
Thus I have to use better name.
- ELP_INIT = __MIN_PROC_STATE__,
- ELP_START,
- ELP_REVIEW_DATA,
- ELP_RUN_EVENT,
- ELP_END,
- __MAX_PROC_STATE__ = ELP_END,
+};
+struct event_list_process +{
- const struct elp_signals_impl *impl;
- const struct run_event_impl *run_impl;
- const struct run_event_observer *run_obs;
- const char *dump_dir_name;
- void *args;
- enum elp_process_states current;
- const GList *events;
- GList *internal_events;
- GList *next;
- GList *processed;
- struct run_event_state *run_state;
- res_post_run_callback post_run_callback;
- void *post_run_param;
- res_logging_callback logging_callback;
- void *logging_param;
- pthread_mutex_t run_mut;
- enum elp_event_run_status last_run;
- struct elp_configuration_issue last_configuration_issue;
+};
+void event_list_process_impl_init(struct elp_signals_impl *impl) +{
- memset(impl, 0, sizeof(*impl));
+}
+struct event_list_process *new_event_list_process(const GList *events,
const struct
elp_signals_impl *impl, + const struct run_event_impl *run_impl, + const struct run_event_observer *run_obs, + const char *dump_dir_name, + void *signal_args) +{
- assert(impl || !"NULL argument");
- assert(run_impl || !"NULL argument");
- struct event_list_process *process = xmalloc(sizeof(*process));
- process->impl = impl;
- process->run_impl = run_impl;
- process->run_obs = run_obs;
- process->dump_dir_name = dump_dir_name;
- process->args = signal_args;
- process->current = __MIN_PROC_STATE__;
- process->events = events;
- process->next = NULL;
- process->internal_events = NULL;
- process->processed = NULL;
- process->run_state = NULL;
- pthread_mutex_init(&(process->run_mut), NULL);
- process->last_run = ELP_ERS_NONE;
- process->post_run_callback = NULL;
- process->post_run_param = NULL;
- process->logging_callback = NULL;
- process->logging_param = NULL;
- return process;
+}
+void free_event_list_process(struct event_list_process *process) +{
- assert(process || !"NULL argument");
- g_list_free(process->internal_events);
- pthread_mutex_destroy(&(process->run_mut));
- free(process);
+}
+const char* elp_get_current_event(const struct event_list_process *process) +{
- assert(process || !"NULL argument");
- return process->processed ? process->processed->data : NULL;
+}
+void *elp_get_signal_args(const struct event_list_process *process) +{
- assert(process || !"NULL argument");
- return process->args;
+}
+enum elp_event_run_status elp_get_last_event_status(const struct event_list_process *process) +{
- assert(process || !"NULL argument");
- return process->last_run;
+}
+void elp_kill_run(struct event_list_process *process) +{
- assert(process || !"NULL argument");
- pthread_mutex_lock(&(process->run_mut));
- if (process->run_state)
run_event_state_kill_command(process->run_state);
- pthread_mutex_unlock(&(process->run_mut));
+}
+void elp_set_post_run_callback(struct event_list_process *process,
res_post_run_callback post_run_callback,
void *post_run_param)
+{
- assert(process || !"NULL argument");
- pthread_mutex_lock(&(process->run_mut));
- process->post_run_callback = post_run_callback;
- process->post_run_param = post_run_param;
- pthread_mutex_unlock(&(process->run_mut));
+}
+void elp_set_logging_callback(struct event_list_process *process,
res_logging_callback logging_callback,
void *logging_param)
+{
- assert(process || !"NULL argument");
- pthread_mutex_lock(&(process->run_mut));
- process->logging_callback = logging_callback;
- process->logging_param = logging_param;
- pthread_mutex_unlock(&(process->run_mut));
+}
+const struct elp_configuration_issue *elp_get_last_configuration_issue(const struct event_list_process *process) +{
- return&(process->last_configuration_issue);
+}
+static const struct run_event_impl *elp_get_run_impl(struct event_list_process *process) +{
- assert(process || !"NULL argument");
- return process->run_impl;
+}
+static const struct run_event_observer *elp_get_run_obs(struct event_list_process *process) +{
- assert(process || !"NULL argument");
- return process->run_obs;
+}
+static problem_data_t *elp_create_problem_data(const struct event_list_process *process) +{
- struct dump_dir *dd = dd_opendir(process->dump_dir_name, 0);
- if (!dd)
return NULL;
- problem_data_t *pd = create_problem_data_from_dump_dir(dd);
- dd_close(dd);
- return pd;
+}
+static bool elp_do_need_review(const struct event_list_process *process, const char *event_name) +{
- const event_config_t *const cfg = get_event_config(event_name);
- if (cfg&& !cfg->ec_skip_review)
return true;
- problem_data_t *const pd = elp_create_problem_data(process);
- if (pd)
return check_event_usability(cfg, pd, NULL, NULL, EUS_LOW) !=
EUS_GOOD; +
- return true;
+}
+static void elp_set_last_configuration_issues(struct event_list_process *process, + const char *event_name, + enum event_usability_status status, + const GList *issues) +{
- pthread_mutex_lock(&(process->run_mut));
- process->last_configuration_issue.event_name = event_name;
- process->last_configuration_issue.usability = status;
- process->last_configuration_issue.issues = issues;
- pthread_mutex_unlock(&(process->run_mut));
+}
+static enum elp_signal_ret elp_event_configuration_issue(struct event_list_process *process, + const char *event_name, + enum event_usability_status status, + const GList *issues) +{
- enum elp_signal_ret ret = ELPSR_FINISH;
- if (process->impl->configuration_issue)
- {
elp_set_last_configuration_issues(process, event_name, status,
issues); +
ret = process->impl->configuration_issue(process);
elp_set_last_configuration_issues(process, NULL, EUS_GOOD, NULL);
- }
- return ret;
+}
+typedef enum event_usability_status (*validator_fn)(const event_config_t *, const problem_data_t *, + const GList *, GList **, enum event_usability_status); + +static enum elp_signal_ret elp_validate_event_config(struct event_list_process *process, + const event_config_t *cfg, + const char *event_name, + const problem_data_t *pd, + const GList *not_loaded_items, + enum event_usability_status min_level, + validator_fn *validators) +{
- enum elp_signal_ret resp = ELPSR_CONTINUE;
- if (!cfg)
return resp;
- enum event_usability_status status = EUS_GOOD;
- validator_fn *validator = validators;
- GList* issues = NULL;
- while(*validator)
- {
status = worst_usability(status, (*validator)(cfg, pd,
not_loaded_items,&issues, min_level)); + ++validator;
- }
- if (status == EUS_UNUSABLE)
- {
resp = elp_event_configuration_issue(process, event_name, status,
issues); + g_list_free_full(issues, (GDestroyNotify)free);
if (resp == ELPSR_CONTINUE)
error_msg_and_die("Cannot continue with unusable event '%s'",
event_name); + }
- return resp;
+}
+static enum elp_signal_ret elp_validate_event(struct event_list_process *process, + const char *event_name, + const problem_data_t *pd, + const GList *not_loaded_items, + enum event_usability_status min_level, + validator_fn *validators) +{
- return elp_validate_event_config(process,
get_event_config(event_name), event_name, pd, + not_loaded_items, min_level, validators); +}
+static const GList* elp_select_next_event(struct event_list_process *process, const GList **event_list, const GList *not_loaded_items, enum elp_signal_ret *ret) +{
- assert(process || !"NULL argument");
- assert(event_list || !"NULL argument");
- static validator_fn validators[] = {
check_event_usability,
NULL,
- };
- const GList *tmp;
- tmp = *event_list;
- *event_list = g_list_next(tmp);
- problem_data_t *pd = elp_create_problem_data(process);
- if (!pd)
return tmp;
- while(tmp)
- {
const enum elp_signal_ret resp = elp_validate_event(process,
tmp->data, pd, + not_loaded_items, EUS_UNUSABLE, validators); +
if (ret)
*ret = resp;
if (resp == ELPSR_CONTINUE)
break;
else if (resp == ELPSR_FINISH)
{
tmp = *event_list = NULL;
break;
}
else if (resp != ELPSR_NEXT_EVENT)
{
assert(!"unexpected return value");
}
tmp = *event_list;
*event_list = g_list_next(tmp);
- }
- free_problem_data(pd);
- return tmp;
+}
+static GList* elp_revise_event_list(struct event_list_process *process, const GList *event_list) +{
- GList *revised_list = NULL;
- GList *created_items = NULL;
- enum elp_signal_ret ret;
- const GList *event;
- while ((event = elp_select_next_event(process,&event_list,
created_items,&ret))) + {
const event_config_t *const cfg = get_event_config(event->data);
if (cfg&& cfg->ec_creates_items)
{
/* have to create a copy because of threads */
/* TODO : find a better place for following lines */
char *item_list = xstrdup(cfg->ec_creates_items);
char *bck = item_list;
while (item_list[0])
{
char *end = strchr(item_list, ',');
if (end) *end = '\0';
created_items = g_list_prepend(created_items,
xstrdup(item_list)); + if (!end)
break;
item_list = end + 1;
}
free(bck);
}
revised_list = g_list_prepend(revised_list, event->data);
- }
- g_list_free_full(created_items, (GDestroyNotify)free);
- if (ret != ELPSR_FINISH)
return g_list_reverse(revised_list);
- g_list_free(revised_list);
- return NULL;
+}
+static GList *elp_next_event(struct event_list_process *process) +{
- assert(process || !"NULL argument");
- return (GList *)elp_select_next_event(process, (const GList
**)&process->next, NULL, NULL); +}
+static bool event_list_process_call(struct event_list_process *process,
elp_signal cb,
enum elp_process_states ok_state,
enum elp_signal_ret default_ret)
+{
- assert(process || !"NULL argument");
- assert((ok_state>= __MIN_PROC_STATE__&& ok_state<=
__MAX_PROC_STATE__) || !"out of range argument"); +
- const enum elp_signal_ret ret = cb ? cb(process) : default_ret;
- switch (ret)
- {
case ELPSR_CONTINUE:
process->current = ok_state;
break;
case ELPSR_NEXT_EVENT:
process->current = ELP_START;
break;
case ELPSR_FINISH:
process->current = ELP_END;
break;
default:
assert(!"unexpected signal return value");
error_msg_and_die("unexpected signal return value %d", ret);
break;
- }
- return ret == ELPSR_CONTINUE;
+}
+struct process_run_event_watcher +{
- const struct run_event_observer *wrapped_obs;
- bool failed;
- bool stopper;
+};
+static void process_command_started(pid_t pid, void *args) +{
- struct process_run_event_watcher *pre = (struct
process_run_event_watcher *)args; +
- if (pre->wrapped_obs&& pre->wrapped_obs->command_started)
pre->wrapped_obs->command_started(pid, pre->wrapped_obs->args);
+}
+static void process_command_msg_processed(const char *msg, const char *raw_input, const char *rsp, void *args) +{
- struct process_run_event_watcher *pre = (struct
process_run_event_watcher *)args; +
- /* TODO : should I move it to client.h ? */
- if (!pre->stopper)
pre->stopper = !strncmp(msg, "THANK_YOU", sizeof("THANK_YOU")
-1);
- if (pre->wrapped_obs&& pre->wrapped_obs->command_msg_processed)
pre->wrapped_obs->command_msg_processed(msg, raw_input, rsp,
pre->wrapped_obs->args); +}
+static void process_command_finished(int retval, int status, const char *err_msg, void *args) +{
- struct process_run_event_watcher *pre = (struct
process_run_event_watcher *)args; +
- pre->failed = retval != 0;
- if (pre->wrapped_obs&& pre->wrapped_obs->command_finished)
pre->wrapped_obs->command_finished(retval, status, err_msg,
pre->wrapped_obs->args); +}
+int elp_next_step(struct event_list_process *process) +{
- /* before process start, revise event list and remove not configured
and */ + /* unusable events */
- if (process->current == ELP_INIT)
- {
process->current = ELP_START;
/* create internal event list from origin event list */
/* it is a copy of original list without unusable events */
process->internal_events = process->next =
elp_revise_event_list(process, process->events); + }
- /* Process all event from list until reaches event that needs review,
event run failure */ + /* or the end of the event list */
- while (1)
- {
if (process->current == ELP_START)
{
/* selects next event from internal list */
process->processed = elp_next_event(process);
if (!process->processed)
{ /* Event list is empty now */
process->current = ELP_END;
}
else
{
/* If callback next_event_selected is not set the process
expects that */ + /* there is no problem to continue thus default response is CONTINUE */ + /* It callback returns a positive response process state is set to START */ + if (!event_list_process_call(process, process->impl->next_event_selected, ELP_START, ELPSR_CONTINUE)) + return true;
if (elp_do_need_review(process,
elp_get_current_event(process))) + process->current = ELP_REVIEW_DATA;
else
process->current = ELP_RUN_EVENT;
}
/* return is not here because we want to continue with next
state */ + /* use case : next_step with start state causes beginning to review */ + /* or immediate run of event commands */
}
if (process->current == ELP_REVIEW_DATA)
{
/* review_data signal has high priority thus if call back is
not set, */ + /* then we expect that data are not reviewed and event is skipped */ +
/* return pauses the processing of events because process has
to wait on next */ + /* step signaling that review is finished, thus return if positive answer */ + /* was returned from review_data callback */
/* if negative answer was returned we want to select next
event immediately */ + /* or finish the process */
if(event_list_process_call(process,
process->impl->review_data, ELP_RUN_EVENT, ELPSR_NEXT_EVENT)) + return true;
}
if (process->current == ELP_RUN_EVENT)
{
/* If callback run_event is not set the process expects that
*/ + /* there is no problem to continue thus default response is CONTINUE */ +
/* if negative answer was returned we want to select next
event immediately */ + /* or finish the process */
if (!event_list_process_call(process,
process->impl->run_event, ELP_RUN_EVENT, ELPSR_CONTINUE)) + return true;
const char *event_name = elp_get_current_event(process);
struct process_run_event_watcher process_watcher = {
.failed = false,
.stopper = false,
};
process_watcher.wrapped_obs = elp_get_run_obs(process);
struct run_event_observer process_obs = {
.command_started = process_command_started,
.command_msg_processed = process_command_msg_processed,
.command_finished = process_command_finished,
};
process_obs.args =&process_watcher;
const char *dump_dir_name = process->dump_dir_name;
pthread_mutex_lock(&(process->run_mut));
struct run_event_state *run_state = new_run_event_state();
process->run_state = run_state;
run_state->post_run_callback = process->post_run_callback;
run_state->post_run_param = process->post_run_param;
run_state->logging_callback = process->logging_callback;
run_state->logging_param = process->logging_param;
pthread_mutex_unlock(&(process->run_mut));
prepare_commands(run_state, dump_dir_name, event_name);
run_event_state_on_dir_name(run_state, dump_dir_name,
event_name, + elp_get_run_impl(process),&process_obs); +
/* take care about stopper occurrence only if event was
successful */ + /* thus default value is not stopper occurrence */
bool stopper = false;
process->last_run = ELP_ERS_FAILED;
if (!process_watcher.failed)
{
if (run_state->children_count == 0)
{
process->last_run = ELP_ERS_NOT_FOUND;
/* run_event do not provide any callback for not
event found notification */ + /* and no event is considered as fail thus setting failed var to TRUE */ + process_watcher.failed = true;
}
else
{
process->last_run = ELP_ERS_SUCCESSFUL;
stopper = process_watcher.stopper;
}
}
/* stopper means that event decided to stop all further
processing the */ + /* thus successful state is ELP_END and default response is ELPSR_FINISH */ + event_list_process_call(process, process->impl->event_done, + stopper ? ELP_END : ELP_START, + stopper ? ELPSR_FINISH : ELPSR_CONTINUE); +
pthread_mutex_lock(&(process->run_mut));
free_run_event_state(run_state);
process->run_state = NULL;
pthread_mutex_unlock(&(process->run_mut));
/* do not forward to next step if event failed */
if (process_watcher.failed)
return true;
}
if (process->current == ELP_END)
{
/* Don't care about result, result can't change anything */
event_list_process_call(process, process->impl->finished,
ELP_END, ELPSR_FINISH); + return false;
}
- }
- assert(!"cannot get here");
- return false;
+}
Before these patches:
# grep -r thread . ./src/gui-wizard-gtk/Makefile.am:# -lgthread-2.0 ./src/lib/json.c:#include <btparser/thread.h> ./src/lib/json.c: struct btp_thread *core_bt = btp_load_core_backtrace(pd_item); ./src/lib/json.c: ureport_add_int(item, "thread", 0); ./src/lib/json.c: btp_thread_free(core_bt); ./src/lib/json.c: * ureport_add_item_int(ureport, pd, "crash_thread", NULL); ./src/lib/json.c: ureport_add_int(ureport, "crash_thread", 0); ./src/plugins/rhbz.c:#include <btparser/thread.h> ./src/plugins/rhbz.c: * - otherwise preview of crashed thread stack trace is created ./src/plugins/rhbz.c: /* Get optimized thread stack trace for 10 top most frames */ ./src/plugins/rhbz.c: struct btp_thread *thread = btp_backtrace_get_optimized_thread(backtrace, 10); ./src/plugins/rhbz.c: if (!thread) ./src/plugins/rhbz.c: log(_("Can't find crash thread")); ./src/plugins/rhbz.c: btp_thread_append_to_str(thread, bt, true); ./src/plugins/rhbz.c: btp_thread_free(thread);
After these patches:
# grep -r thread . ./libreport.spec.in:%{_includedir}/libreport/run_event_list_thread.h ./src/gui-wizard-gtk/wizard.c:#include "run_event_list_thread.h" ./src/gui-wizard-gtk/wizard.c:static pthread_t g_gui_thread_id; ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: show_error_as_msgbox_mts(msg, !pthread_equal(g_gui_thread_id, pthread_self())); ./src/gui-wizard-gtk/wizard.c:static void process_step_done_callback_fn(struct elp_thread_args *args) ./src/gui-wizard-gtk/wizard.c:static void process_start_callback_fn(struct elp_thread_args *args) ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c:static void process_finish_callback_fn(struct elp_thread_args *args) ./src/gui-wizard-gtk/wizard.c: free_event_list_process(elp_thread_args_get_process(args)); ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: elp_thread_loop_kill_command(g_event_process); ./src/gui-wizard-gtk/wizard.c: if (g_reviewing_data || !elp_thread_is_runnig(g_event_process)) ./src/gui-wizard-gtk/wizard.c: elp_thread_loop_next_step(g_event_process); ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: elp_thread_run(g_event_process, proc); ./src/gui-wizard-gtk/wizard.c: elp_thread_loop_next_step(g_event_process); ./src/gui-wizard-gtk/wizard.c: gdk_threads_enter(); ./src/gui-wizard-gtk/wizard.c: gdk_threads_leave(); ./src/gui-wizard-gtk/wizard.c: elp_thread_loop_next_step(g_event_process); ./src/gui-wizard-gtk/wizard.c: elp_thread_args_set_on_step_done(g_event_process, process_step_done_callback_fn); ./src/gui-wizard-gtk/wizard.c: elp_thread_args_set_on_start(g_event_process, process_start_callback_fn); ./src/gui-wizard-gtk/wizard.c: elp_thread_args_set_on_finish(g_event_process, process_finish_callback_fn); ./src/gui-wizard-gtk/wizard.c: g_gui_thread_id = pthread_self(); ./src/gui-wizard-gtk/wizard.c: g_source_attach((GSource*)gtk_assistant_source, g_main_context_get_thread_default()); ./src/gui-wizard-gtk/main.c:#include "run_event_list_thread.h" ./src/gui-wizard-gtk/main.c:struct elp_thread_args *g_event_process; ./src/gui-wizard-gtk/main.c: g_event_process = new_elp_thread_args(); ./src/gui-wizard-gtk/main.c: gdk_threads_init (); ./src/gui-wizard-gtk/main.c: gdk_threads_enter (); ./src/gui-wizard-gtk/main.c: if (elp_thread_init(g_event_process)) ./src/gui-wizard-gtk/main.c: perror_msg_and_die("initializatoin of event process thread failed"); ./src/gui-wizard-gtk/main.c: gdk_threads_leave(); ./src/gui-wizard-gtk/main.c: free_elp_thread_args(g_event_process); ./src/gui-wizard-gtk/wizard.h:struct elp_thread_args; ./src/gui-wizard-gtk/wizard.h:extern struct elp_thread_args *g_event_process; ./src/gui-wizard-gtk/Makefile.am:# -lgthread-2.0 ./src/include/run_event_list_thread.h: * struct elp_thread_args *thread_proc = new_elp_thread_args(); ./src/include/run_event_list_thread.h: * if (elp_thread_init(thread_proc)) ./src/include/run_event_list_thread.h: * elp_thread_run(thread_proc, event_proc); ./src/include/run_event_list_thread.h: * elp_thread_loop_next_step(thread_proc); ./src/include/run_event_list_thread.h: * free_event_list_process(elp_thread_get_process(thread_proc)); ./src/include/run_event_list_thread.h: * free_elp_thread(thread_proc); ./src/include/run_event_list_thread.h: * Structure that holds state of thread process. ./src/include/run_event_list_thread.h:struct elp_thread_args; ./src/include/run_event_list_thread.h: * Callback used by thread process to send signals about process state ./src/include/run_event_list_thread.h:typedef void(* elp_thread_args_callback)(struct elp_thread_args *args); ./src/include/run_event_list_thread.h: * Creates a new thread process state ./src/include/run_event_list_thread.h:struct elp_thread_args *new_elp_thread_args(); ./src/include/run_event_list_thread.h: * @param args Not NULL pointer to thread process state ./src/include/run_event_list_thread.h:void elp_thread_args_set_on_start(struct elp_thread_args *args, ./src/include/run_event_list_thread.h: elp_thread_args_callback cb); ./src/include/run_event_list_thread.h: * @param args Not NULL pointer to thread process state ./src/include/run_event_list_thread.h:void elp_thread_args_set_on_step_done(struct elp_thread_args *args, ./src/include/run_event_list_thread.h: elp_thread_args_callback cb); ./src/include/run_event_list_thread.h: * @param args Not NULL pointer to thread process state ./src/include/run_event_list_thread.h:void elp_thread_args_set_on_finish(struct elp_thread_args *args, ./src/include/run_event_list_thread.h: elp_thread_args_callback cb); ./src/include/run_event_list_thread.h: * @param args Not NULL pointer to thread process state ./src/include/run_event_list_thread.h:struct event_list_process *elp_thread_args_get_process(const struct elp_thread_args *args); ./src/include/run_event_list_thread.h: * Destroy thread process ./src/include/run_event_list_thread.h: * @param args A pointer to thread process ./src/include/run_event_list_thread.h:void free_elp_thread_args(struct elp_thread_args *args); ./src/include/run_event_list_thread.h: * Creates a thread used by a process ./src/include/run_event_list_thread.h: * @param args Not NULL pointer to thread process state ./src/include/run_event_list_thread.h:int elp_thread_init(struct elp_thread_args *args); ./src/include/run_event_list_thread.h: * Interrupts an event list process and kills a process thread. ./src/include/run_event_list_thread.h: * A thread process cannot be used after this function call. ./src/include/run_event_list_thread.h: * @param args Not NULL pointer to thread process state ./src/include/run_event_list_thread.h:void elp_thread_kill(struct elp_thread_args *args); ./src/include/run_event_list_thread.h: * Interrupts an event list process and leaves a process thread untouched. ./src/include/run_event_list_thread.h: * @param args Not NULL pointer to thread process state ./src/include/run_event_list_thread.h:void elp_thread_interrupt(struct elp_thread_args *args); ./src/include/run_event_list_thread.h: * @param args Not NULL pointer to thread process state ./src/include/run_event_list_thread.h:void elp_thread_run(struct elp_thread_args *args, struct event_list_process *process); ./src/include/run_event_list_thread.h: * @param args Not NULL pointer to thread process state ./src/include/run_event_list_thread.h:int elp_thread_is_runnig(struct elp_thread_args *args); ./src/include/run_event_list_thread.h: * Notify a thread process to perform next event list process step. ./src/include/run_event_list_thread.h: * @param args Not NULL pointer to thread process state ./src/include/run_event_list_thread.h:void elp_thread_loop_next_step(struct elp_thread_args *args); ./src/include/run_event_list_thread.h: * @param args Not NULL pointer to thread process state ./src/include/run_event_list_thread.h:void elp_thread_loop_kill_command(struct elp_thread_args *args); ./src/include/Makefile.am: run_event_list_thread.h ./src/lib/run_event_list.c:#include <pthread.h> ./src/lib/run_event_list.c: pthread_mutex_t run_mut; ./src/lib/run_event_list.c: pthread_mutex_init(&(process->run_mut), NULL); ./src/lib/run_event_list.c: pthread_mutex_destroy(&(process->run_mut)); ./src/lib/run_event_list.c: pthread_mutex_lock(&(process->run_mut)); ./src/lib/run_event_list.c: pthread_mutex_unlock(&(process->run_mut)); ./src/lib/run_event_list.c: pthread_mutex_lock(&(process->run_mut)); ./src/lib/run_event_list.c: pthread_mutex_unlock(&(process->run_mut)); ./src/lib/run_event_list.c: pthread_mutex_lock(&(process->run_mut)); ./src/lib/run_event_list.c: pthread_mutex_unlock(&(process->run_mut)); ./src/lib/run_event_list.c: pthread_mutex_lock(&(process->run_mut)); ./src/lib/run_event_list.c: pthread_mutex_unlock(&(process->run_mut)); ./src/lib/run_event_list.c: /* have to create a copy because of threads */ ./src/lib/run_event_list.c: pthread_mutex_lock(&(process->run_mut)); ./src/lib/run_event_list.c: pthread_mutex_unlock(&(process->run_mut)); ./src/lib/run_event_list.c: pthread_mutex_lock(&(process->run_mut)); ./src/lib/run_event_list.c: pthread_mutex_unlock(&(process->run_mut)); ./src/lib/run_event_list_thread.c:#include <pthread.h> ./src/lib/run_event_list_thread.c:#include "run_event_list_thread.h" ./src/lib/run_event_list_thread.c:struct elp_thread_args ./src/lib/run_event_list_thread.c: pthread_t process_tid; ./src/lib/run_event_list_thread.c: pthread_mutex_t sync_mut; ./src/lib/run_event_list_thread.c: pthread_cond_t proc_cond; ./src/lib/run_event_list_thread.c: pthread_cond_t step_cond; ./src/lib/run_event_list_thread.c: elp_thread_args_callback start_cb; ./src/lib/run_event_list_thread.c: elp_thread_args_callback step_done_cb; ./src/lib/run_event_list_thread.c: elp_thread_args_callback finish_cb; ./src/lib/run_event_list_thread.c:struct elp_thread_args *new_elp_thread_args() ./src/lib/run_event_list_thread.c: struct elp_thread_args *args = xzalloc(sizeof(*args)); ./src/lib/run_event_list_thread.c: pthread_mutex_init(&(args->sync_mut), NULL); ./src/lib/run_event_list_thread.c: pthread_cond_init(&(args->proc_cond), NULL); ./src/lib/run_event_list_thread.c: pthread_cond_init(&(args->step_cond), NULL); ./src/lib/run_event_list_thread.c:void elp_thread_args_set_on_start(struct elp_thread_args *args, ./src/lib/run_event_list_thread.c: elp_thread_args_callback cb) ./src/lib/run_event_list_thread.c:void elp_thread_args_set_on_step_done(struct elp_thread_args *args, ./src/lib/run_event_list_thread.c: elp_thread_args_callback cb) ./src/lib/run_event_list_thread.c:void elp_thread_args_set_on_finish(struct elp_thread_args *args, ./src/lib/run_event_list_thread.c: elp_thread_args_callback cb) ./src/lib/run_event_list_thread.c:static void elp_thread_args_set_process(struct elp_thread_args *args, ./src/lib/run_event_list_thread.c:struct event_list_process *elp_thread_args_get_process(const struct elp_thread_args *args) ./src/lib/run_event_list_thread.c:static void elp_thread_args_on_start(struct elp_thread_args *args) ./src/lib/run_event_list_thread.c:static void elp_thread_args_on_step_done(struct elp_thread_args *args) ./src/lib/run_event_list_thread.c:static void elp_thread_args_on_finish(struct elp_thread_args *args) ./src/lib/run_event_list_thread.c:void elp_thread_interrupt_internal(struct elp_thread_args *args) ./src/lib/run_event_list_thread.c:static void elp_thread_kill_internal(struct elp_thread_args *args) ./src/lib/run_event_list_thread.c: elp_thread_interrupt_internal(args); ./src/lib/run_event_list_thread.c:static int elp_thread_wait_on_process(struct elp_thread_args *args) ./src/lib/run_event_list_thread.c: pthread_mutex_lock(&(args->sync_mut)); ./src/lib/run_event_list_thread.c: const int error = pthread_cond_wait(&(args->proc_cond), &(args->sync_mut)); ./src/lib/run_event_list_thread.c: elp_thread_kill_internal(args); ./src/lib/run_event_list_thread.c: pthread_mutex_unlock(&(args->sync_mut)); ./src/lib/run_event_list_thread.c:static int elp_thread_wait_on_step(struct elp_thread_args *args) ./src/lib/run_event_list_thread.c: pthread_mutex_lock(&(args->sync_mut)); ./src/lib/run_event_list_thread.c: const int error = pthread_cond_wait(&(args->step_cond), &(args->sync_mut)); ./src/lib/run_event_list_thread.c: elp_thread_kill_internal(args); ./src/lib/run_event_list_thread.c: pthread_mutex_unlock(&(args->sync_mut)); ./src/lib/run_event_list_thread.c:void free_elp_thread_args(struct elp_thread_args *args) ./src/lib/run_event_list_thread.c: pthread_cond_destroy(&(args->step_cond)); ./src/lib/run_event_list_thread.c: pthread_cond_destroy(&(args->proc_cond)); ./src/lib/run_event_list_thread.c: pthread_mutex_destroy(&(args->sync_mut)); ./src/lib/run_event_list_thread.c:void elp_thread_run(struct elp_thread_args *args, struct event_list_process *process) ./src/lib/run_event_list_thread.c: pthread_mutex_lock(&(args->sync_mut)); ./src/lib/run_event_list_thread.c: if (elp_thread_is_runnig(args)) ./src/lib/run_event_list_thread.c: error_msg_and_die("event list process thread is already runnig"); ./src/lib/run_event_list_thread.c: elp_thread_args_set_process(args, process); ./src/lib/run_event_list_thread.c: pthread_cond_signal(&(args->proc_cond)); ./src/lib/run_event_list_thread.c: pthread_mutex_unlock(&(args->sync_mut)); ./src/lib/run_event_list_thread.c:int elp_thread_is_runnig(struct elp_thread_args *args) ./src/lib/run_event_list_thread.c:void elp_thread_loop_next_step(struct elp_thread_args *args) ./src/lib/run_event_list_thread.c: pthread_mutex_lock(&(args->sync_mut)); ./src/lib/run_event_list_thread.c: pthread_cond_signal(&(args->step_cond)); ./src/lib/run_event_list_thread.c: pthread_mutex_unlock(&(args->sync_mut)); ./src/lib/run_event_list_thread.c:static void *elp_thread(void *args) ./src/lib/run_event_list_thread.c: struct elp_thread_args *p = (struct elp_thread_args *)args; ./src/lib/run_event_list_thread.c: while (elp_thread_wait_on_process(p)) ./src/lib/run_event_list_thread.c: elp_thread_args_on_start(p); ./src/lib/run_event_list_thread.c: pthread_mutex_lock(&(p->sync_mut)); ./src/lib/run_event_list_thread.c: pthread_mutex_unlock(&(p->sync_mut)); ./src/lib/run_event_list_thread.c: elp_thread_wait_on_step(p); ./src/lib/run_event_list_thread.c: while (elp_thread_wait_on_step(p) && cont) ./src/lib/run_event_list_thread.c: elp_thread_args_on_step_done(p); ./src/lib/run_event_list_thread.c: pthread_mutex_lock(&(p->sync_mut)); ./src/lib/run_event_list_thread.c: pthread_mutex_unlock(&(p->sync_mut)); ./src/lib/run_event_list_thread.c: elp_thread_args_on_finish(p); ./src/lib/run_event_list_thread.c:int elp_thread_init(struct elp_thread_args *args) ./src/lib/run_event_list_thread.c: return pthread_create(&(args->process_tid), NULL, elp_thread, args); ./src/lib/run_event_list_thread.c:void elp_thread_loop_kill_command(struct elp_thread_args *args) ./src/lib/run_event_list_thread.c: pthread_mutex_lock(&(args->sync_mut)); ./src/lib/run_event_list_thread.c: pthread_mutex_unlock(&(args->sync_mut)); ./src/lib/run_event_list_thread.c:void elp_thread_interrupt(struct elp_thread_args *args) ./src/lib/run_event_list_thread.c: pthread_mutex_lock(&(args->sync_mut)); ./src/lib/run_event_list_thread.c: elp_thread_interrupt_internal(args); ./src/lib/run_event_list_thread.c: pthread_cond_signal(&(args->step_cond)); ./src/lib/run_event_list_thread.c: pthread_mutex_unlock(&(args->sync_mut)); ./src/lib/run_event_list_thread.c:void elp_thread_kill(struct elp_thread_args *args) ./src/lib/run_event_list_thread.c: pthread_mutex_lock(&(args->sync_mut)); ./src/lib/run_event_list_thread.c: elp_thread_kill_internal(args); ./src/lib/run_event_list_thread.c: pthread_cond_signal(&(args->proc_cond)); ./src/lib/run_event_list_thread.c: pthread_mutex_unlock(&(args->sync_mut)); ./src/lib/json.c:#include <btparser/thread.h> ./src/lib/json.c: struct btp_thread *core_bt = btp_load_core_backtrace(pd_item); ./src/lib/json.c: ureport_add_int(item, "thread", 0); ./src/lib/json.c: btp_thread_free(core_bt); ./src/lib/json.c: * ureport_add_item_int(ureport, pd, "crash_thread", NULL); ./src/lib/json.c: ureport_add_int(ureport, "crash_thread", 0); ./src/lib/Makefile.am: run_event_list_thread.c ./src/plugins/rhbz.c:#include <btparser/thread.h> ./src/plugins/rhbz.c: * - otherwise preview of crashed thread stack trace is created ./src/plugins/rhbz.c: /* Get optimized thread stack trace for 10 top most frames */ ./src/plugins/rhbz.c: struct btp_thread *thread = btp_backtrace_get_optimized_thread(backtrace, 10); ./src/plugins/rhbz.c: if (!thread) ./src/plugins/rhbz.c: log(_("Can't find crash thread")); ./src/plugins/rhbz.c: btp_thread_append_to_str(thread, bt, true); ./src/plugins/rhbz.c: btp_thread_free(thread);
I don't think this is an improvement. Who will debug the next nightmage threading bug?
I use only two threads :( All nightmare stuff is hidden in run_event_list_thread.
From my point of view, this approach is better than running event processes
from GUI * it allows me to use callbacks in run_event_on_dir(). * it is not necessary to implement run_event_on_dir in GUI again * implementation of run event list can be shared between reporters * reporters are much more simple