From 9d8b0addf94c86f224093bb1ca66ebe18494ff7c Mon Sep 17 00:00:00 2001 From: Dirk Hohndel Date: Tue, 6 Oct 2015 10:15:38 +0100 Subject: [PATCH] Correctly copy preferences When just assigning one structure to the other we copy the string pointers. If we then modify those strings in the copy, we happily free the strings of the original. And then resetting the preferences equally happily reused those strings, pointing to long since freed memory. I think what I did now is excessive for the current use case in that it copies a ton of strings that are unset in the default_prefs. But I figured this is a rarely used function and I might as well do it correctly. Also, once we implement multi user support with per user preferences we will be copying completely populated preferences around (at least that's my guess). Fixes #940 Signed-off-by: Dirk Hohndel --- export-html.cpp | 2 +- main.cpp | 2 +- qt-ui/preferences.cpp | 3 ++- subsurfacestartup.c | 22 ++++++++++++++++++++++ subsurfacestartup.h | 1 + tests/testgitstorage.cpp | 3 ++- tests/testplan.cpp | 4 ++-- 7 files changed, 31 insertions(+), 6 deletions(-) diff --git a/export-html.cpp b/export-html.cpp index da570f4b0..940cb60df 100644 --- a/export-html.cpp +++ b/export-html.cpp @@ -18,7 +18,7 @@ int main(int argc, char **argv) { QApplication *application = new QApplication(argc, argv); git_libgit2_init(); - prefs = default_prefs; + copy_prefs(&default_prefs, &prefs); init_qt_late(); QCommandLineParser parser; diff --git a/main.cpp b/main.cpp index 6c5e16928..7807b20df 100644 --- a/main.cpp +++ b/main.cpp @@ -53,7 +53,7 @@ int main(int argc, char **argv) git_libgit2_init(); #endif setup_system_prefs(); - prefs = default_prefs; + copy_prefs(&default_prefs, &prefs); fill_profile_color(); parse_xml_init(); taglist_init_global(); diff --git a/qt-ui/preferences.cpp b/qt-ui/preferences.cpp index 32f05c4ab..715745e84 100644 --- a/qt-ui/preferences.cpp +++ b/qt-ui/preferences.cpp @@ -4,6 +4,7 @@ #include "divelocationmodel.h" #include "prefs-macros.h" #include "qthelper.h" +#include "subsurfacestartup.h" #include #include @@ -494,7 +495,7 @@ void PreferencesDialog::on_resetSettings_clicked() int result = response.exec(); if (result == QMessageBox::Ok) { - prefs = default_prefs; + copy_prefs(&default_prefs, &prefs); setUiFromPrefs(); Q_FOREACH (QString key, s.allKeys()) { s.remove(key); diff --git a/subsurfacestartup.c b/subsurfacestartup.c index 33aa9fb72..d474ab924 100644 --- a/subsurfacestartup.c +++ b/subsurfacestartup.c @@ -272,6 +272,28 @@ void setup_system_prefs(void) default_prefs.units = IMPERIAL_units; } +/* copy a preferences block, including making copies of all included strings */ +void copy_prefs(struct preferences *src, struct preferences *dest) +{ + *dest = *src; + dest->divelist_font = copy_string(src->divelist_font); + dest->default_filename = copy_string(src->default_filename); + dest->default_cylinder = copy_string(src->default_cylinder); + dest->cloud_base_url = copy_string(src->cloud_base_url); + dest->cloud_git_url = copy_string(src->cloud_git_url); + dest->userid = copy_string(src->userid); + dest->proxy_host = copy_string(src->proxy_host); + dest->proxy_user = copy_string(src->proxy_user); + dest->proxy_pass = copy_string(src->proxy_pass); + dest->cloud_storage_password = copy_string(src->cloud_storage_password); + dest->cloud_storage_newpassword = copy_string(src->cloud_storage_newpassword); + dest->cloud_storage_email = copy_string(src->cloud_storage_email); + dest->cloud_storage_email_encoded = copy_string(src->cloud_storage_email_encoded); + dest->facebook.access_token = copy_string(src->facebook.access_token); + dest->facebook.user_id = copy_string(src->facebook.user_id); + dest->facebook.album_id = copy_string(src->facebook.album_id); +} + /* * Free strduped prefs before exit. * diff --git a/subsurfacestartup.h b/subsurfacestartup.h index 8c70c1fe5..3ccc24aa4 100644 --- a/subsurfacestartup.h +++ b/subsurfacestartup.h @@ -16,6 +16,7 @@ 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); #ifdef __cplusplus diff --git a/tests/testgitstorage.cpp b/tests/testgitstorage.cpp index 1b7c50fdb..ed9465386 100644 --- a/tests/testgitstorage.cpp +++ b/tests/testgitstorage.cpp @@ -5,6 +5,7 @@ #include "git2.h" #include "prefs-macros.h" #include "windowtitleupdate.h" +#include "subsurfacestartup.h" #include #include #include @@ -17,7 +18,7 @@ extern "C" char *get_local_dir(const char *remote, const char *branch); void TestGitStorage::testSetup() { // first, setup the preferences an proxy information - prefs = default_prefs; + copy_prefs(&default_prefs, &prefs); QCoreApplication::setOrganizationName("Subsurface"); QCoreApplication::setOrganizationDomain("subsurface.hohndel.org"); QCoreApplication::setApplicationName("Subsurface"); diff --git a/tests/testplan.cpp b/tests/testplan.cpp index 9818a8360..e73070439 100644 --- a/tests/testplan.cpp +++ b/tests/testplan.cpp @@ -14,7 +14,7 @@ extern pressure_t first_ceiling_pressure; void setupPrefs() { - prefs = default_prefs; + copy_prefs(&default_prefs, &prefs); prefs.ascrate50 = feet_to_mm(30) / 60; prefs.ascrate75 = prefs.ascrate50; prefs.ascratestops = prefs.ascrate50; @@ -24,7 +24,7 @@ void setupPrefs() void setupPrefsVpmb() { - prefs = default_prefs; + copy_prefs(&default_prefs, &prefs); prefs.ascrate50 = 10000 / 60; prefs.ascrate75 = prefs.ascrate50; prefs.ascratestops = prefs.ascrate50;