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
This commit is contained in:
patrick96 2019-06-25 00:23:44 +02:00 committed by Patrick Ziegler
parent 9f7363c9ee
commit 33b68ec7cb
2 changed files with 36 additions and 29 deletions

View File

@ -40,6 +40,8 @@ struct randr_output {
bool match(const string& o, bool exact = true) const; bool match(const string& o, bool exact = true) const;
bool match(const position& p) const; bool match(const position& p) const;
bool contains(const randr_output& output) const;
}; };
using monitor_t = shared_ptr<randr_output>; using monitor_t = shared_ptr<randr_output>;

View File

@ -32,6 +32,16 @@ bool randr_output::match(const position& p) const {
return p.x == x && p.y == y; 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 { namespace randr_util {
/** /**
* XRandR version * XRandR version
@ -135,40 +145,35 @@ namespace randr_util {
} }
} }
// clang-format off if(purge_clones) {
const auto remove_monitor = [&](const monitor_t& monitor) { for (auto& outer : monitors) {
monitors.erase(find(monitors.begin(), monitors.end(), monitor)); if (outer->w == 0) {
};
// clang-format on
for (auto m = monitors.rbegin(); m != monitors.rend(); m++) {
if ((*m)->w == 0) {
remove_monitor(*m);
continue; continue;
} }
// Test if there are any clones in the set for (auto& inner : monitors) {
if (outer == inner || inner->w == 0) {
if (!purge_clones) { continue;
} else if (check_monitor_support() && (inner->output == XCB_NONE || outer->output == XCB_NONE)) {
continue; continue;
} }
for (auto& monitor : monitors) { // If inner is contained in outer, inner is removed
if ((*m) == monitor || monitor->w == 0) { // If both happen to be the same size and have the same coordinates,
continue; // inner is still removed but it doesn't matter since both occupy the
} else if (check_monitor_support() && (monitor->output == XCB_NONE || (*m)->output == XCB_NONE)) { // exact same space
continue; if (outer->contains(*inner)) {
// Reset width so that the output gets removed afterwards
inner->w = 0;
}
}
} }
// clang-format off // Remove all cloned monitors (monitors with 0 width)
if (monitor->x >= (*m)->x && monitor->x + monitor->w <= (*m)->x + (*m)->w && monitors.erase(
monitor->y >= (*m)->y && monitor->y + monitor->h <= (*m)->y + (*m)->h) { std::remove_if(monitors.begin(), monitors.end(),
// Reset width so that the output gets [](const auto & monitor) { return monitor->w == 0; }),
// removed in the base loop monitors.end());
monitor->w = 0;
}
// clang-format on
}
} }
sort(monitors.begin(), monitors.end(), [](monitor_t& m1, monitor_t& m2) -> bool { sort(monitors.begin(), monitors.end(), [](monitor_t& m1, monitor_t& m2) -> bool {