From c71862174a60e0719751afa9ca8f2ebcaa774f39 Mon Sep 17 00:00:00 2001 From: rubidium Date: Wed, 7 Jan 2009 18:59:46 +0000 Subject: (svn r14905) -Fix (r14899): in some corner cases already freed memory could be read. --- src/viewport.cpp | 3 +-- src/window.cpp | 59 +++++++++++++++++++++++++++++++++++++++----------------- src/window_gui.h | 11 +++++++++-- 3 files changed, 51 insertions(+), 22 deletions(-) diff --git a/src/viewport.cpp b/src/viewport.cpp index 64f4898a1..88278309b 100644 --- a/src/viewport.cpp +++ b/src/viewport.cpp @@ -219,8 +219,7 @@ static Point _vp_move_offs; static void DoSetViewportPosition(const Window *w, int left, int top, int width, int height) { - - for (; w != NULL; w = w->z_front) { + FOR_ALL_WINDOWS_FROM_BACK_FROM(w, w) { if (left + width > w->left && w->left + w->width > left && top + height > w->top && diff --git a/src/window.cpp b/src/window.cpp index 334533253..5f5bc91a8 100644 --- a/src/window.cpp +++ b/src/window.cpp @@ -296,7 +296,8 @@ static void DispatchMouseWheelEvent(Window *w, int widget, int wheel) */ static void DrawOverlappedWindow(Window *w, int left, int top, int right, int bottom) { - for (const Window *v = w->z_front; v != NULL; v = v->z_front) { + const Window *v; + FOR_ALL_WINDOWS_FROM_BACK_FROM(v, w->z_front) { if (right > v->left && bottom > v->top && left < v->left + v->width && @@ -427,21 +428,6 @@ Window::~Window() /* Prevent Mouseover() from resetting mouse-over coordinates on a non-existing window */ if (_mouseover_last_w == this) _mouseover_last_w = NULL; - /* Find the window in the z-array, and effectively remove it - * by moving all windows after it one to the left. This must be - * done before removing the child so we cannot cause recursion - * between the deletion of the parent and the child. */ - if (this->z_front == NULL) { - _z_front_window = this->z_back; - } else { - this->z_front->z_back = this->z_back; - } - if (this->z_back == NULL) { - _z_back_window = this->z_front; - } else { - this->z_back->z_front = this->z_front; - } - this->DeleteChildWindows(); if (this->viewport != NULL) DeleteWindowViewport(this); @@ -454,6 +440,8 @@ Window::~Window() } free(this->widget); } + + this->window_class = WC_INVALID; } /** @@ -1091,7 +1079,17 @@ void InitWindowSystem() */ void UnInitWindowSystem() { - while (_z_front_window != NULL) delete _z_front_window; + Window *w; + FOR_ALL_WINDOWS_FROM_FRONT(w) delete w; + + for (w = _z_front_window; w != NULL; /* nothing */) { + Window *to_del = w; + w = w->z_back; + free(to_del); + } + + _z_front_window = NULL; + _z_back_window = NULL; } /** @@ -1583,7 +1581,8 @@ static bool MaybeBringWindowToFront(Window *w) return true; } - for (Window *u = w->z_front; u != NULL; u = u->z_front) { + Window *u; + FOR_ALL_WINDOWS_FROM_BACK_FROM(u, w->z_front) { /* A modal child will prevent the activation of the parent window */ if (u->parent == w && (u->desc_flags & WDF_MODAL)) { u->flags4 |= WF_WHITE_BORDER_MASK; @@ -1973,6 +1972,30 @@ void InputLoop() CheckSoftLimit(); HandleKeyScrolling(); + /* Do the actual free of the deleted windows. */ + for (Window *v = _z_front_window; v != NULL; /* nothing */) { + Window *w = v; + v = v->z_back; + + if (w->window_class != WC_INVALID) continue; + + /* Find the window in the z-array, and effectively remove it + * by moving all windows after it one to the left. This must be + * done before removing the child so we cannot cause recursion + * between the deletion of the parent and the child. */ + if (w->z_front == NULL) { + _z_front_window = w->z_back; + } else { + w->z_front->z_back = w->z_back; + } + if (w->z_back == NULL) { + _z_back_window = w->z_front; + } else { + w->z_back->z_front = w->z_front; + } + free(w); + } + if (_input_events_this_tick != 0) { /* The input loop is called only once per GameLoop() - so we can clear the counter here */ _input_events_this_tick = 0; diff --git a/src/window_gui.h b/src/window_gui.h index ea2a5f076..266bf5a80 100644 --- a/src/window_gui.h +++ b/src/window_gui.h @@ -203,6 +203,11 @@ public: Window(const WindowDesc *desc, WindowNumber number = 0); virtual ~Window(); + /* Don't allow arrays; arrays of Windows are pointless as you need + * to destruct them all at the same time too, which is kinda hard. */ + FORCEINLINE void *operator new[](size_t size) { NOT_REACHED(); } + /* Don't free the window directly; it corrupts the linked list when iterating */ + FORCEINLINE void operator delete(void *ptr, size_t size) {} uint16 flags4; ///< Window flags, @see WindowFlags WindowClass window_class; ///< Window class @@ -543,8 +548,10 @@ extern Window *_z_front_window; extern Window *_z_back_window; /** Iterate over all windows */ -#define FOR_ALL_WINDOWS_FROM_BACK(w) for (w = _z_back_window; w != NULL; w = w->z_front) -#define FOR_ALL_WINDOWS_FROM_FRONT(w) for (w = _z_front_window; w != NULL; w = w->z_back) +#define FOR_ALL_WINDOWS_FROM_BACK_FROM(w, start) for (w = start; w != NULL; w = w->z_front) if (w->window_class != WC_INVALID) +#define FOR_ALL_WINDOWS_FROM_FRONT_FROM(w, start) for (w = start; w != NULL; w = w->z_back) if (w->window_class != WC_INVALID) +#define FOR_ALL_WINDOWS_FROM_BACK(w) FOR_ALL_WINDOWS_FROM_BACK_FROM(w, _z_back_window) +#define FOR_ALL_WINDOWS_FROM_FRONT(w) FOR_ALL_WINDOWS_FROM_FRONT_FROM(w, _z_front_window) /** * Disable scrolling of the main viewport when an input-window is active. -- cgit v1.2.3-54-g00ecf