Re: [PATCH] Re: Crashes with monitor_resolve_name

2021-11-28 Thread Dominik Vogt
Updated patch in dv/monitor-parsing-fix.

Doesn't take care of the NULL m->Desktops crash.  Don't know how
to reproduce.

Ciao

Dominik ^_^  ^_^

--

Dominik Vogt



Re: [PATCH] Re: Crashes with monitor_resolve_name

2021-11-27 Thread Dominik Vogt
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 == 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 != x ||
m->Desktops->ewmh_working_area.y != y ||
m->Desktops->ewmh_working_area.width != width ||
m->Desktops->ewmh_working_area.height != 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  0x55ef2959458c in ewmh_ComputeAndSetWorkArea (m=m@entry=0x55ef2b101a20)
at ewmh.c:1048

1048if (
(gdb) bt
#0  0x55ef2959458c in ewmh_ComputeAndSetWorkArea (m=m@entry=0x55ef2b101a20)
at ewmh.c:1048
#1  0x55ef29594ebe in EWMH_WindowDestroyed () at ewmh.c:1863
#2  0x55ef29565137 in HandleUnmapNotify (ea=)
at events.c:3903
#3  0x55ef29563d3e in dispatch_event (e=e@entry=0x7ffef344f560)
at events.c:4185
#4  0x55ef29564b0a in HandleEvents () at events.c:4231
#5  0x55ef2958304f in main (argc=, argv=)
at fvwm3.c:2547

Don't know how I triggered that.  I was testing
GotoDesk/Page/DeskAndPage with that console.

Ciao

Dominik ^_^  ^_^

--

Dominik Vogt
From 7e8d87d872b571c0b7d5c1fbc024c4a8339cf1e7 Mon Sep 17 00:00:00 2001
From: Dominik Vogt 
Date: Sat, 27 Nov 2021 18:01:29 +0100
Subject: [PATCH] Fix monitor parsing II.

---
 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
--- a/fvwm/ewmh_conf.c
+++ b/fvwm/ewmh_conf.c
@@ -110,8 +110,7 @@ void CMD_EwmhNumberOfDesktops(F_CMD_ARGS)
 		option = PeekToken(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, );

-		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(_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, )) != NULL)
-			m = monitor_resolve_name(token);
+		token = PeekToken(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
---