Experimental Wayland support #3
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds experimental Wayland support to the runwhenidle application using the ext-idle-notify-v1 protocol, providing an alternative to X11 for detecting user idle time. The implementation attempts to initialize Wayland first, falling back to the existing X11 implementation if Wayland is not available.
Key changes:
- Implements Wayland idle detection using the ext-idle-notify-v1 protocol with event-driven notifications
- Adds helper functions for Linux-specific features (pidfd_open syscall, timerfd for event-driven timing)
- Introduces a SIGCHLD handler to detect child process exits more reliably
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| main.c | Adds Wayland client initialization, event loop, idle notification callbacks, and helper functions for timer/process management; restructures main() to try Wayland before X11 |
| ext-idle-notify-v1-protocol.c | Auto-generated Wayland protocol implementation file for ext-idle-notify-v1 |
| ext-idle-notify-v1-client-protocol.h | Auto-generated header file with protocol definitions and inline functions for ext-idle-notify-v1 client interface |
| Makefile | Updates to include Wayland client library, math library, and the new protocol source file in the build |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| PREFIX := /usr | ||
| endif | ||
| SOURCES = time_utils.c sleep_utils.c tty_utils.c process_handling.c arguments_parsing.c main.c | ||
| SOURCES = time_utils.c sleep_utils.c tty_utils.c process_handling.c arguments_parsing.c ext-idle-notify-v1-protocol.c main.c |
There was a problem hiding this comment.
There are two spaces between 'arguments_parsing.c' and 'ext-idle-notify-v1-protocol.c' which is inconsistent with the single space used between other files in the SOURCES list.
| SOURCES = time_utils.c sleep_utils.c tty_utils.c process_handling.c arguments_parsing.c ext-idle-notify-v1-protocol.c main.c | |
| SOURCES = time_utils.c sleep_utils.c tty_utils.c process_handling.c arguments_parsing.c ext-idle-notify-v1-protocol.c main.c |
| int start_monitor_poll_index = poll_file_descriptor_count++; | ||
| poll_file_descriptors[start_monitor_poll_index] = (struct pollfd){ .fd = start_monitor_timer_file_descriptor, .events = POLLIN, .revents = 0 }; |
There was a problem hiding this comment.
If create_one_shot_timer_file_descriptor_after_ms() fails and returns -1, this file descriptor is still unconditionally added to the poll array at lines 417-418. Polling on an invalid file descriptor (-1) can cause undefined behavior or poll failures. This should be handled conditionally like the other optional file descriptors (process_exit_wait_file_descriptor and external_pid_fallback_check_timer_file_descriptor), checking if the descriptor is valid before adding it to the poll array.
| int start_monitor_poll_index = poll_file_descriptor_count++; | |
| poll_file_descriptors[start_monitor_poll_index] = (struct pollfd){ .fd = start_monitor_timer_file_descriptor, .events = POLLIN, .revents = 0 }; | |
| int start_monitor_poll_index = -1; | |
| if (start_monitor_timer_file_descriptor >= 0) { | |
| start_monitor_poll_index = poll_file_descriptor_count++; | |
| poll_file_descriptors[start_monitor_poll_index] = (struct pollfd){ | |
| .fd = start_monitor_timer_file_descriptor, .events = POLLIN, .revents = 0 | |
| }; | |
| } |
| if (!monitoring_started && (poll_file_descriptors[start_monitor_poll_index].revents & POLLIN)) { | ||
| consume_timer_file_descriptor(start_monitor_timer_file_descriptor); | ||
| monitoring_started = 1; | ||
|
|
||
| if (verbose) { | ||
| fprintf(stderr, "Starting to monitor user activity (Wayland ext-idle-notify-v1)\n"); | ||
| } | ||
|
|
||
| if (start_wayland_idle_notification_object() < 0) { | ||
| fprintf_error("Failed to create Wayland idle notification object, user will be considered idle.\n"); | ||
| } else { | ||
| if (!command_paused) { | ||
| pause_command_recursively(pid); | ||
| command_paused = 1; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This code checks the poll events for the start_monitor_timer without first verifying that the timer file descriptor was successfully created. If create_one_shot_timer_file_descriptor_after_ms() failed at line 402, this would be checking events for an invalid file descriptor. The check should be guarded with a condition verifying the timer was created successfully, similar to how process_exit_poll_index and external_pid_fallback_poll_index are handled at lines 481 and 485.
Makefile
Outdated
| @@ -1,10 +1,10 @@ | |||
| TARGET_EXEC := runwhenidle | |||
| LDLIBS=-lXss -lX11 | |||
| LDLIBS=-lXss -lX11 -lwayland-client -lm | |||
There was a problem hiding this comment.
The math library (-lm) is being linked but no math functions are used in the code. This adds an unnecessary dependency that doesn't appear to be needed for the Wayland support implementation.
| LDLIBS=-lXss -lX11 -lwayland-client -lm | |
| LDLIBS=-lXss -lX11 -lwayland-client |
No description provided.