From f6a072430d4de159e229d6172adc5ba5ae985512 Mon Sep 17 00:00:00 2001
From: simonb <simonb>
Date: Mon, 24 Apr 2006 13:34:14 +0000
Subject: fix memory leaks in menu code

---
 ChangeLog          |  2 ++
 src/FbTk/Menu.cc   | 10 ++++++----
 src/MenuCreator.cc |  1 -
 src/Screen.cc      |  5 +++++
 src/Slit.cc        |  3 +++
 src/Toolbar.cc     |  3 +++
 src/fluxbox.cc     | 13 +++++++------
 7 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 5eac1dc..97023c6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,6 +1,8 @@
 (Format: Year/Month/Day)
 Changes for 0.9.16:
 *06/04/24:
+   * Fix memory leaks & other errors in Menu code (Simon)
+     Screen.cc Slit.cc Toolbar.cc fluxbox.cc MenuCreator.cc FbTk/Menu.cc
    * Use external tabs by default (Simon)
      - they provide a unique look to fluxbox
      - backwards compatible with 0.1.14 ("stable")
diff --git a/src/FbTk/Menu.cc b/src/FbTk/Menu.cc
index 4775ae9..b08bfe9 100644
--- a/src/FbTk/Menu.cc
+++ b/src/FbTk/Menu.cc
@@ -232,14 +232,16 @@ int Menu::remove(unsigned int index) {
     if (item) {
         menuitems.erase(it);
 
-        if (!m_internal_menu && item->submenu() != 0) {
+        if (item->submenu() != 0) {
             Menu *tmp = item->submenu();
-            // if menu is interal we should just hide it instead
+            // if menu is internal we should just hide it instead
             // if destroying it
             if (! tmp->m_internal_menu) {
                 delete tmp;
-            } else
-                tmp->internal_hide();
+            }
+            // We can't internal_hide here, as the child may be deleted!
+//            } else
+//                tmp->internal_hide();
         }
         
 		
diff --git a/src/MenuCreator.cc b/src/MenuCreator.cc
index 4a2cc0e..c97d51d 100644
--- a/src/MenuCreator.cc
+++ b/src/MenuCreator.cc
@@ -454,7 +454,6 @@ FbTk::Menu *MenuCreator::createMenuType(const std::string &type, int screen_num)
     } else if (type == "windowmenu") {
         FbTk::Menu *menu = screen->createMenu("");
 
-        menu->removeAll(); // clear old items
         menu->disableTitle(); // not titlebar
         if (screen->windowMenuFilename().empty() ||
             ! createWindowMenuFromFile(screen->windowMenuFilename(), *menu, true)) {
diff --git a/src/Screen.cc b/src/Screen.cc
index a41fb0f..03ec510 100644
--- a/src/Screen.cc
+++ b/src/Screen.cc
@@ -728,6 +728,7 @@ void BScreen::addExtraWindowMenu(const char *label, FbTk::Menu *menu) {
     m_extramenus.push_back(std::make_pair(label, menu));
     // recreate window menu
     m_windowmenu.reset(MenuCreator::createMenuType("windowmenu", screenNumber()));
+    m_windowmenu->setInternalMenu();
 }
 
 void BScreen::removeExtraWindowMenu(FbTk::Menu *menu) {
@@ -739,6 +740,7 @@ void BScreen::removeExtraWindowMenu(FbTk::Menu *menu) {
         m_extramenus.erase(it);
     // recreate window menu
     m_windowmenu.reset(MenuCreator::createMenuType("windowmenu", screenNumber()));
+    m_windowmenu->setInternalMenu();
 }
 
 void BScreen::hideMenus() {
@@ -827,6 +829,7 @@ void BScreen::reconfigure() {
     m_configmenu->reconfigure();
     // recreate window menu
     m_windowmenu.reset(MenuCreator::createMenuType("windowmenu", screenNumber()));
+    m_windowmenu->setInternalMenu();
 
     // We need to check to see if the timestamps
     // changed before we actually can restore the menus 
@@ -1501,6 +1504,7 @@ void BScreen::reassociateWindow(FluxboxWindow *w, unsigned int wkspc_id,
 void BScreen::initMenus() {
     m_workspacemenu.reset(MenuCreator::createMenuType("workspacemenu", screenNumber()));
     m_windowmenu.reset(MenuCreator::createMenuType("windowmenu", screenNumber()));
+    m_windowmenu->setInternalMenu();
     initMenu();
 }
 
@@ -1554,6 +1558,7 @@ void BScreen::removeConfigMenu(FbTk::Menu &menu) {
                                                            FbTk::Select2nd<Configmenus::value_type>()));
     if (erase_it != m_configmenu_list.end())
         m_configmenu_list.erase(erase_it);
+
     setupConfigmenu(*m_configmenu.get());
 
 }    
diff --git a/src/Slit.cc b/src/Slit.cc
index af3a0be..db2deae 100644
--- a/src/Slit.cc
+++ b/src/Slit.cc
@@ -347,6 +347,9 @@ Slit::~Slit() {
     if (frame.pixmap != 0)
         screen().imageControl().removeImage(frame.pixmap);
 
+    // otherwise it will try to access it on deletion
+    screen().removeConfigMenu(m_slitmenu); 
+
     shutdown();
 }
 
diff --git a/src/Toolbar.cc b/src/Toolbar.cc
index 6384430..a863b3f 100644
--- a/src/Toolbar.cc
+++ b/src/Toolbar.cc
@@ -252,6 +252,7 @@ Toolbar::Toolbar(BScreen &scrn, FbTk::XLayer &layer, size_t width):
 
     m_layermenu.setInternalMenu();
     m_placementmenu.setInternalMenu();
+    m_toolbarmenu.setInternalMenu();
     setupMenus();
     // add menu to screen
     screen().addConfigMenu(_FBTEXT(Toolbar, Toolbar, "Toolbar", "title of toolbar menu item"), menu());
@@ -287,6 +288,8 @@ Toolbar::~Toolbar() {
     // remove menu items before we delete tools so we dont end up
     // with dangling pointers to old submenu items (internal menus)
     // from the tools
+    screen().removeConfigMenu(menu());
+
     menu().removeAll();
 
     deleteItems();
diff --git a/src/fluxbox.cc b/src/fluxbox.cc
index f5066b7..c18cc32 100644
--- a/src/fluxbox.cc
+++ b/src/fluxbox.cc
@@ -418,12 +418,6 @@ Fluxbox::~Fluxbox() {
         delete m_toolbars.back();
         m_toolbars.pop_back();
     }
-   
-    // destroy screens
-    while (!m_screen_list.empty()) {
-        delete m_screen_list.back();
-        m_screen_list.pop_back();
-    }
 
     // destroy atomhandlers
     for (AtomHandlerContainerIt it= m_atomhandler.begin();
@@ -433,6 +427,13 @@ Fluxbox::~Fluxbox() {
     }
     m_atomhandler.clear();
 
+    // destroy screens (after others, as they may do screen things)
+    while (!m_screen_list.empty()) {
+        delete m_screen_list.back();
+        m_screen_list.pop_back();
+    }
+
+
     clearMenuFilenames();
 }
 
-- 
cgit v0.11.2