bbidulock / blackboxwm

A window manager for X11
Other
169 stars 28 forks source link

XSizeHints is made of int #44

Closed rikigigi closed 1 year ago

rikigigi commented 2 years ago

The code wrongly assumes that XSizeHints contains unsigned int only, while this is not the case, leading to crashes if negative ints are found there the crashes are like

blackbox: Window.cc:303: bt::Rect constrain(const bt::Rect&, const bt::EWMH::Strut&, const WMNormalHints&, Corner): Assertion `dw >= base_width && dh >= base_height' failed.
zsh: IOT instruction (core dumped)  blackbox

at the moment the patch

diff --git a/src/Window.cc b/src/Window.cc
index 71db507..4cebe3b 100644
--- a/src/Window.cc
+++ b/src/Window.cc
@@ -280,12 +280,12 @@ static bt::Rect constrain(const bt::Rect &rect,
               rect.right() - static_cast<signed>(margin.right),
               rect.bottom() - static_cast<signed>(margin.bottom));

-  unsigned int dw = r.width(), dh = r.height();
+  int dw = r.width(), dh = r.height();

-  const unsigned int base_width = (wmnormal.base_width
+  const int base_width = (wmnormal.base_width>0
                                    ? wmnormal.base_width
                                    : wmnormal.min_width),
-                    base_height = (wmnormal.base_height
+                    base_height = (wmnormal.base_height>0
                                    ? wmnormal.base_height
                                    : wmnormal.min_height);

@@ -299,6 +299,10 @@ static bt::Rect constrain(const bt::Rect &rect,
     dw = wmnormal.max_width;
   if (dh > wmnormal.max_height)
     dh = wmnormal.max_height;
+#ifdef    DEBUG
+    fprintf(stderr, "(dw, dh) = (%i, %i)\nbw=%i bh=%i\nbase_width\tmin_width\tbase_height\tmin_height\n %i\t%i\t%i\t%i\n",
+            dw, dh, base_width, base_height,wmnormal.base_width, wmnormal.min_width, wmnormal.base_height, wmnormal.min_height);
+#endif // DEBUG

   assert(dw >= base_width && dh >= base_height);

is the fastest possible correction in the case

(dw, dh) = (640, 480)
bw=-1 bh=-1
base_width  min_width   base_height min_height
 -1 1   -1  1

but the correct solution is to change the data type from unsigned int to int everywhere it is needed

bbidulock commented 2 years ago

How can there be a negative width?

rikigigi commented 2 years ago

that's a good question, but if the XSizeHints is not unsigned, the code must be able to deal with negative integers and decide what to do (discard it). My particular case was the Zoom client 5.11.10 (4400) + gentoo linux

bbidulock commented 1 year ago

The X server will not return a negative width, so that is not an issue.