From 2f3b67c9e060366c9018efa96f6bd4f3aef0bf43 Mon Sep 17 00:00:00 2001 From: Norman Feske Date: Wed, 19 Mar 2014 14:13:47 +0100 Subject: [PATCH] nitpicker: Fix implementation of Mode interface The generalization of nitpicker's graphic backend changed the interface of 'Mode::forget', which is a (non-pure) virtual function implemented by 'User_state::forget'. Unfortunately, the signature change was not applied to 'User_state::forget' so that the actual implementation was no longer called. This inconsistency remained unnoticed because there is a default implementation of the virtual function. The effect of the omission of the 'User_state::forget' call was a dangling pointer ('User_state::_pointed_view'). Lesson learned: Always annotate functions with the C++11 'override' when implementing virtual functions. --- os/src/server/nitpicker/mode.h | 3 ++- os/src/server/nitpicker/user_state.cc | 2 +- os/src/server/nitpicker/user_state.h | 2 +- os/src/server/nitpicker/view_stack.cc | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/os/src/server/nitpicker/mode.h b/os/src/server/nitpicker/mode.h index 1b95fe56b..a6e645760 100644 --- a/os/src/server/nitpicker/mode.h +++ b/os/src/server/nitpicker/mode.h @@ -15,6 +15,7 @@ #define _MODE_H_ class View; +class Canvas_base; class Mode { @@ -53,7 +54,7 @@ class Mode /** * Discard all references to specified view */ - virtual void forget(View const &v) { + virtual void forget(Canvas_base &, View const &v) { if (&v == _focused_view) _focused_view = 0; } }; diff --git a/os/src/server/nitpicker/user_state.cc b/os/src/server/nitpicker/user_state.cc index 8b854a50b..7ee395c46 100644 --- a/os/src/server/nitpicker/user_state.cc +++ b/os/src/server/nitpicker/user_state.cc @@ -281,7 +281,7 @@ void User_state::handle_event(Input::Event ev, Canvas_base &canvas) void User_state::forget(Canvas_base &canvas, View const &view) { if (focused_view() == &view) { - Mode::forget(view); + Mode::forget(canvas, view); _menubar.state(Menubar_state(*this, "", "", BLACK)); update_all_views(canvas); } diff --git a/os/src/server/nitpicker/user_state.h b/os/src/server/nitpicker/user_state.h index 4d9d38a32..5c412c319 100644 --- a/os/src/server/nitpicker/user_state.h +++ b/os/src/server/nitpicker/user_state.h @@ -89,7 +89,7 @@ class User_state : public Mode, public View_stack /** * Mode interface */ - void forget(Canvas_base &, View const &); + void forget(Canvas_base &, View const &) override; }; #endif diff --git a/os/src/server/nitpicker/view_stack.cc b/os/src/server/nitpicker/view_stack.cc index a72e2343e..89230750a 100644 --- a/os/src/server/nitpicker/view_stack.cc +++ b/os/src/server/nitpicker/view_stack.cc @@ -315,7 +315,7 @@ void View_stack::remove_view(Canvas_base &canvas, View const &view, bool redraw) * the current one, resulting in a dangling pointer right after the view * gets destructed by the caller of 'removed_view'. */ - _mode.forget(view); + _mode.forget(canvas, view); /* redraw area where the view was visible */ if (redraw)