[openbox] [PATCH] fix "key leak" 2nd attempt

ManMower manmower at signalmarketing.com
Tue Feb 11 15:26:27 EST 2003


AND HERE IS THE ATTACHMENT!

On Tue, 11 Feb 2003, ManMower wrote:

>
> changes from the last patch include revised reset key handling, replacing
> an accidentally removed pointer update on chain transition, and not
> allowing a python callback to ungrab the keyboard completely while in the
> middle of processing a keychain.
>
> it's still dangerous though.
>
>
-------------- next part --------------
? ABOUT-NLS
Index: otk/display.cc
===================================================================
RCS file: /cvs/cvsroot/openbox/otk/display.cc,v
retrieving revision 1.37
diff -p -u -r1.37 display.cc
--- otk/display.cc	2003/02/09 23:07:53	1.37
+++ otk/display.cc	2003/02/11 20:21:36
@@ -300,4 +300,9 @@ void Display::ungrabKey(unsigned int key
                grab_window);
 }
 
+void Display::ungrabAllKeys(Window grab_window) const
+{
+  XUngrabKey(_display, AnyKey, AnyModifier, grab_window);
+}
+
 }
Index: otk/display.hh
===================================================================
RCS file: /cvs/cvsroot/openbox/otk/display.hh,v
retrieving revision 1.21
diff -p -u -r1.21 display.hh
--- otk/display.hh	2003/01/29 08:50:30	1.21
+++ otk/display.hh	2003/02/11 20:21:36
@@ -133,6 +133,7 @@ public:
                bool allow_scroll_lock) const;
   void ungrabKey(unsigned int keycode, unsigned int modifiers,
                  Window grab_window) const;
+  void ungrabAllKeys(Window grab_window) const;
 };
 
 }
Index: src/bindings.cc
===================================================================
RCS file: /cvs/cvsroot/openbox/src/bindings.cc,v
retrieving revision 1.72
diff -p -u -r1.72 bindings.cc
--- src/bindings.cc	2003/02/09 23:07:54	1.72
+++ src/bindings.cc	2003/02/11 20:21:37
@@ -145,9 +145,10 @@ Bindings::Bindings()
   : _curpos(&_keytree),
     _resetkey(0,0),
     _timer((otk::Timer *) 0),
-    _keybgrab_callback(0, 0)
+    _keybgrab_callback(0, 0),
+    _grabbed(0)
 {
-//  setResetKey("C-g"); // set the default reset key
+  setResetKey("C-g"); // set the default reset key
 }
 
 
@@ -155,7 +156,11 @@ Bindings::~Bindings()
 {
   if (_timer)
     delete _timer;
-
+  if (_grabbed) {
+    _grabbed = false;
+    XUngrabKeyboard(**otk::display, CurrentTime);
+  }
+  grabKeys(false);
   removeAllKeys();
   //removeAllButtons(); // this is done by each client as they are unmanaged
   removeAllEvents();
@@ -296,13 +301,8 @@ void Bindings::setResetKey(const std::st
 {
   Binding b(0, 0);
   if (translate(key, b)) {
-    // grab the server here to make sure no key pressed go missed
-    otk::display->grab();
-    grabKeys(false);
     _resetkey.key = b.key;
     _resetkey.modifiers = b.modifiers;
-    grabKeys(true);
-    otk::display->ungrab();
   }
 }
 
@@ -323,7 +323,6 @@ static void remove_branch(KeyBindingTree
   }
 }
 
-
 void Bindings::removeAllKeys()
 {
   grabKeys(false);
@@ -341,28 +340,17 @@ void Bindings::grabKeys(bool grab)
     Screen *sc = openbox->screen(i);
     if (!sc) continue; // not a managed screen
     Window root = otk::display->screenInfo(i)->rootWindow();
-
-    KeyBindingTree *p = _curpos->first_child;
+    if (!grab) {
+      otk::display->ungrabAllKeys(root);
+      continue;
+    }
+    KeyBindingTree *p = _keytree.first_child;
     while (p) {
-      if (grab) {
-        otk::display->grabKey(p->binding.key, p->binding.modifiers,
-                                root, false, GrabModeAsync, GrabModeAsync,
-                                false);
-      }
-      else
-        otk::display->ungrabKey(p->binding.key, p->binding.modifiers,
-                                  root);
+      otk::display->grabKey(p->binding.key, p->binding.modifiers,
+                              root, false, GrabModeAsync, GrabModeSync,
+                              false);
       p = p->next_sibling;
     }
-
-    if (_resetkey.key)
-      if (grab)
-        otk::display->grabKey(_resetkey.key, _resetkey.modifiers,
-                                root, false, GrabModeAsync, GrabModeAsync,
-                                false);
-      else
-        otk::display->ungrabKey(_resetkey.key, _resetkey.modifiers,
-                                  root);
   }
 }
 
