noop: Call {enable,disable}_seat callbacks. v1 APPLIED

Anna (navi) Figueiredo Gomes: 4
 noop: Call {enable,disable}_seat callbacks.
 noop: Return seat0 as the seat name
 noop: Close the open devices when close device is called.
 noop: Additional open flags for `open(2)`

 4 files changed, 6 insertions(+), 3 deletions(-)

Patch 2 and 4 have been applied (with 1 and 3 discarded as they were 
tested to be unnecessary on IRC). Thanks!

[PATCH 1/4] noop: Call {enable,disable}_seat callbacks. Export this patch

wlroots expects enable_seat to be called in order for the session
backend count as enabled. Without this it times out.

Signed-off-by: Anna (navi) Figueiredo Gomes <navi@vlhl.dev>
---
 libseat/backend/noop.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libseat/backend/noop.c b/libseat/backend/noop.c
index 7db8a55..8818406 100644
--- a/libseat/backend/noop.c
+++ b/libseat/backend/noop.c
@@ -35,6 +35,7 @@ static void destroy(struct backend_noop *backend) {
 
 static int close_seat(struct libseat *base) {
 	struct backend_noop *backend = backend_noop_from_libseat_backend(base);
+	backend->seat_listener->disable_seat(&backend->base, backend->seat_listener_data);
 	destroy(backend);
 	return 0;
 }
@@ -120,6 +121,8 @@ static struct libseat *noop_open_seat(const struct libseat_seat_listener *listen
 	backend->seat_listener_data = data;
 	backend->base.impl = &noop_impl;
 
+	backend->seat_listener->enable_seat(&backend->base, backend->seat_listener_data);
+
 	return &backend->base;
 }

Kenny Levinsen <kl@kl.wtf>

8 months ago

Hmm, looking at the code we should actually already be sending the enable:

1. When seat is opened with noop, `backend->initial_setup` is set to true.
2. In dispatch, if `backend->initial_setup` is set, the enable_seat 
callback is called and the flag is cleared.
3. In wlroots' libseat_session_init, wlroots calls `libseat_dispatch` 
shortly after `libseat_open`, so this logic should activate.

Note that 9b8b6e0bf88f02b77835c977ca97017ac6ed850f introduced the 
missing `backend->initial_setup = true` line, and so it would not have 
worked on the 0.7 release.

Would you mind checking why the current setup is not working? If it 
cannot work, it should be removed - but if it can be made to work, 
firing the event from dispatch will be more in line with the other backends.

 
-- 
2.39.2

[PATCH 2/4] noop: Return seat0 as the seat name Export this patch

wlroots' libinput backend expects the name of the seat to either match
ID_SEAT from udev, or in case ID_SEAT returns nothing, match seat0. As
noop has no seat switching, always returning seat0 as the session name
fixes that.

Signed-off-by: Anna (navi) Figueiredo Gomes <navi@vlhl.dev>
---
 libseat/backend/noop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libseat/backend/noop.c b/libseat/backend/noop.c
index 8818406..0cf19ca 100644
--- a/libseat/backend/noop.c
+++ b/libseat/backend/noop.c
@@ -47,7 +47,7 @@ static int disable_seat(struct libseat *base) {
 
 static const char *seat_name(struct libseat *base) {
 	(void)base;
-	return "noop";
+	return "seat0";
 }
 
 static int open_device(struct libseat *base, const char *path, int *fd) {
-- 
2.39.2

[PATCH 3/4] noop: Close the open devices when close device is called. Export this patch

Signed-off-by: Anna (navi) Figueiredo Gomes <navi@vlhl.dev>
---
 libseat/backend/noop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libseat/backend/noop.c b/libseat/backend/noop.c
index 0cf19ca..c1d8f99 100644
--- a/libseat/backend/noop.c
+++ b/libseat/backend/noop.c
@@ -65,7 +65,7 @@ static int open_device(struct libseat *base, const char *path, int *fd) {
 
 static int close_device(struct libseat *base, int device_id) {
 	(void)base;
-	(void)device_id;
+	close(device_id);
 	return 0;
 }

Kenny Levinsen <kl@kl.wtf>

8 months ago

We are not supposed to close the users' fd here. The call is to tell 
seatd/logind that we're done with the device, which is just a noop for 
the noop backend.

You can see wlroot's own close after libseat_close_device in 
wlr_session_close_file, so this would become a double-close.

 
-- 
2.39.2

Kenny Levinsen <kl@kl.wtf>

8 months ago

[PATCH 4/4] noop: Additional open flags for `open(2)` Export this patch

Matching the functionallity by the seatd open call. O_NONBLOCK is
specially important for libseat, otherwise it hangs while trying to
drain all events from an input device fd.

Signed-off-by: Anna (navi) Figueiredo Gomes <navi@vlhl.dev>
---
 libseat/backend/noop.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libseat/backend/noop.c b/libseat/backend/noop.c
index c1d8f99..2167473 100644
--- a/libseat/backend/noop.c
+++ b/libseat/backend/noop.c
@@ -53,7 +53,7 @@ static const char *seat_name(struct libseat *base) {
 static int open_device(struct libseat *base, const char *path, int *fd) {
 	(void)base;
 
-	int tmpfd = open(path, O_RDWR | O_CLOEXEC);
+	int tmpfd = open(path, O_RDWR | O_NOCTTY | O_NOFOLLOW | O_CLOEXEC | O_NONBLOCK);
 	if (tmpfd < 0) {
 		log_errorf("Failed to open device: %s", strerror(errno));
 		return -1;
-- 
2.39.2

Kenny Levinsen <kl@kl.wtf>

8 months ago

Patch 2 and 4 have been applied (with 1 and 3 discarded as they were 
tested to be unnecessary on IRC). Thanks!