Fix ownership issues in preferences code

Each preferences object owns its string members. In three cases, pointers
were copied instead of strings, leading to (in the best case) dangling
pointers if the user edited values:
1) In the GET_TXT macro in core/prefs-macros.h
2) In the PreferencesDialog::defaultsRequested() method
3) In main() of the mobile version

This patch fixes these issues, by using copy_string() or copy_prefs()
as appropriate.

The only reason that the old code didn't crash regularly is that the
default_prefs object was only used at startup and defaultsRequested()
is (at the moment?) dead code.

This patch also aligns the backslashes in core/pref.h and fixes a typo.
The declaration of copy_prefs() is moved to the core/prefs.h header.

Signed-off-by: Berthold Stoeger <bstoeger@mail.tuwien.ac.at>
This commit is contained in:
Berthold Stoeger 2017-11-25 09:22:19 +01:00 committed by Dirk Hohndel
parent cc5a56b275
commit 4fb01dd766
5 changed files with 9 additions and 9 deletions

View file

@ -189,6 +189,7 @@ extern const char *system_default_directory(void);
extern const char *system_default_filename();
extern bool subsurface_ignore_font(const char *font);
extern void subsurface_OS_pref_setup();
extern void copy_prefs(struct preferences *src, struct preferences *dest);
#ifdef __cplusplus
}

View file

@ -18,10 +18,10 @@
else \
prefs.units.field = default_prefs.units.field
#define GET_UNIT_BOOL(name, field) \
#define GET_UNIT_BOOL(name, field) \
v = s.value(QString(name)); \
if (v.isValid()) \
prefs.units.field = v.toBool(); \
prefs.units.field = v.toBool(); \
else \
prefs.units.field = default_prefs.units.field
@ -53,10 +53,10 @@
else \
prefs.field = default_prefs.field
#define GET_INT_DEF(name, field, defval) \
#define GET_INT_DEF(name, field, defval) \
v = s.value(QString(name)); \
if (v.isValid()) \
prefs.field = v.toInt(); \
prefs.field = v.toInt(); \
else \
prefs.field = defval
@ -65,7 +65,7 @@
if (v.isValid()) \
prefs.field = strdup(v.toString().toUtf8().constData()); \
else \
prefs.field = default_prefs.field
prefs.field = copy_string(default_prefs.field)
#define SAVE_OR_REMOVE_SPECIAL(_setting, _default, _compare, _value) \
if (_compare != _default) \

View file

@ -17,7 +17,6 @@ extern bool imported;
void setup_system_prefs(void);
void parse_argument(const char *arg);
void free_prefs(void);
void copy_prefs(struct preferences *src, struct preferences *dest);
void print_files(void);
void print_version(void);

View file

@ -109,7 +109,7 @@ void PreferencesDialog::refreshPages()
curr->setParent(0);
}
// Readd things.
// Read things
Q_FOREACH(AbstractPreferencesWidget *page, pages) {
QListWidgetItem *item = new QListWidgetItem(page->icon(), page->name());
pagesList->addItem(item);
@ -139,7 +139,7 @@ void PreferencesDialog::cancelRequested()
void PreferencesDialog::defaultsRequested()
{
prefs = default_prefs;
copy_prefs(&default_prefs, &prefs);
Q_FOREACH(AbstractPreferencesWidget *page, pages) {
page->refreshSettings();
}

View file

@ -41,7 +41,7 @@ int main(int argc, char **argv)
setup_system_prefs();
if (uiLanguage(0).contains("-US"))
default_prefs.units = IMPERIAL_units;
prefs = default_prefs;
copy_prefs(&default_prefs, &prefs);
fill_profile_color();
fill_computer_list();