On Sat, Nov 27, 2021 at 07:02:29PM +0100, Dominik Vogt wrote: > There are some NULL pointer crashes and bugs telated to > moitor_resove_ame():
Attempt to fix these, please proof read the patch. * Parsing fixes. * monitor_resolve_name() does not crash if scr == NULL but returns NULL. * Callers deal with NULL beng returned. * FScreenGetScrRect() uses the global screen f the screen is not found. * Export monitor_global. (A functions seems to be overkill.) * Some other minor related cleanup. Doesn't crash, but I can't test it with a single monitor. One thing to double check if whether callers should use the global, primary or current screen if monitor_resolve_name() returns NULL. My guesses may not be all correct. Ciao Dominik ^_^ ^_^ -- Dominik Vogt
From 288bfd7f95a87bdeb5783d5d4a7afcd1926680d0 Mon Sep 17 00:00:00 2001 From: Dominik Vogt <dominik.v...@gmx.de> Date: Sat, 27 Nov 2021 18:01:29 +0100 Subject: [PATCH] Fix monitor parsing. --- fvwm/ewmh_conf.c | 9 +-- fvwm/expand.c | 4 +- fvwm/move_resize.c | 14 ++--- fvwm/placement.c | 8 ++- fvwm/virtual.c | 100 ++++++++++++++----------------- libs/FScreen.c | 37 ++++++------ libs/FScreen.h | 1 + modules/FvwmIconMan/readconfig.c | 4 ++ 8 files changed, 87 insertions(+), 90 deletions(-) diff --git a/fvwm/ewmh_conf.c b/fvwm/ewmh_conf.c index cb2bb126..e94604f0 100644 --- a/fvwm/ewmh_conf.c +++ b/fvwm/ewmh_conf.c @@ -110,8 +110,7 @@ void CMD_EwmhNumberOfDesktops(F_CMD_ARGS) option = PeekToken(action, &action); if ((m = monitor_resolve_name(option)) == NULL) { - fvwm_debug(__func__, - "Invalid screen: %s", option); + fvwm_debug(__func__, "Invalid screen: %s", option); } } @@ -150,10 +149,8 @@ void CMD_EwmhBaseStruts(F_CMD_ARGS) /* Actually get the screen value. */ option = PeekToken(action, &action); - m = monitor_resolve_name(option); - if (strcmp(m->si->name, option) != 0) { - fvwm_debug(__func__, - "Invalid screen: %s", option); + if ((m = monitor_resolve_name(option)) == NULL) { + fvwm_debug(__func__, "Invalid screen: %s", option); return; } } diff --git a/fvwm/expand.c b/fvwm/expand.c index 44115ade..7aee8790 100644 --- a/fvwm/expand.c +++ b/fvwm/expand.c @@ -538,9 +538,7 @@ static signed int expand_vars_extended( rest_s = fxstrdup(rest); while ((m_name = strsep(&rest_s, ".")) != NULL) { mon2 = monitor_resolve_name(m_name); - if (m_name == NULL) - return -1; - if (strcmp(mon2->si->name, m_name) == 1) + if (mon2 == NULL) return -1; /* Skip over the monitor name. */ diff --git a/fvwm/move_resize.c b/fvwm/move_resize.c index cd070fa8..47fd322e 100644 --- a/fvwm/move_resize.c +++ b/fvwm/move_resize.c @@ -2071,11 +2071,15 @@ static void __move_window(F_CMD_ARGS, Bool do_animate, int mode) rectangle r; rectangle s; rectangle t; - struct monitor *m = monitor_get_current(); + struct monitor *m; char *token; - if (action != NULL && (token = PeekToken(action, &action)) != NULL) - m = monitor_resolve_name(token); + token = PeekToken(action, &action); + m = monitor_resolve_name(token); + if (m == NULL) + { + m = monitor_get_current(); + } s.x = m->si->x; s.y = m->si->y; @@ -3301,10 +3305,6 @@ void CMD_GeometryWindow(F_CMD_ARGS) if (token != NULL) { Scr.SizeWindow.m = monitor_resolve_name(token); - if (strcasecmp(Scr.SizeWindow.m->si->name, token) != 0) { - /* Incorrect RandR screen found. */ - Scr.SizeWindow.m = NULL; - } } } } diff --git a/fvwm/placement.c b/fvwm/placement.c index 9d081d98..0169730c 100644 --- a/fvwm/placement.c +++ b/fvwm/placement.c @@ -1732,6 +1732,8 @@ static int __place_window( if (flags.do_honor_starts_on_screen) { fscreen_scr_arg arg; + struct monitor *m; + arg.mouse_ev = NULL; /* FIXME: expand the screen name here. It's possible @@ -1762,8 +1764,12 @@ static int __place_window( * "_global" screen, which is a faked monitor for the * purposes of an older API. */ + m = NULL; if (strcmp(arg.name, "g") != 0) - fw->m = monitor_resolve_name(arg.name); + m = monitor_resolve_name(arg.name); + if (m == NULL) + m = monitor_get_current(); + fw->m = m; free(e); } else diff --git a/fvwm/virtual.c b/fvwm/virtual.c index dacb39cb..8cbcc712 100644 --- a/fvwm/virtual.c +++ b/fvwm/virtual.c @@ -1783,41 +1783,38 @@ void do_move_window_to_desk(FvwmWindow *fw, int desk) return; } -Bool get_page_arguments(FvwmWindow *fw, char *action, int *page_x, int *page_y, struct monitor **mret) +Bool get_page_arguments( + FvwmWindow *fw, char *action, int *page_x, int *page_y, + struct monitor **mret) { int val[2]; int suffix[2]; int mw, mh; char *token; - char *taction, *action_cpy, *action_cpy_start; + char *taction; + char *next; int wrapx; int wrapy; int limitdeskx; int limitdesky; - struct monitor *m, *m_use; + struct monitor *m; wrapx = 0; wrapy = 0; limitdeskx = 1; limitdesky = 1; - m_use = (fw && fw->m) ? fw->m : monitor_get_current(); - action_cpy = strdup(action); - action_cpy_start = action_cpy; - token = PeekToken(action_cpy, &action_cpy); - free(action_cpy_start); - - if (token == NULL) - return (False); - + token = PeekToken(action, &next); m = monitor_resolve_name(token); - if (strcmp(m->si->name, token) != 0) - m = m_use; + if (m != NULL) + { + action = next; + } else - PeekToken(action, &action); - - if (mret != NULL) - *mret = m; + { + m = (fw && fw->m) ? fw->m : monitor_get_current(); + } + *mret = m; mw = monitor_get_all_widths(); mh = monitor_get_all_heights(); @@ -1872,7 +1869,7 @@ Bool get_page_arguments(FvwmWindow *fw, char *action, int *page_x, int *page_y, if (GetSuffixedIntegerArguments(action, NULL, val, 2, "pw", suffix) != 2) { - return 0; + return False; } if (suffix[0] == 1) @@ -2000,7 +1997,8 @@ void parse_edge_leave_command(char *action, int type) option = PeekToken(action, &action); m = monitor_resolve_name(option); - if (strcmp(m->si->name, option) != 0) { + if (m == NULL) + { fvwm_debug(__func__, "Invalid screen: %s", option); return; @@ -2460,19 +2458,20 @@ void CMD_DesktopSize(F_CMD_ARGS) */ void CMD_GotoDesk(F_CMD_ARGS) { - struct monitor *m_use = monitor_get_current(), *m, *m_loop; - char *action_cpy, *action_cpy_start, *token; + struct monitor *m, *m_loop; + char *next, *token; int new_desk; - action_cpy = strdup(action); - action_cpy_start = action_cpy; - token = PeekToken(action_cpy, &action_cpy); - + token = PeekToken(action, &next); m = monitor_resolve_name(token); - if (strcmp(m->si->name, token) != 0) - m = m_use; + if (m != NULL) + { + action = next; + } else - PeekToken(action, &action); + { + m = monitor_get_current(); + } new_desk = GetDeskNumber(m, action, m->virtual_scr.CurrentDesk); @@ -2503,14 +2502,11 @@ void CMD_GotoDesk(F_CMD_ARGS) m_loop->virtual_scr.is_swapping = false; m->virtual_scr.is_swapping = false; - goto end; + return; } } } goto_desk(new_desk, m); -end: - free(action_cpy_start); - return; } void CMD_Desk(F_CMD_ARGS) @@ -2533,22 +2529,20 @@ void CMD_GotoDeskAndPage(F_CMD_ARGS) { int val[3]; Bool is_new_desk; - char *action_cpy, *action_cpy_start; + char *next; char *token; - struct monitor *m_use = monitor_get_current(), *m; - - action_cpy = strdup(action); - action_cpy_start = action_cpy; - token = PeekToken(action_cpy, &action_cpy); + struct monitor *m; + token = PeekToken(action, &next); m = monitor_resolve_name(token); - if (strcmp(m->si->name, token) != 0) - m = m_use; + if (m != NULL) + { + action = next; + } else - PeekToken(action, &action); - - free(action_cpy_start); - + { + m = monitor_get_current(); + } /* FIXME: monitor needs broadcast when global. */ if (MatchToken(action, "prev")) @@ -2696,20 +2690,18 @@ void CMD_Scroll(F_CMD_ARGS) int x,y; int val1, val2, val1_unit, val2_unit; char *option; + char *next; struct monitor *m = monitor_get_current(); - option = PeekToken(action, NULL); + option = PeekToken(action, &next); if (StrEquals(option, "screen")) { - /* Skip literal 'screen' */ - option = PeekToken(action, &action); /* Actually get the screen value. */ - option = PeekToken(action, &action); - + option = PeekToken(next, &action); m = monitor_resolve_name(option); - if (strcmp(m->si->name, option) != 0) { - fvwm_debug(__func__, - "Invalid screen: %s", option); - return; + if (m == NULL) + { + fvwm_debug(__func__, "Invalid screen: %s", option); + return; } } diff --git a/libs/FScreen.c b/libs/FScreen.c index 8ad08e86..692cbd04 100644 --- a/libs/FScreen.c +++ b/libs/FScreen.c @@ -57,7 +57,7 @@ struct screen_infos screen_info_q; struct monitors monitor_q; int randr_event; const char *prev_focused_monitor; -static struct monitor *monitor_global; +struct monitor *monitor_global = NULL; static void GetMouseXY(XEvent *eventp, int *x, int *y) { @@ -186,6 +186,10 @@ monitor_resolve_name(const char *scr) { struct monitor *m = NULL; + if (scr == NULL) + { + return NULL; + } /* Assume the monitor name is a literal RandR name (such as HDMI2) and * look it up regardless. */ @@ -207,11 +211,8 @@ monitor_resolve_name(const char *scr) if (strcmp(scr, "p") == 0) m = monitor_by_primary(); - if (m == NULL) { - /* Should not happen. */ + if (m == NULL) fvwm_debug(__func__, "no monitor found with name '%s'", scr); - return (TAILQ_FIRST(&monitor_q)); - } return (m); } @@ -762,8 +763,8 @@ Bool FScreenGetScrRect(fscreen_scr_arg *arg, fscreen_scr_t screen, { struct monitor *m = FindScreen(arg, screen); if (m == NULL) { - fvwm_debug(__func__, "%s: m is NULL\n", __func__); - return (True); + fvwm_debug(__func__, "m is NULL, using global screen\n"); + m = monitor_global; } if (x) @@ -775,8 +776,7 @@ Bool FScreenGetScrRect(fscreen_scr_arg *arg, fscreen_scr_t screen, if (h) *h = m->si->h; - return !((monitor_get_count() > 1) && - (strcmp(m->si->name, GLOBAL_SCREEN_NAME) == 0)); + return !((monitor_get_count() > 1) && m == monitor_global); } /* Translates the coodinates *x *y from the screen specified by arg_src and @@ -995,17 +995,16 @@ int FScreenParseGeometry( parsestring, x_return, y_return, width_return, height_return, &scr); - if (scr != NULL) { - m = monitor_resolve_name(scr); - fprintf( - stderr, - "Found monitor with name of: %s (%s)\n", scr, - m->si->name); - x = m->si->x; - y = m->si->y; - w = m->si->w; - h = m->si->h; + m = monitor_resolve_name(scr); + if (m == NULL) + { + /* fall back to current screen */ + m = monitor_get_current(); } + x = m->si->x; + y = m->si->y; + w = m->si->w; + h = m->si->h; } /* adapt geometry to selected screen */ diff --git a/libs/FScreen.h b/libs/FScreen.h index 2aa16163..66559a29 100644 --- a/libs/FScreen.h +++ b/libs/FScreen.h @@ -141,6 +141,7 @@ struct monitor { TAILQ_HEAD(monitors, monitor); extern struct monitors monitor_q; +extern struct monitor *monitor_global; struct monitor *monitor_resolve_name(const char *); struct monitor *monitor_by_xy(int, int); diff --git a/modules/FvwmIconMan/readconfig.c b/modules/FvwmIconMan/readconfig.c index d593f0f4..385684cd 100644 --- a/modules/FvwmIconMan/readconfig.c +++ b/modules/FvwmIconMan/readconfig.c @@ -1203,6 +1203,10 @@ static void handle_resolution_config(int man, char *line) break; } struct monitor *mon = monitor_resolve_name(token); + if (mon == NULL) + { + mon = monitor_global; + } SET_MANAGER(man, scr, (char *)mon->si->name); line = nline; break; -- 2.30.2