From 33b68ec7cb179d739765e8bd750e8b291b286e8a Mon Sep 17 00:00:00 2001
From: patrick96 <p.ziegler96@gmail.com>
Date: Tue, 25 Jun 2019 00:23:44 +0200
Subject: [PATCH] fix(randr): Undefined behavior when removing clones

Because of how monitors are removed inside the loop and depending on the
monitor order a cloned monitor may be assigned a width of 0 but is never
actually removed resulting in polybar saying the bar is out of bounds

Fixes #1794
---
 include/x11/extensions/randr.hpp |  2 +
 src/x11/extensions/randr.cpp     | 63 +++++++++++++++++---------------
 2 files changed, 36 insertions(+), 29 deletions(-)

diff --git a/include/x11/extensions/randr.hpp b/include/x11/extensions/randr.hpp
index 5a42b703..b53f395e 100644
--- a/include/x11/extensions/randr.hpp
+++ b/include/x11/extensions/randr.hpp
@@ -40,6 +40,8 @@ struct randr_output {
 
   bool match(const string& o, bool exact = true) const;
   bool match(const position& p) const;
+
+  bool contains(const randr_output& output) const;
 };
 
 using monitor_t = shared_ptr<randr_output>;
diff --git a/src/x11/extensions/randr.cpp b/src/x11/extensions/randr.cpp
index 64bde0b9..9127b4e5 100644
--- a/src/x11/extensions/randr.cpp
+++ b/src/x11/extensions/randr.cpp
@@ -32,6 +32,16 @@ bool randr_output::match(const position& p) const {
   return p.x == x && p.y == y;
 }
 
+/**
+ * Checks if inner is contained withing this.
+ *
+ * Also returns true for outputs that occupy the exact same space
+ */
+bool randr_output::contains(const randr_output& inner) const {
+  return inner.x >= x && inner.x + inner.w <= x + w
+    && inner.y >= y && inner.y + inner.h <= y + h;
+}
+
 namespace randr_util {
   /**
    * XRandR version
@@ -135,40 +145,35 @@ namespace randr_util {
       }
     }
 
-    // clang-format off
-    const auto remove_monitor = [&](const monitor_t& monitor) {
-      monitors.erase(find(monitors.begin(), monitors.end(), monitor));
-    };
-    // clang-format on
-
-    for (auto m = monitors.rbegin(); m != monitors.rend(); m++) {
-      if ((*m)->w == 0) {
-        remove_monitor(*m);
-        continue;
-      }
-
-      // Test if there are any clones in the set
-
-      if (!purge_clones) {
-        continue;
-      }
-
-      for (auto& monitor : monitors) {
-        if ((*m) == monitor || monitor->w == 0) {
-          continue;
-        } else if (check_monitor_support() && (monitor->output == XCB_NONE || (*m)->output == XCB_NONE)) {
+    if(purge_clones) {
+      for (auto& outer : monitors) {
+        if (outer->w == 0) {
           continue;
         }
 
-        // clang-format off
-        if (monitor->x >= (*m)->x && monitor->x + monitor->w <= (*m)->x + (*m)->w &&
-            monitor->y >= (*m)->y && monitor->y + monitor->h <= (*m)->y + (*m)->h) {
-          // Reset width so that the output gets
-          // removed in the base loop
-          monitor->w = 0;
+        for (auto& inner : monitors) {
+          if (outer == inner || inner->w == 0) {
+            continue;
+          } else if (check_monitor_support() && (inner->output == XCB_NONE || outer->output == XCB_NONE)) {
+            continue;
+          }
+
+          // If inner is contained in outer, inner is removed
+          // If both happen to be the same size and have the same coordinates,
+          // inner is still removed but it doesn't matter since both occupy the
+          // exact same space
+          if (outer->contains(*inner)) {
+            // Reset width so that the output gets removed afterwards
+            inner->w = 0;
+          }
         }
-        // clang-format on
       }
+
+      // Remove all cloned monitors (monitors with 0 width)
+      monitors.erase(
+          std::remove_if(monitors.begin(), monitors.end(),
+            [](const auto & monitor) { return monitor->w == 0; }),
+          monitors.end());
     }
 
     sort(monitors.begin(), monitors.end(), [](monitor_t& m1, monitor_t& m2) -> bool {