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)
