diff --git a/src/libei-device.c b/src/libei-device.c index 9a74903..c85cef6 100644 --- a/src/libei-device.c +++ b/src/libei-device.c @@ -59,38 +59,13 @@ ei_device_set_state(struct ei_device *device, ei_device_state_to_string(state)); } -static void -ei_device_add_to_seat(struct ei_device *device) -{ - struct ei_seat *seat = ei_device_get_seat(device); - - list_remove(&device->link); /* remove from pending */ - list_append(&seat->devices, &device->link); - /* Once the seat has the device proper, it owns a ref to the - * device (and the device always owns a ref to the seat) - */ - ei_device_ref(device); - - ei_device_set_state(device, EI_DEVICE_STATE_CONNECTING); -} - -static void -ei_device_remove_from_seat(struct ei_device *device) -{ - list_remove(&device->link); - list_init(&device->link); - /* seat only has a ref for devices already added */ - if (device->state != EI_DEVICE_STATE_NEW) - ei_device_unref(device); -} - static void ei_device_destroy(struct ei_device *device) { struct ei_seat *seat = ei_device_get_seat(device); - if (device->state == EI_DEVICE_STATE_NEW) - ei_device_remove_from_seat(device); + assert(device->state == EI_DEVICE_STATE_DEAD); + list_remove(&device->link); ei_keymap_unref(device->keymap); ei_seat_unref(seat); free(device->name); @@ -138,12 +113,15 @@ ei_device_new(struct ei_seat *seat) device->state = EI_DEVICE_STATE_NEW; device->name = xaprintf("unnamed device %d", device->id); - list_append(&seat->devices_pending, &device->link); /* not a ref! */ - /* We have a ref to the seat to make sure our seat doesn't get * destroyed while a ref to the device is still alive. + * And the seat has a ref to the device, dropped when the device + * state changes to DEAD (ei_device_remove()). */ ei_seat_ref(seat); + ei_device_ref(device); + + list_append(&seat->devices, &device->link); return device; } @@ -354,7 +332,7 @@ ei_device_add(struct ei_device *device) __func__, device->name); if (ei_send_add_device(device) == 0) - ei_device_add_to_seat(device); + ei_device_set_state(device, EI_DEVICE_STATE_CONNECTING); } _public_ void @@ -362,14 +340,17 @@ ei_device_remove(struct ei_device *device) { switch (device->state) { case EI_DEVICE_STATE_NEW: + ei_device_set_state(device, EI_DEVICE_STATE_DEAD); + ei_device_unref(device); + break; case EI_DEVICE_STATE_DEAD: case EI_DEVICE_STATE_REMOVED_FROM_CLIENT: break; case EI_DEVICE_STATE_REMOVED_FROM_SERVER: /* device is properly dead now */ ei_device_set_state(device, EI_DEVICE_STATE_DEAD); - ei_device_remove_from_seat(device); ei_send_remove_device(device); + ei_device_unref(device); break; case EI_DEVICE_STATE_CONNECTING: case EI_DEVICE_STATE_SUSPENDED: @@ -392,14 +373,14 @@ ei_device_removed_by_server(struct ei_device *device) /* device is properly dead now */ ei_queue_device_removed_event(device); ei_device_set_state(device, EI_DEVICE_STATE_DEAD); - ei_device_remove_from_seat(device); + ei_device_unref(device); break; case EI_DEVICE_STATE_CONNECTING: /* Device never seen by server, but our client still thinks * it's there */ ei_queue_device_removed_event(device); ei_device_set_state(device, EI_DEVICE_STATE_DEAD); - ei_device_remove_from_seat(device); + ei_device_unref(device); break; case EI_DEVICE_STATE_SUSPENDED: case EI_DEVICE_STATE_RESUMED: @@ -647,8 +628,12 @@ ei_touch_up(struct ei_touch *touch) #include "src/util-memfile.h" #define FAKE_SEAT(_seat) \ + struct ei ei_##__LINE__ = { 0 }; \ struct ei_seat seat_##__LINE__ = { \ - .object = { .refcount = 1, }, \ + .object = { \ + .parent = &ei_##__LINE__.object, \ + .refcount = 1, \ + }, \ .id = __LINE__ << 16, \ .capabilities = ~0, \ .name = "default", \ @@ -669,6 +654,9 @@ MUNIT_TEST(test_device_new) struct ei_device *unrefd = ei_device_unref(d); munit_assert_ptr_null(unrefd); + + ei_device_remove(d); + return MUNIT_OK; } @@ -683,6 +671,10 @@ MUNIT_TEST(test_device_ids) munit_assert_int(d1->id, <, d3->id); munit_assert_int(d2->id, <, d3->id); + ei_device_remove(d1); + ei_device_remove(d2); + ei_device_remove(d3); + return MUNIT_OK; } @@ -691,14 +683,17 @@ MUNIT_TEST(test_device_ref_unref) FAKE_SEAT(seat); struct ei_device *d = ei_device_new(seat); - munit_assert_int(d->object.refcount, ==, 1); + munit_assert_int(d->object.refcount, ==, 2); struct ei_device *refd = ei_device_ref(d); munit_assert_ptr_equal(d, refd); - munit_assert_int(d->object.refcount, ==, 2); + munit_assert_int(d->object.refcount, ==, 3); struct ei_device *unrefd = ei_device_unref(d); munit_assert_ptr_null(unrefd); + munit_assert_int(d->object.refcount, ==, 2); + + ei_device_remove(d); munit_assert_int(d->object.refcount, ==, 1); unrefd = ei_device_unref(d); @@ -732,6 +727,8 @@ MUNIT_TEST(test_device_cap) munit_assert(!ei_device_configure_capability(d, EI_DEVICE_CAP_TOUCH + 1)); munit_assert(!ei_device_has_capability(d, EI_DEVICE_CAP_TOUCH + 1)); + ei_device_remove(d); + return MUNIT_OK; } @@ -743,6 +740,8 @@ MUNIT_TEST(test_device_get_seat) munit_assert_ptr_equal(d->object.parent, seat); munit_assert_ptr_equal(ei_device_get_seat(d), seat); + ei_device_remove(d); + return MUNIT_OK; } @@ -775,6 +774,8 @@ MUNIT_TEST(test_device_pointer_ranges) munit_assert_int(ei_device_pointer_get_width(d), ==, 640); munit_assert_int(ei_device_pointer_get_height(d), ==, 480); + ei_device_remove(d); + return MUNIT_OK; } @@ -807,6 +808,8 @@ MUNIT_TEST(test_device_touch_ranges) munit_assert_int(ei_device_touch_get_width(d), ==, 640); munit_assert_int(ei_device_touch_get_height(d), ==, 480); + ei_device_remove(d); + return MUNIT_OK; } diff --git a/src/libei-seat.c b/src/libei-seat.c index ec695b3..1b17d1f 100644 --- a/src/libei-seat.c +++ b/src/libei-seat.c @@ -71,7 +71,6 @@ ei_seat_new(struct ei *ei, uint32_t id, const char *name, uint32_t capabilities) seat->capabilities = capabilities; list_init(&seat->devices); - list_init(&seat->devices_pending); list_init(&seat->link); return seat; /* ref owned by caller */ @@ -120,9 +119,5 @@ ei_seat_find_device(struct ei_seat *seat, uint32_t deviceid) return device; } - list_for_each(device, &seat->devices_pending, link) { - if (device->id == deviceid) - return device; - } return NULL; } diff --git a/src/libei.h b/src/libei.h index b6e7dca..dd54f95 100644 --- a/src/libei.h +++ b/src/libei.h @@ -532,6 +532,10 @@ ei_device_unref(struct ei_device *device); * the device, then call ei_device_add() to request creation of the device * by the server. * + * @note A caller that does not want to add a created device to a seat + * **must** call ei_device_remove() for this device to ensure the + * resources are released. + * * The returned object must be released by the caller with ei_event_unref() */ struct ei_device * diff --git a/test/test-ei.c b/test/test-ei.c index 158b012..d1ae178 100644 --- a/test/test-ei.c +++ b/test/test-ei.c @@ -596,6 +596,44 @@ MUNIT_TEST(test_ei_device_set_name) return MUNIT_OK; } +MUNIT_TEST(test_ei_device_never_added) +{ + _unref_(peck) *peck = peck_new(); + + peck_enable_eis_behavior(peck, PECK_EIS_BEHAVIOR_ACCEPT_ALL); + peck_dispatch_until_stable(peck); + + /* unref after remove */ + with_client(peck) { + struct ei_seat *seat = peck_ei_get_default_seat(peck); + _unref_(ei_device) *device = ei_device_new(seat); + ei_device_remove(device); + } + + peck_dispatch_until_stable(peck); + + /* unref before remove. + * + * This would be invalid client code since you can't expect to have + * a ref after unref, but since we know the device is still here, we + * can test for the lib being correct. + */ + with_client(peck) { + struct ei_seat *seat = peck_ei_get_default_seat(peck); + struct ei_device *device = ei_device_new(seat); + ei_device_unref(device); + ei_device_remove(device); + } + + peck_dispatch_until_stable(peck); + + with_server(peck) { + peck_assert_no_eis_events(eis); + } + + return MUNIT_OK; +} + MUNIT_TEST(test_ei_device_add_remove) { _unref_(peck) *peck = peck_new();