Hi, this looks mostly fine, few comments below, and we want a v2 after those.
Oh, could you put your real name in the sender field, so we get it in the git history properly? On Wed, 27 Mar 2013 14:59:28 +0100 blackwolf12333 <[email protected]> wrote: > --- > src/compositor.c | 58 > +++++++++++++++++++++++++++++++++++++++++++++++++------- > 1 file changed, 51 insertions(+), 7 deletions(-) > > diff --git a/src/compositor.c b/src/compositor.c > index a2860fd..92e9cbe 100644 > --- a/src/compositor.c > +++ b/src/compositor.c > @@ -3220,7 +3220,7 @@ print_backtrace(void) > > #endif > > -static void > +/*static void // we should not need them anymore > on_segv_signal(int s, siginfo_t *siginfo, void *context) > { > /* This SIGSEGV handler will do a best-effort backtrace, and > @@ -3229,7 +3229,7 @@ on_segv_signal(int s, siginfo_t *siginfo, void *context) > * raise SIGTRAP. If we run weston under gdb from X or a > * different vt, and tell gdb "handle SIGSEGV nostop", this > * will allow weston to switch back to gdb on crash and then > - * gdb will catch the crash with SIGTRAP. */ > + * gdb will catch the crash with SIGTRAP. > > weston_log("caught segv\n"); > > @@ -3240,6 +3240,40 @@ on_segv_signal(int s, siginfo_t *siginfo, void > *context) > raise(SIGTRAP); > } > > +static void > +on_abrt_signal(int s, siginfo_t *siginfo, void *context) > +{ > + /* This SIGABRT handler will do a best-effort backtrace, and > + * then call the backend restore function, which will switch > + * back to the vt we launched from or ungrab X etc and then > + * raise SIGTRAP. If we run weston under gdb from X or a > + * different vt, and tell gdb "handle SIGABRT nostop", this > + * will allow weston to switch back to gdb on crash and then > + * gdb will catch the crash with SIGTRAP. > + > + weston_log("caught abrt\n"); > + > + print_backtrace(); > + > + segv_compositor->restore(segv_compositor); > + > + raise(SIGTRAP); > +}*/ Just delete all the dead code, there no point in keeping it. > + > +static void > +on_caught_signal(int s, siginfo_t *siginfo, void *context) > +{ > + /* This handler should handle all signals set in catch_signals() > + * Currently that would be: SIG_SEGV and SIG_ABRT Spelled without the underscore in code. > + */ > + weston_log("caught signal: %d\n", s); > + > + print_backtrace(); > + > + segv_compositor->restore(segv_compositor); > + > + raise(SIGTRAP); > +} > > static void * > load_module(const char *name, const char *entrypoint) > @@ -3393,6 +3427,20 @@ usage(int error_code) > exit(error_code); > } > > +static void > +catch_signals() I'd expect a void in parens, doesn't this produce a warning? > +{ > + struct sigaction action; > + > + action.sa_flags = SA_SIGINFO | SA_RESETHAND; > + action.sa_sigaction = on_caught_signal; > + sigemptyset(&action.sa_mask); > + sigaction(SIGSEGV, &action, NULL); > + > + sigemptyset(&action.sa_mask); No need to sigemptyset again, the argument to sigaction is const. > + sigaction(SIGABRT, &action, NULL); > +} > + > int main(int argc, char *argv[]) > { > int ret = EXIT_SUCCESS; > @@ -3400,7 +3448,6 @@ int main(int argc, char *argv[]) > struct weston_compositor *ec; > struct wl_event_source *signals[4]; > struct wl_event_loop *loop; > - struct sigaction segv_action; > struct weston_compositor > *(*backend_init)(struct wl_display *display, > int *argc, char *argv[], const char > *config_file); > @@ -3491,10 +3538,7 @@ int main(int argc, char *argv[]) > exit(EXIT_FAILURE); > } > > - segv_action.sa_flags = SA_SIGINFO | SA_RESETHAND; > - segv_action.sa_sigaction = on_segv_signal; > - sigemptyset(&segv_action.sa_mask); > - sigaction(SIGSEGV, &segv_action, NULL); > + catch_signals(); > segv_compositor = ec; > > ec->option_idle_time = idle_time; With the above fixed, reviewed-by me. Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
