[Scummvm-git-logs] scummvm master -> 74b3b45c61a41300c2e84201f813817eada27351

criezy criezy at scummvm.org
Sat Oct 22 22:33:30 CEST 2016


This automated email contains information about 2 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
9f94294d80 GUI: Fix incorrect initialisation of some tab Ids in OptionsDialog
74b3b45c61 GUI: Fix possible access to free'ed memory or double deletion in tab widget


Commit: 9f94294d80e904a7d7af51bdf8fe8fb9456b6832
    https://github.com/scummvm/scummvm/commit/9f94294d80e904a7d7af51bdf8fe8fb9456b6832
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2016-10-22T21:32:16+01:00

Commit Message:
GUI: Fix incorrect initialisation of some tab Ids in OptionsDialog

A value of 0 is valid for tab ids, so the correct initialisation at this
stage is -1. However only one constructor properly initialized all the
tab ids to -1 in its initialisation list, but it was then changed to 0 in
init(). I have added the missing ones to the other constructors and
removed the incorrect ones in init(). But maybe all tab ids should be
initialised in init() rather than in the constructors initialisation lists.

Changed paths:
    gui/options.cpp



diff --git a/gui/options.cpp b/gui/options.cpp
index 4ebb60b..3701369 100644
--- a/gui/options.cpp
+++ b/gui/options.cpp
@@ -122,7 +122,7 @@ OptionsDialog::OptionsDialog(const Common::String &domain, int x, int y, int w,
 }
 
 OptionsDialog::OptionsDialog(const Common::String &domain, const Common::String &name)
-	: Dialog(name), _domain(domain), _graphicsTabId(-1), _tabWidget(0) {
+	: Dialog(name), _domain(domain), _graphicsTabId(-1), _midiTabId(-1), _pathsTabId(-1), _tabWidget(0) {
 	init();
 }
 
@@ -140,7 +140,6 @@ void OptionsDialog::init() {
 	_filteringCheckbox = 0;
 	_aspectCheckbox = 0;
 	_enableAudioSettings = false;
-	_midiTabId = 0;
 	_midiPopUp = 0;
 	_midiPopUpDesc = 0;
 	_oplPopUp = 0;
@@ -183,7 +182,6 @@ void OptionsDialog::init() {
 	_subSpeedSlider = 0;
 	_subSpeedLabel = 0;
 
-	_pathsTabId = 0;
 	_oldTheme = g_gui.theme()->getThemeId();
 
 	// Retrieve game GUI options


Commit: 74b3b45c61a41300c2e84201f813817eada27351
    https://github.com/scummvm/scummvm/commit/74b3b45c61a41300c2e84201f813817eada27351
Author: Thierry Crozat (criezy at scummvm.org)
Date: 2016-10-22T21:32:16+01:00

Commit Message:
GUI: Fix possible access to free'ed memory or double deletion in tab widget

The issue could occur when adding or removing widgets to a tab, and then
not switching to a different tab before the destructor or reflowLayout() were
called. In such a case the firstWidget of the current widget in the _tabs list
could be out of date. Accessing this first widget from the destructor or from
reflowLayout() could then cause a crash, or random issues caused to access
to free'ed memory. In theory this could also lead to a memory leak, although
I don't think this could occur in our current code.

Usually we add several tabs to a TabWidget and then switch back to the first
tab after building all the tabs. So in such a case the issue would not occur.
But because we are deleting and reconstructing the clear buttons for the
MIDI and Path tabs of the options dialog from reflowLayout(), if the current
tab is the Path tab, it would be kept as active tab after adding and removing
widget to it and the issue would occur.

This fixes bug #9618.

Changed paths:
    gui/widgets/tab.cpp



diff --git a/gui/widgets/tab.cpp b/gui/widgets/tab.cpp
index 15e6a9d..ed261c9 100644
--- a/gui/widgets/tab.cpp
+++ b/gui/widgets/tab.cpp
@@ -70,6 +70,12 @@ void TabWidget::init() {
 }
 
 TabWidget::~TabWidget() {
+	// If widgets were added or removed in the current tab, without tabs
+	// having been switched using setActiveTab() afterward, then the
+	// firstWidget in the _tabs list for the active tab may not be up to
+	// date. So update it now.
+	if (_activeTab != -1)
+		_tabs[_activeTab].firstWidget = _firstWidget;
 	_firstWidget = 0;
 	for (uint i = 0; i < _tabs.size(); ++i) {
 		delete _tabs[i].firstWidget;
@@ -274,6 +280,13 @@ void TabWidget::reflowLayout() {
 	_tabWidth = g_gui.xmlEval()->getVar("Globals.TabWidget.Tab.Width");
 	_titleVPad = g_gui.xmlEval()->getVar("Globals.TabWidget.Tab.Padding.Top");
 
+	// If widgets were added or removed in the current tab, without tabs
+	// having been switched using setActiveTab() afterward, then the
+	// firstWidget in the _tabs list for the active tab may not be up to
+	// date. So update it now.
+	if (_activeTab != -1)
+		_tabs[_activeTab].firstWidget = _firstWidget;
+
 	for (uint i = 0; i < _tabs.size(); ++i) {
 		Widget *w = _tabs[i].firstWidget;
 		while (w) {





More information about the Scummvm-git-logs mailing list