Re: [PATCH] Re: Crashes with monitor_resolve_name
Dominik Vogt <[email protected]> Sat, 27 Nov 2021 21:14:46 +0100
| Newsgroups | gmane.comp.window-managers.fvwm.devel |
|---|---|
| Message-ID | <[email protected]> |
--XvM4VYnzavdufOfn
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Sat, Nov 27, 2021 at 08:30:52PM +0100, Dominik Vogt wrote:
> 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 =3D=3D 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.
Updated patch attached.
During testing I got a crash in ewmh.c:1049 because m->Desktops of
the primary screen is NULL:
if (
m->Desktops->ewmh_working_area.x !=3D x ||
m->Desktops->ewmh_working_area.y !=3D y ||
m->Desktops->ewmh_working_area.width !=3D width ||
m->Desktops->ewmh_working_area.height !=3D height)
{
I think that's unrelated to the patch. It was triggered by
destroying an FvwmConsole window.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x000055ef2959458c in ewmh_ComputeAndSetWorkArea (m=3Dm@entry=3D0x55ef=
2b101a20)
at ewmh.c:1048
1048 if (
(gdb) bt
#0 0x000055ef2959458c in ewmh_ComputeAndSetWorkArea (m=3Dm@entry=3D0x55ef=
2b101a20)
at ewmh.c:1048
#1 0x000055ef29594ebe in EWMH_WindowDestroyed () at ewmh.c:1863
#2 0x000055ef29565137 in HandleUnmapNotify (ea=3D<optimized out>)
at events.c:3903
#3 0x000055ef29563d3e in dispatch_event (e=3De@entry=3D0x7ffef344f560)
at events.c:4185
#4 0x000055ef29564b0a in HandleEvents () at events.c:4231
#5 0x000055ef2958304f in main (argc=3D<optimized out>, argv=3D<optimized =
out>)
at fvwm3.c:2547
Don't know how I triggered that. I was testing
GotoDesk/Page/DeskAndPage with that console.
Ciao
Dominik ^_^ ^_^
=2D-
Dominik Vogt
--XvM4VYnzavdufOfn
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0001-Fix-monitor-parsing-II.patch"
Content-Transfer-Encoding: quoted-printable
=46rom 7e8d87d872b571c0b7d5c1fbc024c4a8339cf1e7 Mon Sep 17 00:00:00 2001
From: Dominik Vogt <[email protected]>
Date: Sat, 27 Nov 2021 18:01:29 +0100
Subject: [PATCH] Fix monitor parsing II.
=2D--
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 | 83 ++++++++-----------------
libs/FScreen.h | 1 +
modules/FvwmIconMan/readconfig.c | 4 ++
8 files changed, 95 insertions(+), 128 deletions(-)
diff --git a/fvwm/ewmh_conf.c b/fvwm/ewmh_conf.c
index cb2bb126..e94604f0 100644
=2D-- a/fvwm/ewmh_conf.c
+++ b/fvwm/ewmh_conf.c
@@ -110,8 +110,7 @@ void CMD_EwmhNumberOfDesktops(F_CMD_ARGS)
option =3D PeekToken(action, &action);
if ((m =3D monitor_resolve_name(option)) =3D=3D 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 =3D PeekToken(action, &action);
- m =3D monitor_resolve_name(option);
- if (strcmp(m->si->name, option) !=3D 0) {
- fvwm_debug(__func__,
- "Invalid screen: %s", option);
+ if ((m =3D monitor_resolve_name(option)) =3D=3D NULL) {
+ fvwm_debug(__func__, "Invalid screen: %s", option);
return;
}
}
diff --git a/fvwm/expand.c b/fvwm/expand.c
index 44115ade..7aee8790 100644
=2D-- a/fvwm/expand.c
+++ b/fvwm/expand.c
@@ -538,9 +538,7 @@ static signed int expand_vars_extended(
rest_s =3D fxstrdup(rest);
while ((m_name =3D strsep(&rest_s, ".")) !=3D NULL) {
mon2 =3D monitor_resolve_name(m_name);
- if (m_name =3D=3D NULL)
- return -1;
- if (strcmp(mon2->si->name, m_name) =3D=3D 1)
+ if (mon2 =3D=3D NULL)
return -1;
/* Skip over the monitor name. */
diff --git a/fvwm/move_resize.c b/fvwm/move_resize.c
index cd070fa8..47fd322e 100644
=2D-- a/fvwm/move_resize.c
+++ b/fvwm/move_resize.c
@@ -2071,11 +2071,15 @@ static void __move_window(F_CMD_ARGS, Bool do_anim=
ate, int mode)
rectangle r;
rectangle s;
rectangle t;
- struct monitor *m =3D monitor_get_current();
+ struct monitor *m;
char *token;
- if (action !=3D NULL && (token =3D PeekToken(action, &action)) !=3D NUL=
L)
- m =3D monitor_resolve_name(token);
+ token =3D PeekToken(action, &action);
+ m =3D monitor_resolve_name(token);
+ if (m =3D=3D NULL)
+ {
+ m =3D monitor_get_current();
+ }
s.x =3D m->si->x;
s.y =3D m->si->y;
@@ -3301,10 +3305,6 @@ void CMD_GeometryWindow(F_CMD_ARGS)
if (token !=3D NULL)
{
Scr.SizeWindow.m =3D monitor_resolve_name(token);
- if (strcasecmp(Scr.SizeWindow.m->si->name, token) !=3D 0) {
- /* Incorrect RandR screen found. */
- Scr.SizeWindow.m =3D NULL;
- }
}
}
}
diff --git a/fvwm/placement.c b/fvwm/placement.c
index 9d081d98..0169730c 100644
=2D-- 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 =3D 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 =3D NULL;
if (strcmp(arg.name, "g") !=3D 0)
- fw->m =3D monitor_resolve_name(arg.name);
+ m =3D monitor_resolve_name(arg.name);
+ if (m =3D=3D NULL)
+ m =3D monitor_get_current();
+ fw->m =3D m;
free(e);
}
else
diff --git a/fvwm/virtual.c b/fvwm/virtual.c
index dacb39cb..8cbcc712 100644
=2D-- a/fvwm/virtual.c
+++ b/fvwm/virtual.c
@@ -1783,41 +1783,38 @@ void do_move_window_to_desk(FvwmWindow *fw, int de=
sk)
return;
}
-Bool get_page_arguments(FvwmWindow *fw, char *action, int *page_x, int *p=
age_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 =3D 0;
wrapy =3D 0;
limitdeskx =3D 1;
limitdesky =3D 1;
- m_use =3D (fw && fw->m) ? fw->m : monitor_get_current();
- action_cpy =3D strdup(action);
- action_cpy_start =3D action_cpy;
- token =3D PeekToken(action_cpy, &action_cpy);
- free(action_cpy_start);
-
- if (token =3D=3D NULL)
- return (False);
-
+ token =3D PeekToken(action, &next);
m =3D monitor_resolve_name(token);
- if (strcmp(m->si->name, token) !=3D 0)
- m =3D m_use;
+ if (m !=3D NULL)
+ {
+ action =3D next;
+ }
else
- PeekToken(action, &action);
-
- if (mret !=3D NULL)
- *mret =3D m;
+ {
+ m =3D (fw && fw->m) ? fw->m : monitor_get_current();
+ }
+ *mret =3D m;
mw =3D monitor_get_all_widths();
mh =3D 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) !=3D
2)
{
- return 0;
+ return False;
}
if (suffix[0] =3D=3D 1)
@@ -2000,7 +1997,8 @@ void parse_edge_leave_command(char *action, int type=
)
option =3D PeekToken(action, &action);
m =3D monitor_resolve_name(option);
- if (strcmp(m->si->name, option) !=3D 0) {
+ if (m =3D=3D 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 =3D 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 =3D strdup(action);
- action_cpy_start =3D action_cpy;
- token =3D PeekToken(action_cpy, &action_cpy);
-
+ token =3D PeekToken(action, &next);
m =3D monitor_resolve_name(token);
- if (strcmp(m->si->name, token) !=3D 0)
- m =3D m_use;
+ if (m !=3D NULL)
+ {
+ action =3D next;
+ }
else
- PeekToken(action, &action);
+ {
+ m =3D monitor_get_current();
+ }
new_desk =3D GetDeskNumber(m, action, m->virtual_scr.CurrentDesk);
@@ -2503,14 +2502,11 @@ void CMD_GotoDesk(F_CMD_ARGS)
m_loop->virtual_scr.is_swapping =3D false;
m->virtual_scr.is_swapping =3D 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 =3D monitor_get_current(), *m;
-
- action_cpy =3D strdup(action);
- action_cpy_start =3D action_cpy;
- token =3D PeekToken(action_cpy, &action_cpy);
+ struct monitor *m;
+ token =3D PeekToken(action, &next);
m =3D monitor_resolve_name(token);
- if (strcmp(m->si->name, token) !=3D 0)
- m =3D m_use;
+ if (m !=3D NULL)
+ {
+ action =3D next;
+ }
else
- PeekToken(action, &action);
-
- free(action_cpy_start);
-
+ {
+ m =3D 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 =3D monitor_get_current();
- option =3D PeekToken(action, NULL);
+ option =3D PeekToken(action, &next);
if (StrEquals(option, "screen")) {
- /* Skip literal 'screen' */
- option =3D PeekToken(action, &action);
/* Actually get the screen value. */
- option =3D PeekToken(action, &action);
-
+ option =3D PeekToken(next, &action);
m =3D monitor_resolve_name(option);
- if (strcmp(m->si->name, option) !=3D 0) {
- fvwm_debug(__func__,
- "Invalid screen: %s", option);
- return;
+ if (m =3D=3D NULL)
+ {
+ fvwm_debug(__func__, "Invalid screen: %s", option);
+ return;
}
}
diff --git a/libs/FScreen.c b/libs/FScreen.c
index 8ad08e86..8adbc1e1 100644
=2D-- 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 =3D NULL;
static void GetMouseXY(XEvent *eventp, int *x, int *y)
{
@@ -186,32 +186,25 @@ monitor_resolve_name(const char *scr)
{
struct monitor *m =3D NULL;
- /* Assume the monitor name is a literal RandR name (such as HDMI2) and
- * look it up regardless.
- */
- m =3D monitor_by_name(scr);
-
- /* If we've asked for "@g" then use the global screen. The
- * x,y,w,h values are already assigned, so skip that.
- */
+ if (scr =3D=3D NULL)
+ {
+ return NULL;
+ }
+ /* "@g" is for the global screen. */
if (strcmp(scr, "g") =3D=3D 0) {
monitor_refresh_global();
m =3D monitor_global;
}
-
/* "@c" is for the current screen. */
- if (strcmp(scr, "c") =3D=3D 0)
+ else if (strcmp(scr, "c") =3D=3D 0)
m =3D monitor_get_current();
-
/* "@p" is for the primary screen. */
- if (strcmp(scr, "p") =3D=3D 0)
+ else if (strcmp(scr, "p") =3D=3D 0)
m =3D monitor_by_primary();
-
- if (m =3D=3D NULL) {
- /* Should not happen. */
- fvwm_debug(__func__, "no monitor found with name '%s'", scr);
- return (TAILQ_FIRST(&monitor_q));
- }
+ else
+ /* Assume the monitor name is a literal RandR name (such as
+ * HDMI2). */
+ m =3D monitor_by_name(scr);
return (m);
}
@@ -219,7 +212,7 @@ monitor_resolve_name(const char *scr)
static struct monitor *
monitor_by_name(const char *name)
{
- struct monitor *m, *mret =3D NULL;
+ struct monitor *m, *mret;
if (name =3D=3D NULL) {
fvwm_debug(__func__, "%s: name is NULL; shouldn't happen. "
@@ -227,6 +220,7 @@ monitor_by_name(const char *name)
return (monitor_get_current());
}
+ mret =3D NULL;
TAILQ_FOREACH(m, &monitor_q, entry) {
if (strcmp(m->si->name, name) =3D=3D 0) {
mret =3D m;
@@ -234,29 +228,6 @@ monitor_by_name(const char *name)
}
}
- if (mret =3D=3D NULL && (strcmp(name, GLOBAL_SCREEN_NAME) =3D=3D 0)) {
- if (monitor_get_count() =3D=3D 1) {
- /* In this case, the global screen was requested, but
- * we've only one monitor in use. Return this monitor
- * instead.
- */
- return (TAILQ_FIRST(&monitor_q));
- } else {
- /* Return the current monitor. */
- mret =3D monitor_get_current();
- }
- }
-
- /* Then we couldn't find the named monitor at all. Return the current
- * monitor instead.
- */
-
- if (mret =3D=3D NULL) {
- mret =3D monitor_get_current();
- if (mret =3D=3D NULL)
- return (NULL);
- }
-
return (mret);
}
@@ -762,8 +733,8 @@ Bool FScreenGetScrRect(fscreen_scr_arg *arg, fscreen_s=
cr_t screen,
{
struct monitor *m =3D FindScreen(arg, screen);
if (m =3D=3D NULL) {
- fvwm_debug(__func__, "%s: m is NULL\n", __func__);
- return (True);
+ fvwm_debug(__func__, "m is NULL, using global screen\n");
+ m =3D monitor_global;
}
if (x)
@@ -775,8 +746,7 @@ Bool FScreenGetScrRect(fscreen_scr_arg *arg, fscreen_s=
cr_t screen,
if (h)
*h =3D m->si->h;
- return !((monitor_get_count() > 1) &&
- (strcmp(m->si->name, GLOBAL_SCREEN_NAME) =3D=3D 0));
+ return !((monitor_get_count() > 1) && m =3D=3D monitor_global);
}
/* Translates the coodinates *x *y from the screen specified by arg_src a=
nd
@@ -995,17 +965,16 @@ int FScreenParseGeometry(
parsestring, x_return, y_return, width_return,
height_return, &scr);
- if (scr !=3D NULL) {
- m =3D monitor_resolve_name(scr);
- fprintf(
- stderr,
- "Found monitor with name of: %s (%s)\n", scr,
- m->si->name);
- x =3D m->si->x;
- y =3D m->si->y;
- w =3D m->si->w;
- h =3D m->si->h;
+ m =3D monitor_resolve_name(scr);
+ if (m =3D=3D NULL)
+ {
+ /* fall back to current screen */
+ m =3D monitor_get_current();
}
+ x =3D m->si->x;
+ y =3D m->si->y;
+ w =3D m->si->w;
+ h =3D m->si->h;
}
/* adapt geometry to selected screen */
diff --git a/libs/FScreen.h b/libs/FScreen.h
index 2aa16163..66559a29 100644
=2D-- 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/readco=
nfig.c
index d593f0f4..385684cd 100644
=2D-- 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 =3D monitor_resolve_name(token);
+ if (mon =3D=3D NULL)
+ {
+ mon =3D monitor_global;
+ }
SET_MANAGER(man, scr, (char *)mon->si->name);
line =3D nline;
break;
=2D-
2.30.2
--XvM4VYnzavdufOfn--