From 5c01729d6ffd82d2fe796a72c3216f2f480ef5a9 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Thu, 9 Mar 2023 10:36:43 +0000 Subject: [PATCH] waitforx logging improvements --- sesman/session.c | 26 +++++-- sesman/xwait.c | 69 ++++++++++++++---- sesman/xwait.h | 13 +++- waitforx/Makefile.am | 4 +- waitforx/waitforx.c | 162 +++++++++++++++++++++++++++++-------------- 5 files changed, 200 insertions(+), 74 deletions(-) diff --git a/sesman/session.c b/sesman/session.c index 3bfe5860..8c53e10b 100644 --- a/sesman/session.c +++ b/sesman/session.c @@ -350,9 +350,6 @@ session_start_chansrv(int uid, int display) chansrv_pid = g_fork(); if (chansrv_pid == 0) { - LOG(LOG_LEVEL_INFO, - "Starting the xrdp channel server for display %d", display); - chansrv_params = list_create(); chansrv_params->auto_free = 1; @@ -364,6 +361,9 @@ session_start_chansrv(int uid, int display) g_cfg->env_names, g_cfg->env_values); + LOG(LOG_LEVEL_INFO, + "Starting the xrdp channel server for display %d", display); + /* executing chansrv */ g_execvp_list(exe_path, chansrv_params); @@ -585,12 +585,16 @@ session_start(struct auth_info *auth_info, } else if (window_manager_pid == 0) { + enum xwait_status xws; + env_set_user(s->uid, 0, display, g_cfg->env_names, g_cfg->env_values); - if (wait_for_xserver(display)) + xws = wait_for_xserver(display); + + if (xws == XW_STATUS_OK) { auth_set_env(auth_info); if (s->directory != 0) @@ -661,8 +665,18 @@ session_start(struct auth_info *auth_info, } else { - LOG(LOG_LEVEL_ERROR, - "There is no X server active on display %d", display); + switch (xws) + { + case XW_STATUS_TIMED_OUT: + LOG(LOG_LEVEL_ERROR, "Timed out waiting for X server"); + break; + case XW_STATUS_FAILED_TO_START: + LOG(LOG_LEVEL_ERROR, "X server failed to start"); + break; + default: + LOG(LOG_LEVEL_ERROR, + "An error occurred waiting for the X server"); + } } LOG(LOG_LEVEL_ERROR, "A fatal error has occurred attempting to start " diff --git a/sesman/xwait.c b/sesman/xwait.c index 5cc2775f..a186138b 100644 --- a/sesman/xwait.c +++ b/sesman/xwait.c @@ -9,39 +9,82 @@ #include #include +#include /******************************************************************************/ -int +enum xwait_status wait_for_xserver(int display) { FILE *dp = NULL; - int ret = 0; + enum xwait_status rv = XW_STATUS_MISC_ERROR; + int ret; + char buffer[100]; - char exe_cmd[262]; + const char exe[] = XRDP_LIBEXEC_PATH "/waitforx"; + char cmd[sizeof(exe) + 64]; - LOG(LOG_LEVEL_DEBUG, "Waiting for X server to start on display %d", display); + if (!g_file_exist(exe)) + { + LOG(LOG_LEVEL_ERROR, "Unable to find %s", exe); + return rv; + } - g_snprintf(exe_cmd, sizeof(exe_cmd), "%s/waitforx", XRDP_LIBEXEC_PATH); - dp = popen(exe_cmd, "r"); + g_snprintf(cmd, sizeof(cmd), "%s -d :%d", exe, display); + LOG(LOG_LEVEL_DEBUG, "Waiting for X server to start on display :%d", + display); + + dp = popen(cmd, "r"); if (dp == NULL) { LOG(LOG_LEVEL_ERROR, "Unable to launch waitforx"); - return 1; + return rv; } while (fgets(buffer, 100, dp)) { + const char *msg = buffer; + enum logLevels level = LOG_LEVEL_ERROR; + g_strtrim(buffer, 2); - LOG(LOG_LEVEL_DEBUG, "%s", buffer); + + // Has the message got a class at the start? + if (strlen(buffer) > 3 && buffer[0] == '<' && buffer[2] == '>') + { + switch (buffer[1]) + { + case 'D': + level = LOG_LEVEL_DEBUG; + break; + case 'I': + level = LOG_LEVEL_INFO; + break; + case 'W': + level = LOG_LEVEL_WARNING; + break; + default: + level = LOG_LEVEL_ERROR; + break; + } + msg = buffer + 3; + } + + if (strlen(msg) > 0) + { + LOG(level, "waitforx: %s", msg); + } } ret = pclose(dp); - if (ret != 0) + if (WIFEXITED(ret)) { - LOG(LOG_LEVEL_ERROR, "An error occurred while running waitforx"); - return 0; + rv = (enum xwait_status)WEXITSTATUS(ret); + } + else if (WIFSIGNALED(ret)) + { + int sig = WTERMSIG(ret); + LOG(LOG_LEVEL_ERROR, "waitforx failed with unexpected signal %d", + sig); } - - return 1; + return rv; } diff --git a/sesman/xwait.h b/sesman/xwait.h index d8beb4a5..c9d91d5a 100644 --- a/sesman/xwait.h +++ b/sesman/xwait.h @@ -1,12 +1,21 @@ #ifndef XWAIT_H #define XWAIT_H + +enum xwait_status +{ + XW_STATUS_OK = 0, + XW_STATUS_MISC_ERROR, + XW_STATUS_TIMED_OUT, + XW_STATUS_FAILED_TO_START +}; + /** * * @brief waits for X to start * @param display number - * @return 0 on error, 1 if X has outputs + * @return status * */ -int +enum xwait_status wait_for_xserver(int display); #endif diff --git a/waitforx/Makefile.am b/waitforx/Makefile.am index fa3f9660..4adc6ae8 100644 --- a/waitforx/Makefile.am +++ b/waitforx/Makefile.am @@ -3,7 +3,9 @@ pkglibexec_PROGRAMS = \ AM_LDFLAGS = -lX11 -lXrandr -AM_CPPFLAGS = -I$(top_srcdir)/common +AM_CPPFLAGS = \ + -I$(top_srcdir)/sesman \ + -I$(top_srcdir)/common AM_CFLAGS = $(X_CFLAGS) diff --git a/waitforx/waitforx.c b/waitforx/waitforx.c index c60b114c..b9dabedf 100644 --- a/waitforx/waitforx.c +++ b/waitforx/waitforx.c @@ -8,94 +8,152 @@ #include "config_ac.h" #include "os_calls.h" #include "string_calls.h" +#include "xwait.h" // For return status codes #define ATTEMPTS 10 #define ALARM_WAIT 30 -void +/*****************************************************************************/ +static void alarm_handler(int signal_num) { - /* Avoid printf() in signal handler (see signal-safety(7)) */ - const char msg[] = "Timed out waiting for RandR outputs\n"; + /* Avoid printf() in signal handler (see signal-safety(7)) + * + * Prefix the message with a newline in case another message + * has been partly output */ + const char msg[] = "\nTimed out waiting for RandR outputs\n"; g_file_write(1, msg, g_strlen(msg)); - exit(1); + exit(XW_STATUS_TIMED_OUT); } -int -main(int argc, char **argv) +/*****************************************************************************/ +static Display * +open_display(const char *display) { - char *display = NULL; - int error_base = 0; - int event_base = 0; - int n = 0; - int outputs = 0; - int wait = ATTEMPTS; - Display *dpy = NULL; - XRRScreenResources *res = NULL; + unsigned int wait = ATTEMPTS; + unsigned int n; - display = getenv("DISPLAY"); - - g_set_alarm(alarm_handler, ALARM_WAIT); - - if (!display) - { - printf("DISPLAY is null"); - exit(1); - } - - for (n = 1; n <= wait; ++n) + for (n = 1; n <= ATTEMPTS; ++n) { + printf("Opening display %s. Attempt %u of %u\n", display, n, wait); dpy = XOpenDisplay(display); - printf("Opening display %s. Attempt %d of %d\n", display, n, wait); if (dpy != NULL) { - printf("Opened display %s\n", display); + printf("Opened display %s\n", display); break; } g_sleep(1000); } - if (!dpy) - { - printf("Unable to open display %s\n", display); - exit(1); - } + return dpy; +} + +/*****************************************************************************/ +/** + * Wait for the RandR extension (if in use) to be available + * + * @param dpy Display + * @return 0 if/when outputs are available, 1 otherwise + */ +static int +wait_for_r_and_r(Display *dpy) +{ + int error_base = 0; + int event_base = 0; + unsigned int outputs = 0; + unsigned int wait = ATTEMPTS; + unsigned int n; + + XRRScreenResources *res = NULL; if (!XRRQueryExtension(dpy, &event_base, &error_base)) { - printf("RandR not supported on display %s", display); + printf("RandR not supported on display %s\n", + DisplayString(dpy)); + return 0; } - else + + for (n = 1; n <= wait; ++n) { - for (n = 1; n <= wait; ++n) + printf("Waiting for outputs. Attempt %u of %u\n", n, wait); + res = XRRGetScreenResources(dpy, DefaultRootWindow(dpy)); + if (res != NULL) { - res = XRRGetScreenResources(dpy, DefaultRootWindow(dpy)); - printf("Waiting for outputs. Attempt %d of %d\n", n, wait); - if (res != NULL) + if (res->noutput > 0) { - if (res->noutput > 0) - { - outputs = res->noutput; - XRRFreeScreenResources(res); - printf("Found %d output[s]\n", outputs); - break; - } - XRRFreeScreenResources(res); + outputs = res->noutput; } - g_sleep(1000); + XRRFreeScreenResources(res); } if (outputs > 0) { - printf("display %s ready with %d outputs\n", display, res->noutput); + printf("Display %s ready with %u RandR outputs\n", + DisplayString(dpy), outputs); + return 0; } - else + g_sleep(1000); + } + + printf("Unable to find any RandR outputs\n"); + return 1; +} + +/*****************************************************************************/ +static void +usage(const char *argv0, int status) +{ + printf("Usage: %s -d display\n", argv0); + exit(status); +} + +/*****************************************************************************/ +int +main(int argc, char **argv) +{ + const char *display_name = NULL; + int opt; + int status = XW_STATUS_MISC_ERROR; + Display *dpy = NULL; + + /* Disable stdout buffering so any messages are passed immediately + * to sesman */ + setvbuf(stdout, NULL, _IONBF, 0); + + while ((opt = getopt(argc, argv, "d:")) != -1) + { + switch (opt) { - printf("Unable to find any outputs\n"); - exit(1); + case 'd': + display_name = optarg; + break; + default: /* '?' */ + usage(argv[0], status); } } - exit(0); + if (!display_name) + { + usage(argv[0], status); + } + + g_set_alarm(alarm_handler, ALARM_WAIT); + + dpy = open_display(display_name); + if (!dpy) + { + printf("Unable to open display %s\n", display_name); + status = XW_STATUS_FAILED_TO_START; + } + else + { + if (wait_for_r_and_r(dpy) == 0) + { + status = XW_STATUS_OK; + } + XCloseDisplay(dpy); + } + + return status; }