mail[Wesnoth-commits] r28479 - /trunk/src/editor2/


Others Months | Index by Date | Thread Index
>>   [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Header


Content

Posted by kailoran on August 12, 2008 - 12:10:
Author: ilor
Date: Tue Aug 12 10:23:56 2008
New Revision: 28479

URL: http://svn.gna.org/viewcvs/wesnoth?rev=28479&view=rev
Log:
use auto_ptrs to contain possible memory leaks in editor2 when perform_action 
throws, some minor cleanup

Modified:
    trunk/src/editor2/action.cpp
    trunk/src/editor2/action_base.hpp
    trunk/src/editor2/editor_controller.cpp
    trunk/src/editor2/editor_controller.hpp
    trunk/src/editor2/editor_main.cpp
    trunk/src/editor2/map_context.cpp
    trunk/src/editor2/map_context.hpp

Modified: trunk/src/editor2/action.cpp
URL: 
http://svn.gna.org/viewcvs/wesnoth/trunk/src/editor2/action.cpp?rev=28479&r1=28478&r2=28479&view=diff
==============================================================================
--- trunk/src/editor2/action.cpp (original)
+++ trunk/src/editor2/action.cpp Tue Aug 12 10:23:56 2008
@@ -22,12 +22,24 @@
 #include "../foreach.hpp"
 
 #include <algorithm>
+#include <memory>
 
 namespace editor2 {
 
 int editor_action::next_id_ = 1;
 int editor_action::instance_count_ = 0;
 
+editor_action::editor_action()
+: id_(next_id_++)
+{
+       instance_count_++;
+}
+
+editor_action::~editor_action()
+{
+       instance_count_--;
+}
+
 std::string editor_action::get_description()
 {
        return "Unknown action";
@@ -35,9 +47,9 @@
 
 editor_action* editor_action::perform(map_context& mc) const
 {
-       editor_action* undo = new editor_action_whole_map(mc.get_map());
-       perform_without_undo(mc);
-       return undo;
+       std::auto_ptr<editor_action> undo(new 
editor_action_whole_map(mc.get_map()));
+       perform_without_undo(mc);
+       return undo.release();
 }
 
 void editor_action_whole_map::perform_without_undo(map_context& mc) const {
@@ -55,12 +67,12 @@
        actions_.push_back(a);
 }
 editor_action_chain* editor_action_chain::perform(map_context& mc) const {
-       std::vector<editor_action*> undo;
+       std::auto_ptr<editor_action_chain> undo(new editor_action_chain());
        foreach (editor_action* a, actions_) {
-               undo.push_back(a->perform(mc));
-       }
-       std::reverse(undo.begin(), undo.end());
-       return new editor_action_chain(undo);
+               undo->append_action(a->perform(mc));
+       }
+       std::reverse(undo->actions_.begin(), undo->actions_.end());
+       return undo.release();
 }
 void editor_action_chain::perform_without_undo(map_context& mc) const
 {
@@ -77,9 +89,9 @@
 editor_action_paste* editor_action_paste::perform(map_context& mc) const
 {
        map_fragment mf(mc.get_map(), paste_.get_offset_area(loc_));
-       editor_action_paste* undo = new 
editor_action_paste(gamemap::location(0,0), mf);
-       perform_without_undo(mc);
-       return undo;
+       std::auto_ptr<editor_action_paste> undo(new 
editor_action_paste(gamemap::location(0,0), mf));
+       perform_without_undo(mc);
+       return undo.release();
 }
 void editor_action_paste::perform_without_undo(map_context& mc) const
 {
@@ -90,9 +102,9 @@
 
 editor_action_paint_hex* editor_action_paint_hex::perform(map_context& mc) 
const
 {
-       editor_action_paint_hex* undo = new editor_action_paint_hex(loc_, 
mc.get_map().get_terrain(loc_));
-       perform_without_undo(mc);
-       return undo;
+       std::auto_ptr<editor_action_paint_hex> undo(new 
editor_action_paint_hex(loc_, mc.get_map().get_terrain(loc_)));
+       perform_without_undo(mc);
+       return undo.release();
 }
 void editor_action_paint_hex::perform_without_undo(map_context& mc) const
 {
@@ -103,9 +115,9 @@
 editor_action_paste* editor_action_paint_area::perform(map_context& mc) const
 {
        map_fragment mf(mc.get_map(), area_);
-       editor_action_paste* undo = new 
editor_action_paste(gamemap::location(0,0), mf);
-       perform_without_undo(mc);
-       return undo;
+       std::auto_ptr<editor_action_paste> undo(new 
editor_action_paste(gamemap::location(0,0), mf));
+       perform_without_undo(mc);
+       return undo.release();
 }      
 void editor_action_paint_area::perform_without_undo(map_context& mc) const
 {
@@ -116,10 +128,10 @@
 editor_action_paint_area* editor_action_fill::perform(map_context& mc) const
 {
        std::set<gamemap::location> to_fill = 
mc.get_map().get_contigious_terrain_tiles(loc_);
-       editor_action_paint_area* undo = new 
editor_action_paint_area(to_fill, mc.get_map().get_terrain(loc_));
+       std::auto_ptr<editor_action_paint_area> undo(new 
editor_action_paint_area(to_fill, mc.get_map().get_terrain(loc_)));
        mc.draw_terrain(t_, to_fill, one_layer_);
        mc.set_needs_terrain_rebuild();
-       return undo;
+       return undo.release();
 }
 void editor_action_fill::perform_without_undo(map_context& mc) const
 {
@@ -130,7 +142,7 @@
 
 editor_action* editor_action_starting_position::perform(map_context& mc) 
const
 {
-       editor_action* undo;
+       std::auto_ptr<editor_action> undo;
        int old_player = mc.get_map().is_starting_position(loc_) + 1;
        gamemap::location old_loc = mc.get_map().starting_position(player_);
        LOG_ED << "ssp perform, player_" << player_ << ", loc_ " << loc_ << 
", old_player " << old_player << ", old_loc " << old_loc << "\n";
@@ -138,16 +150,16 @@
                editor_action_chain* undo_chain = new editor_action_chain();
                undo_chain->append_action(new 
editor_action_starting_position(loc_, old_player));
                undo_chain->append_action(new 
editor_action_starting_position(old_loc, player_));
-               undo = undo_chain;
+               undo.reset(undo_chain);
                LOG_ED << "ssp actual: " << old_player << " to " << 
gamemap::location() << "\n";
                mc.get_map().set_starting_position(old_player, 
gamemap::location());
        } else {
-               undo = new editor_action_starting_position(old_loc, player_);
+               undo.reset(new editor_action_starting_position(old_loc, 
player_));
        }
        LOG_ED << "ssp actual: " << player_ << " to " << loc_ << "\n";
        mc.get_map().set_starting_position(player_, loc_);
        mc.set_needs_labels_reset();
-       return undo;
+       return undo.release();
 }
 void editor_action_starting_position::perform_without_undo(map_context& mc) 
const
 {

Modified: trunk/src/editor2/action_base.hpp
URL: 
http://svn.gna.org/viewcvs/wesnoth/trunk/src/editor2/action_base.hpp?rev=28479&r1=28478&r2=28479&view=diff
==============================================================================
--- trunk/src/editor2/action_base.hpp (original)
+++ trunk/src/editor2/action_base.hpp Tue Aug 12 10:23:56 2008
@@ -36,16 +36,8 @@
 class editor_action
 {
     public:
-        editor_action()
-               : id_(next_id_++)
-        {
-                       instance_count_++;
-        }
-               
-        virtual ~editor_action()
-        {
-                       instance_count_--;
-        }
+        editor_action();
+        virtual ~editor_action();
                
                /**
                 * Perform the action, returning an undo action that, when 
performed, will reverse any effects of this action.

Modified: trunk/src/editor2/editor_controller.cpp
URL: 
http://svn.gna.org/viewcvs/wesnoth/trunk/src/editor2/editor_controller.cpp?rev=28479&r1=28478&r2=28479&view=diff
==============================================================================
--- trunk/src/editor2/editor_controller.cpp (original)
+++ trunk/src/editor2/editor_controller.cpp Tue Aug 12 10:23:56 2008
@@ -40,6 +40,8 @@
 #include "../wml_exception.hpp"
 
 #include "SDL.h"
+
+#include <memory>
 
 #include <boost/bind.hpp>
 
@@ -304,8 +306,7 @@
                                        x_offset = 0;
                        }
                        editor_action_resize_map a(w, h, x_offset, y_offset, 
fill);
-                       get_map_context().perform_action(a);
-                       refresh_after_action();
+                       perform_refresh(a);
                }
        }
 }
@@ -510,28 +511,19 @@
                        cut_selection();
                        return true;
                case HOTKEY_EDITOR_SELECT_ALL:
-                       if 
(!get_map_context().get_map().everything_selected()) {
-                               
get_map_context().perform_action(editor_action_select_all());
-                               refresh_after_action();
+                       if (!get_map().everything_selected()) {
+                               perform_refresh(editor_action_select_all());
                                return true;
                        } //else intentionally fall through
                case HOTKEY_EDITOR_SELECT_INVERSE:
-                       
get_map_context().perform_action(editor_action_select_inverse());
-                       refresh_after_action();
-                       return true;
-               case HOTKEY_EDITOR_MAP_FLIP_X: {
-                       editor_action_flip_x fx;
-                       get_map_context().perform_action(fx);
-                       refresh_after_action();
-                       return true;
-                       }
-               case HOTKEY_EDITOR_MAP_FLIP_Y: {
-                       LOG_ED << "FlipYhk\n";
-                       editor_action_flip_y fy;
-                       get_map_context().perform_action(fy);
-                       refresh_after_action();
-                       return true;
-                       }
+                       perform_refresh(editor_action_select_inverse());
+                       return true;
+               case HOTKEY_EDITOR_MAP_FLIP_X:
+                       perform_refresh(editor_action_flip_x());
+                       return true;
+               case HOTKEY_EDITOR_MAP_FLIP_Y:
+                       perform_refresh(editor_action_flip_y());
+                       return true;
                case HOTKEY_EDITOR_MAP_LOAD:
                        load_map_dialog();
                        return true;
@@ -668,8 +660,7 @@
 {
        copy_selection();
        editor_action_paint_area a(get_map().selection(), 
background_terrain_);
-       get_map_context().perform_action(a);
-       refresh_after_action();
+       perform_refresh(a);
 }
 
 void editor_controller::hotkey_set_mouse_action(hotkey::HOTKEY_COMMAND 
command)
@@ -712,6 +703,19 @@
 {
        return mouse_action_;
 }
+
+void editor_controller::perform_refresh_delete(editor_action* action)
+{
+       std::auto_ptr<editor_action> action_auto(action);
+       perform_refresh(*action);
+}
+
+void editor_controller::perform_refresh(const editor_action& action)
+{
+       get_map_context().perform_action(action);
+       refresh_after_action();
+}
+
 
 void editor_controller::redraw_toolbar()
 {
@@ -820,13 +824,13 @@
                        //Partial means that the mouse action has modified 
the last undo action and the controller shouldn't add
                        //anything to the undo stack (hence a diferent 
perform_ call
                        if (a != NULL) {
+                               std::auto_ptr<editor_action> aa(a);
                                if (partial) {
                                        
get_map_context().perform_partial_action(*a);
                                } else {
                                        get_map_context().perform_action(*a);
                                }
                                refresh_after_action(true);
-                               delete a;
                        }
                } else {
                        WRN_ED << __FUNCTION__ << ": There is no mouse action 
active!\n";
@@ -850,9 +854,7 @@
                LOG_ED << "Left click action " << hex_clicked.x << " " << 
hex_clicked.y << "\n";
                editor_action* a = get_mouse_action()->click(*gui_, x, y);
                if (a != NULL) {
-                       get_map_context().perform_action(*a);
-                       refresh_after_action(true);
-                       delete a;
+                       perform_refresh_delete(a);
                }
                return true;
        } else {
@@ -866,10 +868,8 @@
        if (get_mouse_action() != NULL) {
                editor_action* a = get_mouse_action()->drag_end(*gui_, x, y);
                if (a != NULL) {
-                       get_map_context().perform_action(*a);
-                       delete a;
-               }
-               refresh_after_action();
+                       perform_refresh_delete(a);
+               }
        } else {
                LOG_ED << __FUNCTION__ << ": There is no mouse action 
active!\n";
        }       

Modified: trunk/src/editor2/editor_controller.hpp
URL: 
http://svn.gna.org/viewcvs/wesnoth/trunk/src/editor2/editor_controller.hpp?rev=28479&r1=28478&r2=28479&view=diff
==============================================================================
--- trunk/src/editor2/editor_controller.hpp (original)
+++ trunk/src/editor2/editor_controller.hpp Tue Aug 12 10:23:56 2008
@@ -109,6 +109,8 @@
                editor_display& get_display();  
                brush* get_brush();
                mouse_action* get_mouse_action();
+               void perform_refresh_delete(editor_action* action);
+               void perform_refresh(const editor_action& action);
                
        private:                
                /** init the display object and general set-up */ 

Modified: trunk/src/editor2/editor_main.cpp
URL: 
http://svn.gna.org/viewcvs/wesnoth/trunk/src/editor2/editor_main.cpp?rev=28479&r1=28478&r2=28479&view=diff
==============================================================================
--- trunk/src/editor2/editor_main.cpp (original)
+++ trunk/src/editor2/editor_main.cpp Tue Aug 12 10:23:56 2008
@@ -36,7 +36,7 @@
                throw;
        }
        if (editor_action::get_instance_count() != 0) {
-               WRN_ED << "Possibly leaked " << 
editor_action::get_instance_count() << " action objects\n";
+               ERR_ED << "Possibly leaked " << 
editor_action::get_instance_count() << " action objects\n";
        }
        return e;
 }

Modified: trunk/src/editor2/map_context.cpp
URL: 
http://svn.gna.org/viewcvs/wesnoth/trunk/src/editor2/map_context.cpp?rev=28479&r1=28478&r2=28479&view=diff
==============================================================================
--- trunk/src/editor2/map_context.cpp (original)
+++ trunk/src/editor2/map_context.cpp Tue Aug 12 10:23:56 2008
@@ -221,7 +221,7 @@
 void map_context::perform_action_between_stacks(action_stack& from, 
action_stack& to)
 {
        assert(!from.empty());
-       editor_action* action = from.back();
+       std::auto_ptr<editor_action> action(from.back());
        from.pop_back();
        editor_action* reverse_action = action->perform(*this);
        to.push_back(reverse_action);

Modified: trunk/src/editor2/map_context.hpp
URL: 
http://svn.gna.org/viewcvs/wesnoth/trunk/src/editor2/map_context.hpp?rev=28479&r1=28478&r2=28479&view=diff
==============================================================================
--- trunk/src/editor2/map_context.hpp (original)
+++ trunk/src/editor2/map_context.hpp Tue Aug 12 10:23:56 2008
@@ -70,7 +70,8 @@
        
        /**
         * Performs an action (thus modyfying the map). An appropriate undo 
action is added to
-        * the undo stack. The redo stack is cleared.
+        * the undo stack. The redo stack is cleared. Note that this may 
throw, use caution
+        * when calling this with a dereferennced pointer that you own (i.e. 
use a smart pointer).
         */
        void perform_action(const editor_action& action);
        




Related Messages


Powered by MHonArc, Updated Tue Aug 12 13:40:47 2008