Fix error in FindScreenOfXY

[somiaj]

In the previous commit, I defined the m_min variable both inside and outside of an if block (forgot to delete the inside one). So delete the extra definition. Also set m_min to the old return of RB_MIN just in case (this also suppresses warnings from my compiler).

[ThomasAdam]

With this change, can we ever get into a situation where m_min could be NULL? Won't we always find a monitor, regardless?

In which case, I'd probably set m_min to NULL initially and make it a fatal error if it's still NULL before we return from the function.

[somiaj]

No, there shouldn't be any case that m_min is null, since d_min = INT_MAX, unless someone has a setup with so many monitors such that you can give a point that is always greater than INT_MAX away from all monitor edges. I just set that to suppress a warning as my compiler though it might be unused. I'll set to NULL.

[somiaj]

By fatal error, do you just mean return the NULL and let things break? That is what I assumed.

[somiaj]

I guess since integers will overflow and become negative, the only way m_min can be NULL is if the distance to all monitors is equal to INT_MAX, which I would say is not possible.

[somiaj]

@ThomasAdam One more thing, I just took this from the previous code, but if (m == NULL), the only way we got here is if m was null, so could this check also just be taken away?

[ThomasAdam]

Perhaps something like this:

diff --git a/libs/FScreen.c b/libs/FScreen.c
index 6cc1693d6..48139288b 100644
--- a/libs/FScreen.c
+++ b/libs/FScreen.c
@@ -800,7 +800,8 @@ monitor_get_count(void)
 struct monitor *
 FindScreenOfXY(int x, int y)
 {
-	struct monitor	*m;
+	struct monitor	*m, *m_min = NULL;
+	int dmin = INT_MAX;
 	int all_widths = monitor_get_all_widths();
 	int all_heights = monitor_get_all_heights();
 
@@ -824,31 +825,30 @@ FindScreenOfXY(int x, int y)
 			return (m);
 	}
 
-	struct monitor *m_min;
-	if (m == NULL) {
-		/* No monitor found, point is in a dead area.
-		 * Find closest monitor using the taxi cab metric.
-		 */
-		int dmin = INT_MAX;
-		struct monitor *m_min;
-
-		RB_FOREACH(m, monitors, &monitor_q) {
-			int d = 0;
-			if (x < m->si->x)
-				d += m->si->x  - x;
-			if (x > m->si->x + m->si->w)
-				d += x - m->si->x - m->si->w;
-			if (y < m->si->y)
-				d += m->si->y - y;
-			if (y > m->si->y + m->si->h)
-				d += y - m->si->y - m->si->h;
-			if (d < dmin) {
-				dmin = d;
-				m_min = m;
-			}
+	/* No monitor found, point is in a dead area.
+	 * Find closest monitor using the taxi cab metric.
+	 */
+	RB_FOREACH(m, monitors, &monitor_q) {
+		int d = 0;
+		if (x < m->si->x)
+			d += m->si->x  - x;
+		if (x > m->si->x + m->si->w)
+			d += x - m->si->x - m->si->w;
+		if (y < m->si->y)
+			d += m->si->y - y;
+		if (y > m->si->y + m->si->h)
+			d += y - m->si->y - m->si->h;
+		if (d < dmin) {
+			dmin = d;
+			m_min = m;
 		}
 	}
 
+	/* Shouldn't happen. */
+	if (m_min == NULL) {
+		fvwm_debug(__func__, "Couldn't find any monitor");
+		exit(106);
+	}
 	return (m_min);
 }
 
@@ -1433,4 +1433,4 @@ int FScreenFetchMangledScreenFromUSPosHints(XSizeHints *hints)
 		screen = 0;
 
 	return screen;
-}
+}

Note that this isn't on top of this PR, but you get the idea...

[somiaj]

I also noticed I need to skip the global monitor. Note I didn't also test that monitor_get_count() > 0, is this something that could happen? I assumed if we have only a single monitor, that equals the global monitor, and that should match every XY position provided, and thus we wouldn't get to this step.

[ThomasAdam]

Right... you should just be able to do:

if (m_min == monitor_global) continue;

You're right though -- no need to check monitor_get_count() in this instance.

[ThomasAdam]

I can't see anything else which needs changing. Will merge...