Merge remote-tracking branch 'suokko/screen_show_memory_leak' into develop

library/LuaApi.cpp

@@ -2243,11 +2243,7 @@ int screen_show(lua_State *L)
 
     df::viewscreen *screen = dfhack_lua_viewscreen::get_pointer(L, 1, true);
 
-    bool ok = Screen::show(screen, before);
+    bool ok = Screen::show(std::unique_ptr<df::viewscreen>{screen}, before);
-
-    // If it is a table, get_pointer created a new object. Don't leak it.
-    if (!ok && lua_istable(L, 1))
-        delete screen;
 
     lua_pushboolean(L, ok);
     return 1;

library/include/MiscUtils.h

@@ -31,6 +31,7 @@ distribution.
 #include <vector>
 #include <sstream>
 #include <cstdio>
+#include <memory>
 
 using std::ostream;
 using std::stringstream;
@@ -44,6 +45,28 @@ using std::endl;
     #define DFHACK_FUNCTION_SIG __func__
 #endif
 
+/*! \namespace dts
+ * std.reverse() == dts, The namespace that include forward compatible helpers
+ * which can be used from newer standards. The preprocessor check prefers
+ * standard version if one is available. The standard version gets imported with
+ * using.
+ */
+namespace dts {
+//  Check if lib supports the feature test macro or version is over c++14.
+#if __cpp_lib_make_unique < 201304 && __cplusplus < 201402L
+//! Insert c++14 make_unique to be forward compatible. Array versions are
+//! missing
+template<typename T, typename... Args>
+typename std::enable_if<!std::is_array<T>::value, std::unique_ptr<T> >::type
+make_unique(Args&&... args)
+{
+    return std::unique_ptr<T>{new T{std::forward<Args>(args)...}};
+}
+#else /* >= c++14 */
+using std::make_unique;
+#endif
+}
+
 template <typename T>
 void print_bits ( T val, ostream& out )
 {

library/include/modules/Screen.h

@@ -31,6 +31,7 @@ distribution.
 
 #include <string>
 #include <set>
+#include <memory>
 
 #include "DataDefs.h"
 #include "df/graphic.h"
@@ -210,9 +211,9 @@ namespace DFHack
         DFHACK_EXPORT bool findGraphicsTile(const std::string &page, int x, int y, int *ptile, int *pgs = NULL);
 
         // Push and remove viewscreens
-        DFHACK_EXPORT bool show(df::viewscreen *screen, df::viewscreen *before = NULL, Plugin *p = NULL);
+        DFHACK_EXPORT bool show(std::unique_ptr<df::viewscreen> screen, df::viewscreen *before = NULL, Plugin *p = NULL);
-        inline bool show(df::viewscreen *screen, Plugin *p)
+        inline bool show(std::unique_ptr<df::viewscreen> screen, Plugin *p)
-            { return show(screen, NULL, p); }
+            { return show(std::move(screen), NULL, p); }
         DFHACK_EXPORT void dismiss(df::viewscreen *screen, bool to_first = false);
         DFHACK_EXPORT bool isDismissed(df::viewscreen *screen);
         DFHACK_EXPORT bool hasActiveScreens(Plugin *p);

library/modules/Screen.cpp

@@ -299,7 +299,7 @@ bool Screen::findGraphicsTile(const std::string &pagename, int x, int y, int *pt
 
 static std::map<df::viewscreen*, Plugin*> plugin_screens;
 
-bool Screen::show(df::viewscreen *screen, df::viewscreen *before, Plugin *plugin)
+bool Screen::show(std::unique_ptr<df::viewscreen> screen, df::viewscreen *before, Plugin *plugin)
 {
     CHECK_NULL_POINTER(screen);
     CHECK_INVALID_ARGUMENT(!screen->parent && !screen->child);
@@ -316,15 +316,16 @@ bool Screen::show(df::viewscreen *screen, df::viewscreen *before, Plugin *plugin
 
     screen->child = parent->child;
     screen->parent = parent;
-    parent->child = screen;
+    df::viewscreen* s = screen.release();
-    if (screen->child)
+    parent->child = s;
-        screen->child->parent = screen;
+    if (s->child)
+        s->child->parent = s;
 
-    if (dfhack_viewscreen::is_instance(screen))
+    if (dfhack_viewscreen::is_instance(s))
-        static_cast<dfhack_viewscreen*>(screen)->onShow();
+        static_cast<dfhack_viewscreen*>(s)->onShow();
 
     if (plugin)
-        plugin_screens[screen] = plugin;
+        plugin_screens[s] = plugin;
 
     return true;
 }

plugins/autochop.cpp

@@ -809,7 +809,7 @@ struct autochop_hook : public df::viewscreen_dwarfmodest
         if (isInDesignationMenu() && input->count(interface_key::CUSTOM_C))
         {
             sendKey(interface_key::LEAVESCREEN);
-            Screen::show(new ViewscreenAutochop(), plugin_self);
+            Screen::show(dts::make_unique<ViewscreenAutochop>(), plugin_self);
         }
         else
         {
@@ -852,7 +852,7 @@ command_result df_autochop (color_ostream &out, vector <string> & parameters)
             return CR_WRONG_USAGE;
     }
     if (Maps::IsValid())
-        Screen::show(new ViewscreenAutochop(), plugin_self);
+        Screen::show(dts::make_unique<ViewscreenAutochop>(), plugin_self);
     return CR_OK;
 }
 

plugins/buildingplan.cpp

@@ -156,7 +156,7 @@ struct buildingplan_hook : public df::viewscreen_dwarfmodest
                 }
                 else if (input->count(interface_key::CUSTOM_SHIFT_M))
                 {
-                    Screen::show(new ViewscreenChooseMaterial(planner.getDefaultItemFilterForType(type)), plugin_self);
+                    Screen::show(dts::make_unique<ViewscreenChooseMaterial>(planner.getDefaultItemFilterForType(type)), plugin_self);
                 }
                 else if (input->count(interface_key::CUSTOM_Q))
                 {

plugins/command-prompt.cpp

@@ -320,7 +320,7 @@ command_result show_prompt(color_ostream &out, std::vector <std::string> & param
     std::string params;
     for(size_t i=0;i<parameters.size();i++)
         params+=parameters[i]+" ";
-    Screen::show(new viewscreen_commandpromptst(params), plugin_self);
+    Screen::show(dts::make_unique<viewscreen_commandpromptst>(params), plugin_self);
     return CR_OK;
 }
 bool hotkey_allow_all(df::viewscreen *top)

plugins/dwarfmonitor.cpp

@@ -1062,7 +1062,7 @@ public:
         {
             df::unit *selected_unit = (selected_column == 1) ? dwarf_activity_column.getFirstSelectedElem() : nullptr;
             Screen::dismiss(this);
-            Screen::show(new ViewscreenDwarfStats(selected_unit), plugin_self);
+            Screen::show(dts::make_unique<ViewscreenDwarfStats>(selected_unit), plugin_self);
         }
         else if  (input->count(interface_key::CUSTOM_SHIFT_Z))
         {
@@ -1643,7 +1643,7 @@ public:
             {
                 auto unitscr = df::allocate<df::viewscreen_unitst>();
                 unitscr->unit = unit;
-                Screen::show(unitscr);
+                Screen::show(std::unique_ptr<df::viewscreen>(unitscr));
             }
         }
         else if  (input->count(interface_key::CUSTOM_SHIFT_Z))
@@ -1737,7 +1737,7 @@ private:
 
 static void open_stats_screen()
 {
-    Screen::show(new ViewscreenFortStats(), plugin_self);
+    Screen::show(dts::make_unique<ViewscreenFortStats>(), plugin_self);
 }
 
 static void add_work_history(df::unit *unit, activity_type type)
@@ -1977,12 +1977,12 @@ static command_result dwarfmonitor_cmd(color_ostream &out, vector <string> & par
         else if (cmd == 's' || cmd == 'S')
         {
             if(Maps::IsValid())
-                Screen::show(new ViewscreenFortStats(), plugin_self);
+                Screen::show(dts::make_unique<ViewscreenFortStats>(), plugin_self);
         }
         else if (cmd == 'p' || cmd == 'P')
         {
             if(Maps::IsValid())
-                Screen::show(new ViewscreenPreferences(), plugin_self);
+                Screen::show(dts::make_unique<ViewscreenPreferences>(), plugin_self);
         }
         else if (cmd == 'r' || cmd == 'R')
         {

plugins/embark-assistant/finder_ui.cpp

@@ -1463,7 +1463,7 @@ void embark_assist::finder_ui::init(DFHack::Plugin *plugin_self, embark_assist::
     if (!embark_assist::finder_ui::state) {  //  First call. Have to do the setup
         embark_assist::finder_ui::ui_setup(find_callback, max_inorganic);
     }
-    Screen::show(new ViewscreenFindUi(), plugin_self);
+    Screen::show(dts::make_unique<ViewscreenFindUi>(), plugin_self);
 }
 
 //===============================================================================

plugins/embark-assistant/help_ui.cpp

@@ -327,5 +327,5 @@ namespace embark_assist{
 //===============================================================================
 
 void embark_assist::help_ui::init(DFHack::Plugin *plugin_self) {
-    Screen::show(new embark_assist::help_ui::ViewscreenHelpUi(), plugin_self);
+    Screen::show(dts::make_unique<embark_assist::help_ui::ViewscreenHelpUi>(), plugin_self);
 }

plugins/embark-tools.cpp

@@ -696,7 +696,7 @@ struct choose_start_site_hook : df::viewscreen_choose_start_sitest
 
     void display_settings()
     {
-        Screen::show(new embark_tools_settings, plugin_self);
+        Screen::show(dts::make_unique<embark_tools_settings>(), plugin_self);
     }
 
     inline bool is_valid_page()

plugins/hotkeys.cpp

@@ -312,7 +312,7 @@ static command_result hotkeys_cmd(color_ostream &out, vector <string> & paramete
             if (Gui::getFocusString(top_screen) != "dfhack/viewscreen_hotkeys")
             {
                 find_active_keybindings(top_screen);
-                Screen::show(new ViewscreenHotkeys(top_screen), plugin_self);
+                Screen::show(dts::make_unique<ViewscreenHotkeys>(top_screen), plugin_self);
             }
         }
     }

plugins/manipulator.cpp

@@ -1814,14 +1814,14 @@ void viewscreen_unitlaborsst::feed(set<df::interface_key> *events)
 
     if (events->count(interface_key::CUSTOM_B))
     {
-        Screen::show(new viewscreen_unitbatchopst(units, true, &do_refresh_names), plugin_self);
+        Screen::show(dts::make_unique<viewscreen_unitbatchopst>(units, true, &do_refresh_names), plugin_self);
     }
 
     if (events->count(interface_key::CUSTOM_E))
     {
         vector<UnitInfo*> tmp;
         tmp.push_back(cur);
-        Screen::show(new viewscreen_unitbatchopst(tmp, false, &do_refresh_names), plugin_self);
+        Screen::show(dts::make_unique<viewscreen_unitbatchopst>(tmp, false, &do_refresh_names), plugin_self);
     }
 
     if (events->count(interface_key::CUSTOM_P))
@@ -1832,11 +1832,11 @@ void viewscreen_unitlaborsst::feed(set<df::interface_key> *events)
                 has_selected = true;
 
         if (has_selected) {
-            Screen::show(new viewscreen_unitprofessionset(units, true), plugin_self);
+            Screen::show(dts::make_unique<viewscreen_unitprofessionset>(units, true), plugin_self);
         } else {
             vector<UnitInfo*> tmp;
             tmp.push_back(cur);
-            Screen::show(new viewscreen_unitprofessionset(tmp, false), plugin_self);
+            Screen::show(dts::make_unique<viewscreen_unitprofessionset>(tmp, false), plugin_self);
         }
     }
 
@@ -2187,7 +2187,7 @@ struct unitlist_hook : df::viewscreen_unitlistst
         {
             if (units[page].size())
             {
-                Screen::show(new viewscreen_unitlaborsst(units[page], cursor_pos[page]), plugin_self);
+                Screen::show(dts::make_unique<viewscreen_unitlaborsst>(units[page], cursor_pos[page]), plugin_self);
                 return;
             }
         }

plugins/stocks.cpp

@@ -677,7 +677,7 @@ public:
         }
         else if (input->count(interface_key::HELP))
         {
-            Screen::show(new search_help, plugin_self);
+            Screen::show(dts::make_unique<search_help>(), plugin_self);
         }
 
         bool key_processed = false;
@@ -1342,7 +1342,7 @@ struct stocks_hook : public df::viewscreen_storesst
         if (input->count(interface_key::CUSTOM_E))
         {
             Screen::dismiss(this);
-            Screen::show(new ViewscreenStocks(), plugin_self);
+            Screen::show(dts::make_unique<ViewscreenStocks>(), plugin_self);
             return;
         }
         INTERPOSE_NEXT(feed)(input);
@@ -1377,7 +1377,7 @@ struct stocks_stockpile_hook : public df::viewscreen_dwarfmodest
 
         if (input->count(interface_key::CUSTOM_I))
         {
-            Screen::show(new ViewscreenStocks(sp), plugin_self);
+            Screen::show(dts::make_unique<ViewscreenStocks>(sp), plugin_self);
             return true;
         }
 
@@ -1451,7 +1451,7 @@ static command_result stocks_cmd(color_ostream &out, vector <string> & parameter
         }
         else if (toLower(parameters[0])[0] == 's')
         {
-            Screen::show(new ViewscreenStocks(), plugin_self);
+            Screen::show(dts::make_unique<ViewscreenStocks>(), plugin_self);
             return CR_OK;
         }
     }