@@ -390,8 +378,9 @@ void Bindings::ungrabKeyboard()
   if (!_keybgrab_callback.callback) return; // not grabbed
 
   _keybgrab_callback = KeyCallbackData(0, 0);
-  XUngrabKeyboard(**otk::display, CurrentTime);
   XUngrabPointer(**otk::display, CurrentTime);
+  if (!_grabbed)  /* don't release out from under keychains */
+    XUngrabKeyboard(**otk::display, CurrentTime);
 }
 
 
@@ -421,12 +410,13 @@ void Bindings::fireKey(int screen, unsig
     KeyData data(screen, c, time, modifiers, key, action);
     _keybgrab_callback.fire(&data);
   }
-  
+
   // KeyRelease events only occur during keyboard grabs
   if (action == KeyAction::Release) return;
     
   if (key == _resetkey.key && modifiers == _resetkey.modifiers) {
     resetChains(this);
+    XAllowEvents(**otk::display, AsyncKeyboard, CurrentTime);
   } else {
     KeyBindingTree *p = _curpos->first_child;
     while (p) {
@@ -437,18 +427,24 @@ void Bindings::fireKey(int screen, unsig
           _timer = new otk::Timer(5000, // 5 second timeout
                                   (otk::Timer::TimeoutHandler)resetChains,
                                   this);
-          // grab the server here to make sure no key presses get missed
-          otk::display->grab();
-          grabKeys(false);
-          _curpos = p;
-          grabKeys(true);
-          otk::display->ungrab();
+          if (!_grabbed && !_keybgrab_callback.callback) {
+            Window root = otk::display->screenInfo(screen)->rootWindow();
+            //grab should never fail because we should have a sync grab at 
+            //this point
+            XGrabKeyboard(**otk::display, root, 0, GrabModeAsync, 
+                          GrabModeSync, CurrentTime);
+            _grabbed = true;
+            grabKeys(false);
+            _curpos = p;
+          }
+          XAllowEvents(**otk::display, AsyncKeyboard, CurrentTime);
         } else {
           Client *c = openbox->focusedClient();
           KeyData data(screen, c, time, modifiers, key, action);
           KeyCallbackList::iterator it, end = p->callbacks.end();
           for (it = p->callbacks.begin(); it != end; ++it)
             it->fire(&data);
+          XAllowEvents(**otk::display, AsyncKeyboard, CurrentTime);
           resetChains(this);
         }
         break;
@@ -463,13 +459,19 @@ void Bindings::resetChains(Bindings *sel
   if (self->_timer) {
     delete self->_timer;
     self->_timer = (otk::Timer *) 0;
+  }
+  /* if we're not at the root, we've turned off the grabbed keys
+     and we're protected by an exclusive keyboard grab
+     else we've still got the right grabs in place */
+  if (self->_curpos != &self->_keytree) {
+    self->_curpos = &self->_keytree;
+    self->grabKeys(true);
+  }
+  if (self->_grabbed) {
+    self->_grabbed = false;
+    if (!self->_keybgrab_callback.callback)
+      XUngrabKeyboard(**otk::display, CurrentTime);
   }
-  // grab the server here to make sure no key pressed go missed
-  otk::display->grab();
-  self->grabKeys(false);
-  self->_curpos = &self->_keytree;
-  self->grabKeys(true);
-  otk::display->ungrab();
 }
 
 
Index: src/bindings.hh
===================================================================
RCS file: /cvs/cvsroot/openbox/src/bindings.hh,v
retrieving revision 1.32
diff -p -u -r1.32 bindings.hh
--- src/bindings.hh	2003/02/05 15:38:29	1.32
+++ src/bindings.hh	2003/02/11 20:21:37
@@ -116,7 +116,9 @@ private:
   EventCallbackList _eventlist[EventAction::NUM_EVENT_ACTION];
 
   KeyCallbackData _keybgrab_callback;
-  
+
+  bool _grabbed;
+
 public:
   //! Initializes an Bindings object
   Bindings();


More information about the openbox mailing list