On Tue, 9 Jun 2015 20:24:26 +0000 Murray Calavera <[email protected]> wrote:
> Currently the text backend will crash the compositor if > seats have already been created. Hi, crash? I didn't realize I should've been looking for causes of crashes, I only thought the input method simply didn't work on the initial seats. Crashing sounds like there might be more to fix. What was the crash exactly? A more to the point patch summary would be "text: handle existing seats on init". Prefix to show the component(s) affected, and a short statement on what is done here. The commit message body could explain better why this is needed: a following patch will be moving text init call into shell modules, which will be called much later than in current code. > Signed-off-by: Murray Calavera <[email protected]> > --- > src/text-backend.c | 29 +++++++++++++++++++---------- > 1 file changed, 19 insertions(+), 10 deletions(-) > > diff --git a/src/text-backend.c b/src/text-backend.c > index daae03c..01cb70a 100644 > --- a/src/text-backend.c > +++ b/src/text-backend.c > @@ -933,13 +933,9 @@ launch_input_method(struct text_backend *text_backend) > } > > static void > -handle_seat_created(struct wl_listener *listener, > - void *data) > +handle_seat_created(struct weston_seat *seat, > + struct text_backend *text_backend) > { I think a better name for this would now be text_backend_seat_created(struct text_backend *text_backend, struct weston_seat *seat) It would go more in line with other text_backend methods (OO-style a bit), and handle_*() functions are usually directly plugged into callbacks. > - struct weston_seat *seat = data; > - struct text_backend *text_backend = > - container_of(listener, struct text_backend, > - seat_created_listener); > struct input_method *input_method; > struct weston_compositor *ec = seat->compositor; > > @@ -966,6 +962,17 @@ handle_seat_created(struct wl_listener *listener, > } > > static void > +notify_seat_created(struct wl_listener *listener, void *data) > +{ Then this would be called handle_seat_created. > + struct weston_seat *seat = data; > + struct text_backend *text_backend = > + container_of(listener, struct text_backend, > + seat_created_listener); > + > + handle_seat_created(seat, text_backend); > +} > + > +static void > text_backend_configuration(struct text_backend *text_backend) > { > struct weston_config_section *section; > @@ -998,11 +1005,11 @@ text_backend_notifier_destroy(struct wl_listener > *listener, void *data) > free(text_backend); > } > > - > WL_EXPORT int > text_backend_init(struct weston_compositor *ec) > { > struct text_backend *text_backend; > + struct weston_seat *seat; > > text_backend = zalloc(sizeof(*text_backend)); > if (text_backend == NULL) > @@ -1010,15 +1017,17 @@ text_backend_init(struct weston_compositor *ec) > > text_backend->compositor = ec; > > - text_backend->seat_created_listener.notify = handle_seat_created; > + text_backend_configuration(text_backend); > + > + wl_list_for_each(seat, &ec->seat_list, link) > + handle_seat_created(seat, text_backend); > + text_backend->seat_created_listener.notify = notify_seat_created; > wl_signal_add(&ec->seat_created_signal, > &text_backend->seat_created_listener); > > text_backend->destroy_listener.notify = text_backend_notifier_destroy; > wl_signal_add(&ec->destroy_signal, &text_backend->destroy_listener); > > - text_backend_configuration(text_backend); > - > text_input_manager_create(ec); > > return 0; The patch looks good to me. As is, it is: Reviewed-by: Pekka Paalanen <[email protected]> in any case, though I think it would be nice to do the minor adjustments I suggested. Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
