Re: [PATCH weston v2] Copying xkb_info when creating a seat causes problems
On Thu, Sep 05, 2013 at 01:31:40PM +, Andrew Wedgbury wrote: Hi Kristian, Here's a new patch for ref counting weston_xkb_info, as suggested. So a seat created with a NULL keymap will now point to the global xkb_info. Andrew, that looks great. It's a nice cleanup in itself and of course fixes the premature munmap error. Thanks, applied. Kristian --- src/compositor.h |5 ++-- src/input.c| 77 src/text-backend.c |4 +-- 3 files changed, 53 insertions(+), 33 deletions(-) diff --git a/src/compositor.h b/src/compositor.h index 6db3c61..3755650 100644 --- a/src/compositor.h +++ b/src/compositor.h @@ -398,6 +398,7 @@ struct weston_xkb_info { int keymap_fd; size_t keymap_size; char *keymap_area; + int32_t ref_count; xkb_mod_index_t shift_mod; xkb_mod_index_t caps_mod; xkb_mod_index_t ctrl_mod; @@ -468,7 +469,7 @@ struct weston_seat { void (*led_update)(struct weston_seat *ws, enum weston_led leds); - struct weston_xkb_info xkb_info; + struct weston_xkb_info *xkb_info; struct { struct xkb_state *state; enum weston_led leds; @@ -588,7 +589,7 @@ struct weston_compositor { struct xkb_rule_names xkb_names; struct xkb_context *xkb_context; - struct weston_xkb_info xkb_info; + struct weston_xkb_info *xkb_info; /* Raw keyboard processing (no libxkbcommon initialization or handling) */ int use_xkbcommon; diff --git a/src/input.c b/src/input.c index aa40b4e..78b6ead 100644 --- a/src/input.c +++ b/src/input.c @@ -760,24 +760,24 @@ notify_modifiers(struct weston_seat *seat, uint32_t serial) /* And update the modifier_state for bindings. */ mods_lookup = mods_depressed | mods_latched; seat-modifier_state = 0; - if (mods_lookup (1 seat-xkb_info.ctrl_mod)) + if (mods_lookup (1 seat-xkb_info-ctrl_mod)) seat-modifier_state |= MODIFIER_CTRL; - if (mods_lookup (1 seat-xkb_info.alt_mod)) + if (mods_lookup (1 seat-xkb_info-alt_mod)) seat-modifier_state |= MODIFIER_ALT; - if (mods_lookup (1 seat-xkb_info.super_mod)) + if (mods_lookup (1 seat-xkb_info-super_mod)) seat-modifier_state |= MODIFIER_SUPER; - if (mods_lookup (1 seat-xkb_info.shift_mod)) + if (mods_lookup (1 seat-xkb_info-shift_mod)) seat-modifier_state |= MODIFIER_SHIFT; /* Finally, notify the compositor that LEDs have changed. */ if (xkb_state_led_index_is_active(seat-xkb_state.state, - seat-xkb_info.num_led)) + seat-xkb_info-num_led)) leds |= LED_NUM_LOCK; if (xkb_state_led_index_is_active(seat-xkb_state.state, - seat-xkb_info.caps_led)) + seat-xkb_info-caps_led)) leds |= LED_CAPS_LOCK; if (xkb_state_led_index_is_active(seat-xkb_state.state, - seat-xkb_info.scroll_led)) + seat-xkb_info-scroll_led)) leds |= LED_SCROLL_LOCK; if (leds != seat-xkb_state.leds seat-led_update) seat-led_update(seat, leds); @@ -1243,8 +1243,8 @@ seat_get_keyboard(struct wl_client *client, struct wl_resource *resource, if (seat-compositor-use_xkbcommon) { wl_keyboard_send_keymap(cr, WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1, - seat-xkb_info.keymap_fd, - seat-xkb_info.keymap_size); + seat-xkb_info-keymap_fd, + seat-xkb_info-keymap_size); } else { int null_fd = open(/dev/null, O_RDONLY); wl_keyboard_send_keymap(cr, WL_KEYBOARD_KEYMAP_FORMAT_NO_KEYMAP, @@ -1351,8 +1351,12 @@ weston_compositor_xkb_init(struct weston_compositor *ec, return 0; } -static void xkb_info_destroy(struct weston_xkb_info *xkb_info) +static void +weston_xkb_info_destroy(struct weston_xkb_info *xkb_info) { + if (--xkb_info-ref_count 0) + return; + if (xkb_info-keymap) xkb_map_unref(xkb_info-keymap); @@ -1360,6 +1364,7 @@ static void xkb_info_destroy(struct weston_xkb_info *xkb_info) munmap(xkb_info-keymap_area, xkb_info-keymap_size); if (xkb_info-keymap_fd = 0) close(xkb_info-keymap_fd); + free(xkb_info); } void @@ -1377,14 +1382,22 @@ weston_compositor_xkb_destroy(struct weston_compositor *ec) free((char *) ec-xkb_names.layout); free((char *) ec-xkb_names.variant); free((char *) ec-xkb_names.options); - - xkb_info_destroy(ec-xkb_info); + + if
[PATCH weston v2] Copying xkb_info when creating a seat causes problems
Hi Kristian, Here's a new patch for ref counting weston_xkb_info, as suggested. So a seat created with a NULL keymap will now point to the global xkb_info. --- src/compositor.h |5 ++-- src/input.c| 77 src/text-backend.c |4 +-- 3 files changed, 53 insertions(+), 33 deletions(-) diff --git a/src/compositor.h b/src/compositor.h index 6db3c61..3755650 100644 --- a/src/compositor.h +++ b/src/compositor.h @@ -398,6 +398,7 @@ struct weston_xkb_info { int keymap_fd; size_t keymap_size; char *keymap_area; + int32_t ref_count; xkb_mod_index_t shift_mod; xkb_mod_index_t caps_mod; xkb_mod_index_t ctrl_mod; @@ -468,7 +469,7 @@ struct weston_seat { void (*led_update)(struct weston_seat *ws, enum weston_led leds); - struct weston_xkb_info xkb_info; + struct weston_xkb_info *xkb_info; struct { struct xkb_state *state; enum weston_led leds; @@ -588,7 +589,7 @@ struct weston_compositor { struct xkb_rule_names xkb_names; struct xkb_context *xkb_context; - struct weston_xkb_info xkb_info; + struct weston_xkb_info *xkb_info; /* Raw keyboard processing (no libxkbcommon initialization or handling) */ int use_xkbcommon; diff --git a/src/input.c b/src/input.c index aa40b4e..78b6ead 100644 --- a/src/input.c +++ b/src/input.c @@ -760,24 +760,24 @@ notify_modifiers(struct weston_seat *seat, uint32_t serial) /* And update the modifier_state for bindings. */ mods_lookup = mods_depressed | mods_latched; seat-modifier_state = 0; - if (mods_lookup (1 seat-xkb_info.ctrl_mod)) + if (mods_lookup (1 seat-xkb_info-ctrl_mod)) seat-modifier_state |= MODIFIER_CTRL; - if (mods_lookup (1 seat-xkb_info.alt_mod)) + if (mods_lookup (1 seat-xkb_info-alt_mod)) seat-modifier_state |= MODIFIER_ALT; - if (mods_lookup (1 seat-xkb_info.super_mod)) + if (mods_lookup (1 seat-xkb_info-super_mod)) seat-modifier_state |= MODIFIER_SUPER; - if (mods_lookup (1 seat-xkb_info.shift_mod)) + if (mods_lookup (1 seat-xkb_info-shift_mod)) seat-modifier_state |= MODIFIER_SHIFT; /* Finally, notify the compositor that LEDs have changed. */ if (xkb_state_led_index_is_active(seat-xkb_state.state, - seat-xkb_info.num_led)) + seat-xkb_info-num_led)) leds |= LED_NUM_LOCK; if (xkb_state_led_index_is_active(seat-xkb_state.state, - seat-xkb_info.caps_led)) + seat-xkb_info-caps_led)) leds |= LED_CAPS_LOCK; if (xkb_state_led_index_is_active(seat-xkb_state.state, - seat-xkb_info.scroll_led)) + seat-xkb_info-scroll_led)) leds |= LED_SCROLL_LOCK; if (leds != seat-xkb_state.leds seat-led_update) seat-led_update(seat, leds); @@ -1243,8 +1243,8 @@ seat_get_keyboard(struct wl_client *client, struct wl_resource *resource, if (seat-compositor-use_xkbcommon) { wl_keyboard_send_keymap(cr, WL_KEYBOARD_KEYMAP_FORMAT_XKB_V1, - seat-xkb_info.keymap_fd, - seat-xkb_info.keymap_size); + seat-xkb_info-keymap_fd, + seat-xkb_info-keymap_size); } else { int null_fd = open(/dev/null, O_RDONLY); wl_keyboard_send_keymap(cr, WL_KEYBOARD_KEYMAP_FORMAT_NO_KEYMAP, @@ -1351,8 +1351,12 @@ weston_compositor_xkb_init(struct weston_compositor *ec, return 0; } -static void xkb_info_destroy(struct weston_xkb_info *xkb_info) +static void +weston_xkb_info_destroy(struct weston_xkb_info *xkb_info) { + if (--xkb_info-ref_count 0) + return; + if (xkb_info-keymap) xkb_map_unref(xkb_info-keymap); @@ -1360,6 +1364,7 @@ static void xkb_info_destroy(struct weston_xkb_info *xkb_info) munmap(xkb_info-keymap_area, xkb_info-keymap_size); if (xkb_info-keymap_fd = 0) close(xkb_info-keymap_fd); + free(xkb_info); } void @@ -1377,14 +1382,22 @@ weston_compositor_xkb_destroy(struct weston_compositor *ec) free((char *) ec-xkb_names.layout); free((char *) ec-xkb_names.variant); free((char *) ec-xkb_names.options); - - xkb_info_destroy(ec-xkb_info); + + if (ec-xkb_info) + weston_xkb_info_destroy(ec-xkb_info); xkb_context_unref(ec-xkb_context); } -static int -weston_xkb_info_new_keymap(struct weston_xkb_info *xkb_info)