Clean up preference saving code

The old code (on purpose) didn't try to differentiate "nonexisting
boolean configuration" with "existing boolean configuration set to
false", which is problematic if we optimize the saving to not save
default preferences at all.

Which this does.

So in addition to the logic to know about default preferences, this has
to change the interfaces for the PREF_BOOL reading code so that you can
tell the difference between "no value" and "false".

And since the previous calling convention was an abomination of doing
pointer casting and having case-statements for the config types, change
that while at it.  Both from a usage perspective *and* from a back-end
perspective it is actually much simpler to just have different functions
for the string vs boolean config read/write versions.  The OSX versions
in particular end up being one-liners.

(The GConf library is a nightmare, and doesn't seem to have any way to
know whether a boolean value exists or not, so you have to read it as a
GConfVal and then turn it into a gboolean rather than just get the "oh,
it didn't exist" as an error value).

Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Dirk Hohndel <dirk@hohndel.org>
This commit is contained in:
Linus Torvalds 2013-01-10 19:33:53 -08:00 committed by Dirk Hohndel
parent 21f38190d3
commit 8dea49ffe2
5 changed files with 204 additions and 172 deletions

162
gtk-gui.c
View file

@ -76,8 +76,8 @@ static void set_default_dive_computer(const char *vendor, const char *product)
free((void *)default_dive_computer_product);
default_dive_computer_vendor = strdup(vendor);
default_dive_computer_product = strdup(product);
subsurface_set_conf("dive_computer_vendor", PREF_STRING, vendor);
subsurface_set_conf("dive_computer_product", PREF_STRING, product);
subsurface_set_conf("dive_computer_vendor", vendor);
subsurface_set_conf("dive_computer_product", product);
}
static void set_default_dive_computer_device(const char *name)
@ -89,7 +89,7 @@ static void set_default_dive_computer_device(const char *name)
if (default_dive_computer_device)
free((void *)default_dive_computer_device);
default_dive_computer_device = strdup(name);
subsurface_set_conf("dive_computer_device", PREF_STRING, name);
subsurface_set_conf("dive_computer_device", name);
}
void repaint_dive(void)
@ -593,6 +593,19 @@ static void pick_default_file(GtkWidget *w, GtkButton *button)
gtk_widget_set_sensitive(parent, TRUE);
}
static void set_bool_conf(char *name, gboolean value, gboolean def)
{
if (value == def) {
subsurface_unset_conf(name);
return;
}
subsurface_set_conf_bool(name, value);
}
#define __SAVE_BOOLEAN(name, field, value) \
set_bool_conf(name, prefs.field == value, default_prefs.field == value)
#define SAVE_UNIT(name, field, value) __SAVE_BOOLEAN(name, units.field, value)
#define SAVE_BOOL(name, field) __SAVE_BOOLEAN(name, field, TRUE)
static void preferences_dialog(GtkWidget *w, gpointer data)
{
int result;
@ -858,34 +871,39 @@ static void preferences_dialog(GtkWidget *w, gpointer data)
update_screen();
subsurface_set_conf("feet", PREF_BOOL, BOOL_TO_PTR(prefs.units.length == FEET));
subsurface_set_conf("psi", PREF_BOOL, BOOL_TO_PTR(prefs.units.pressure == PSI));
subsurface_set_conf("cuft", PREF_BOOL, BOOL_TO_PTR(prefs.units.volume == CUFT));
subsurface_set_conf("fahrenheit", PREF_BOOL, BOOL_TO_PTR(prefs.units.temperature == FAHRENHEIT));
subsurface_set_conf("lbs", PREF_BOOL, BOOL_TO_PTR(prefs.units.weight == LBS));
SAVE_UNIT("feet", length, FEET);
SAVE_UNIT("psi", pressure, PSI);
SAVE_UNIT("cuft", volume, CUFT);
SAVE_UNIT("fahrenheit", temperature, FAHRENHEIT);
SAVE_UNIT("lbs", weight, LBS);
subsurface_set_conf("TEMPERATURE", PREF_BOOL, BOOL_TO_PTR(prefs.visible_cols.temperature));
subsurface_set_conf("TOTALWEIGHT", PREF_BOOL, BOOL_TO_PTR(prefs.visible_cols.totalweight));
subsurface_set_conf("SUIT", PREF_BOOL, BOOL_TO_PTR(prefs.visible_cols.suit));
subsurface_set_conf("CYLINDER", PREF_BOOL, BOOL_TO_PTR(prefs.visible_cols.cylinder));
subsurface_set_conf("NITROX", PREF_BOOL, BOOL_TO_PTR(prefs.visible_cols.nitrox));
subsurface_set_conf("SAC", PREF_BOOL, BOOL_TO_PTR(prefs.visible_cols.sac));
subsurface_set_conf("OTU", PREF_BOOL, BOOL_TO_PTR(prefs.visible_cols.otu));
subsurface_set_conf("MAXCNS", PREF_BOOL, BOOL_TO_PTR(prefs.visible_cols.maxcns));
SAVE_BOOL("TEMPERATURE", visible_cols.temperature);
SAVE_BOOL("TOTALWEIGHT", visible_cols.totalweight);
SAVE_BOOL("SUIT", visible_cols.suit);
SAVE_BOOL("CYLINDER", visible_cols.cylinder);
SAVE_BOOL("NITROX", visible_cols.nitrox);
SAVE_BOOL("SAC", visible_cols.sac);
SAVE_BOOL("OTU", visible_cols.otu);
SAVE_BOOL("MAXCNS", visible_cols.maxcns);
subsurface_set_conf("divelist_font", PREF_STRING, divelist_font);
subsurface_set_conf("divelist_font", divelist_font);
subsurface_set_conf("po2graph", PREF_BOOL, BOOL_TO_PTR(prefs.pp_graphs.po2));
subsurface_set_conf("pn2graph", PREF_BOOL, BOOL_TO_PTR(prefs.pp_graphs.pn2));
subsurface_set_conf("phegraph", PREF_BOOL, BOOL_TO_PTR(prefs.pp_graphs.phe));
subsurface_set_conf("po2threshold", PREF_STRING, po2_threshold_text);
subsurface_set_conf("pn2threshold", PREF_STRING, pn2_threshold_text);
subsurface_set_conf("phethreshold", PREF_STRING, phe_threshold_text);
subsurface_set_conf("redceiling", PREF_BOOL, BOOL_TO_PTR(prefs.profile_red_ceiling));
subsurface_set_conf("calcceiling", PREF_BOOL, BOOL_TO_PTR(prefs.profile_calc_ceiling));
subsurface_set_conf("calcceiling3m", PREF_BOOL, BOOL_TO_PTR(prefs.calc_ceiling_3m_incr));
subsurface_set_conf("gflow", PREF_STRING, gflow_text);
subsurface_set_conf("gfhigh", PREF_STRING, gfhigh_text);
SAVE_BOOL("po2graph", pp_graphs.po2);
SAVE_BOOL("pn2graph", pp_graphs.pn2);
SAVE_BOOL("phegraph", pp_graphs.phe);
/* Fixme! Only save if different-from-default. unset if default */
subsurface_set_conf("po2threshold", po2_threshold_text);
subsurface_set_conf("pn2threshold", pn2_threshold_text);
subsurface_set_conf("phethreshold", phe_threshold_text);
SAVE_BOOL("redceiling", profile_red_ceiling);
SAVE_BOOL("calcceiling", profile_calc_ceiling);
SAVE_BOOL("calcceiling3m", calc_ceiling_3m_incr);
/* Fixme! Only save if different-from-default. unset if default */
subsurface_set_conf("gflow", gflow_text);
subsurface_set_conf("gfhigh", gfhigh_text);
new_default = strdup(gtk_button_get_label(GTK_BUTTON(xmlfile_button)));
@ -899,7 +917,7 @@ static void preferences_dialog(GtkWidget *w, gpointer data)
}
if (strcmp(current_default, new_default)) {
subsurface_set_conf("default_filename", PREF_STRING, new_default);
subsurface_set_conf("default_filename", new_default);
free((void *)default_filename);
default_filename = new_default;
}
@ -1242,6 +1260,18 @@ static gboolean on_key_press(GtkWidget *w, GdkEventKey *event, GtkWidget *diveli
return FALSE;
}
static gboolean get_bool(char *name, gboolean def)
{
int val = subsurface_get_conf_bool(name);
if (val < 0)
return def;
return val;
}
#define GET_UNIT(name, field, f, t) \
prefs.units.field = get_bool(name, default_prefs.units.field) ? (t) : (f)
#define GET_BOOL(name, field) \
prefs.field = get_bool(name, default_prefs.field)
void init_ui(int *argcp, char ***argvp)
{
GtkWidget *win;
@ -1274,68 +1304,66 @@ void init_ui(int *argcp, char ***argvp)
g_type_init();
subsurface_open_conf();
if (subsurface_get_conf("feet", PREF_BOOL))
prefs.units.length = FEET;
if (subsurface_get_conf("psi", PREF_BOOL))
prefs.units.pressure = PSI;
if (subsurface_get_conf("cuft", PREF_BOOL))
prefs.units.volume = CUFT;
if (subsurface_get_conf("fahrenheit", PREF_BOOL))
prefs.units.temperature = FAHRENHEIT;
if (subsurface_get_conf("lbs", PREF_BOOL))
prefs.units.weight = LBS;
/* an unset key is FALSE - all these are hidden by default */
prefs.visible_cols.cylinder = PTR_TO_BOOL(subsurface_get_conf("CYLINDER", PREF_BOOL));
prefs.visible_cols.temperature = PTR_TO_BOOL(subsurface_get_conf("TEMPERATURE", PREF_BOOL));
prefs.visible_cols.totalweight = PTR_TO_BOOL(subsurface_get_conf("TOTALWEIGHT", PREF_BOOL));
prefs.visible_cols.suit = PTR_TO_BOOL(subsurface_get_conf("SUIT", PREF_BOOL));
prefs.visible_cols.nitrox = PTR_TO_BOOL(subsurface_get_conf("NITROX", PREF_BOOL));
prefs.visible_cols.otu = PTR_TO_BOOL(subsurface_get_conf("OTU", PREF_BOOL));
prefs.visible_cols.maxcns = PTR_TO_BOOL(subsurface_get_conf("MAXCNS", PREF_BOOL));
prefs.visible_cols.sac = PTR_TO_BOOL(subsurface_get_conf("SAC", PREF_BOOL));
prefs.pp_graphs.po2 = PTR_TO_BOOL(subsurface_get_conf("po2graph", PREF_BOOL));
prefs.pp_graphs.pn2 = PTR_TO_BOOL(subsurface_get_conf("pn2graph", PREF_BOOL));
prefs.pp_graphs.phe = PTR_TO_BOOL(subsurface_get_conf("phegraph", PREF_BOOL));
conf_value = subsurface_get_conf("po2threshold", PREF_STRING);
GET_UNIT("feet", length, METERS, FEET);
GET_UNIT("psi", pressure, BAR, PSI);
GET_UNIT("cuft", volume, LITER, CUFT);
GET_UNIT("fahrenheit", temperature, CELSIUS, FAHRENHEIT);
GET_UNIT("lbs", weight, KG, LBS);
/* an unset key is 'default' */
GET_BOOL("CYLINDER", visible_cols.cylinder);
GET_BOOL("TEMPERATURE", visible_cols.temperature);
GET_BOOL("TOTALWEIGHT", visible_cols.totalweight);
GET_BOOL("SUIT", visible_cols.suit);
GET_BOOL("NITROX", visible_cols.nitrox);
GET_BOOL("OTU", visible_cols.otu);
GET_BOOL("MAXCNS", visible_cols.maxcns);
GET_BOOL("SAC", visible_cols.sac);
GET_BOOL("po2graph", pp_graphs.po2);
GET_BOOL("pn2graph", pp_graphs.pn2);
GET_BOOL("phegraph", pp_graphs.phe);
conf_value = subsurface_get_conf("po2threshold");
if (conf_value) {
sscanf(conf_value, "%lf", &prefs.pp_graphs.po2_threshold);
free((void *)conf_value);
}
conf_value = subsurface_get_conf("pn2threshold", PREF_STRING);
conf_value = subsurface_get_conf("pn2threshold");
if (conf_value) {
sscanf(conf_value, "%lf", &prefs.pp_graphs.pn2_threshold);
free((void *)conf_value);
}
conf_value = subsurface_get_conf("phethreshold", PREF_STRING);
conf_value = subsurface_get_conf("phethreshold");
if (conf_value) {
sscanf(conf_value, "%lf", &prefs.pp_graphs.phe_threshold);
free((void *)conf_value);
}
prefs.profile_red_ceiling = PTR_TO_BOOL(subsurface_get_conf("redceiling", PREF_BOOL));
prefs.profile_calc_ceiling = PTR_TO_BOOL(subsurface_get_conf("calcceiling", PREF_BOOL));
prefs.calc_ceiling_3m_incr = PTR_TO_BOOL(subsurface_get_conf("calcceiling3m", PREF_BOOL));
conf_value = subsurface_get_conf("gflow", PREF_STRING);
GET_BOOL("redceiling", profile_red_ceiling);
GET_BOOL("calcceiling", profile_calc_ceiling);
GET_BOOL("calcceiling3m", calc_ceiling_3m_incr);
conf_value = subsurface_get_conf("gflow");
if (conf_value) {
sscanf(conf_value, "%lf", &prefs.gflow);
prefs.gflow /= 100.0;
set_gf(prefs.gflow, -1.0);
free((void *)conf_value);
}
conf_value = subsurface_get_conf("gfhigh", PREF_STRING);
conf_value = subsurface_get_conf("gfhigh");
if (conf_value) {
sscanf(conf_value, "%lf", &prefs.gfhigh);
prefs.gfhigh /= 100.0;
set_gf(-1.0, prefs.gfhigh);
free((void *)conf_value);
}
divelist_font = subsurface_get_conf("divelist_font", PREF_STRING);
divelist_font = subsurface_get_conf("divelist_font");
default_filename = subsurface_get_conf("default_filename", PREF_STRING);
default_filename = subsurface_get_conf("default_filename");
default_dive_computer_vendor = subsurface_get_conf("dive_computer_vendor", PREF_STRING);
default_dive_computer_product = subsurface_get_conf("dive_computer_product", PREF_STRING);
default_dive_computer_device = subsurface_get_conf("dive_computer_device", PREF_STRING);
conf_value = subsurface_get_conf("dc_nicknames", PREF_STRING);
default_dive_computer_vendor = subsurface_get_conf("dive_computer_vendor");
default_dive_computer_product = subsurface_get_conf("dive_computer_product");
default_dive_computer_device = subsurface_get_conf("dive_computer_device");
conf_value = subsurface_get_conf("dc_nicknames");
nicknamestring = strdup("");
if (conf_value) {
char *next_token, *nickname, *model, *conf_copy;
@ -2288,7 +2316,7 @@ void remove_dc(const char *model, uint32_t deviceid, gboolean change_conf)
memmove(nnstring, brace, strlen(brace) + 1);
if (change_conf)
subsurface_set_conf("dc_nicknames", PREF_STRING, nicknamestring);
subsurface_set_conf("dc_nicknames", nicknamestring);
bail:
free(entry);
@ -2319,7 +2347,7 @@ void remember_dc(const char *model, uint32_t deviceid, const char *nickname, gbo
replace_nickname_nicknamestring(model, deviceid, nickname);
}
if (change_conf)
subsurface_set_conf("dc_nicknames", PREF_STRING, nicknamestring);
subsurface_set_conf("dc_nicknames", nicknamestring);
}
void set_dc_nickname(struct dive *dive)

35
linux.c
View file

@ -28,27 +28,32 @@ void subsurface_unset_conf(char *name)
gconf_client_unset(gconf, gconf_name(name), NULL);
}
void subsurface_set_conf(char *name, pref_type_t type, const void *value)
void subsurface_set_conf(char *name, const char *value)
{
switch (type) {
case PREF_BOOL:
gconf_client_set_bool(gconf, gconf_name(name), value != NULL, NULL);
break;
case PREF_STRING:
gconf_client_set_string(gconf, gconf_name(name), value, NULL);
}
}
const void *subsurface_get_conf(char *name, pref_type_t type)
void subsurface_set_conf_bool(char *name, int value)
{
gconf_client_set_bool(gconf, gconf_name(name), value > 0, NULL);
}
const void *subsurface_get_conf(char *name)
{
switch (type) {
case PREF_BOOL:
return gconf_client_get_bool(gconf, gconf_name(name), NULL) ? (void *) 1 : NULL;
case PREF_STRING:
return gconf_client_get_string(gconf, gconf_name(name), NULL);
}
/* we shouldn't get here */
return NULL;
}
int subsurface_get_conf_bool(char *name)
{
GConfValue *val;
gboolean ret;
val = gconf_client_get(gconf, gconf_name(name), NULL);
if (!val)
return -1;
ret = gconf_value_get_bool(val);
gconf_value_free(val);
return ret;
}
void subsurface_flush_conf(void)

40
macos.c
View file

@ -31,38 +31,36 @@ void subsurface_unset_conf(char *name)
CFPreferencesSetAppValue(CFSTR_VAR(name), NULL, SUBSURFACE_PREFERENCES);
}
void subsurface_set_conf(char *name, pref_type_t type, const void *value)
void subsurface_set_conf(char *name, const void *value)
{
switch (type) {
case PREF_BOOL:
CFPreferencesSetAppValue(CFSTR_VAR(name),
value == NULL ? kCFBooleanFalse : kCFBooleanTrue, SUBSURFACE_PREFERENCES);
break;
case PREF_STRING:
CFPreferencesSetAppValue(CFSTR_VAR(name), CFSTR_VAR(value), SUBSURFACE_PREFERENCES);
}
}
const void *subsurface_get_conf(char *name, pref_type_t type)
void subsurface_set_conf(char *name, int value)
{
CFPreferencesSetAppValue(CFSTR_VAR(name),
value ? kCFBooleanTrue : kCFBooleanFalse, SUBSURFACE_PREFERENCES);
}
const void *subsurface_get_conf(char *name)
{
Boolean boolpref;
CFPropertyListRef strpref;
switch (type) {
case PREF_BOOL:
boolpref = CFPreferencesGetAppBooleanValue(CFSTR_VAR(name), SUBSURFACE_PREFERENCES, FALSE);
if (boolpref)
return (void *) 1;
else
return NULL;
case PREF_STRING:
strpref = CFPreferencesCopyAppValue(CFSTR_VAR(name), SUBSURFACE_PREFERENCES);
if (!strpref)
return NULL;
return strdup(CFStringGetCStringPtr(strpref, kCFStringEncodingMacRoman));
}
/* we shouldn't get here, but having this line makes the compiler happy */
return NULL;
}
int subsurface_get_conf_bool(char *name)
{
Boolean boolpref, exists;
CFPropertyListRef strpref;
boolpref = CFPreferencesGetAppBooleanValue(CFSTR_VAR(name), SUBSURFACE_PREFERENCES, &exists);
if (!exists)
return -1;
return boolpref;
}
void subsurface_flush_conf(void)

14
pref.h
View file

@ -36,18 +36,12 @@ extern struct preferences prefs, default_prefs;
#define PP_GRAPHS_ENABLED (prefs.pp_graphs.po2 || prefs.pp_graphs.pn2 || prefs.pp_graphs.phe)
typedef enum {
PREF_BOOL,
PREF_STRING
} pref_type_t;
#define BOOL_TO_PTR(_cond) ((_cond) ? (void *)1 : NULL)
#define PTR_TO_BOOL(_ptr) ((_ptr) != NULL)
extern void subsurface_open_conf(void);
extern void subsurface_set_conf(char *name, pref_type_t type, const void *value);
extern void subsurface_set_conf(char *name, const char *value);
extern void subsurface_set_conf_bool(char *name, gboolean value);
extern void subsurface_unset_conf(char *name);
extern const void *subsurface_get_conf(char *name, pref_type_t type);
extern const void *subsurface_get_conf(char *name);
extern int subsurface_get_conf_bool(char *name);
extern void subsurface_flush_conf(void);
extern void subsurface_close_conf(void);

View file

@ -8,6 +8,7 @@
static HKEY hkey;
/* Return "boolean" 0/1, or -1 if nonexistent */
static int get_from_registry(HKEY hkey, const char *key)
{
DWORD value;
@ -17,8 +18,8 @@ static int get_from_registry(HKEY hkey, const char *key)
success = RegQueryValueEx(hkey, (LPCTSTR)TEXT(key), NULL, NULL,
(LPBYTE) &value, (LPDWORD)&len );
if (success != ERROR_SUCCESS)
return FALSE; /* that's what happens the first time we start */
return value;
return -1;
return value != 0;
}
void subsurface_open_conf(void)
@ -42,7 +43,7 @@ void subsurface_unset_conf(char *name)
RegDeleteKey(hkey, (LPCWSTR)wname);
}
void subsurface_set_conf(char *name, pref_type_t type, const void *value)
void subsurface_set_conf(char *name, const void *value)
{
/* since we are using the pointer 'value' as both an actual
* pointer to the string setting and as a way to pass the
@ -56,12 +57,7 @@ void subsurface_set_conf(char *name, pref_type_t type, const void *value)
wname = (wchar_t *)g_utf8_to_utf16(name, -1, NULL, NULL, NULL);
if (!wname)
return;
switch (type) {
case PREF_BOOL:
/* we simply store the value as DWORD */
RegSetValueExW(hkey, (LPCWSTR)wname, 0, REG_DWORD, (const BYTE *)&value, 4);
break;
case PREF_STRING:
wlen = g_utf8_strlen((char *)value, -1);
wstring = (wchar_t *)g_utf8_to_utf16((char *)value, -1, NULL, NULL, NULL);
if (!wstring || !wlen) {
@ -71,11 +67,23 @@ void subsurface_set_conf(char *name, pref_type_t type, const void *value)
RegSetValueExW(hkey, (LPCWSTR)wname, 0, REG_SZ, (const BYTE *)wstring,
wlen * sizeof(wchar_t));
free(wstring);
}
free(wname);
}
const void *subsurface_get_conf(char *name, pref_type_t type)
void subsurface_set_conf_bool(char *name, int value)
{
wchar_t *wname = NULL;
wname = (wchar_t *)g_utf8_to_utf16(name, -1, NULL, NULL, NULL);
if (!wname)
return;
/* we simply store the value as DWORD */
RegSetValueExW(hkey, (LPCWSTR)wname, 0, REG_DWORD, (const BYTE *)&value, 4);
free(wname);
}
const void *subsurface_get_conf(char *name)
{
const int csize = 64;
int blen = 0;
@ -83,10 +91,6 @@ const void *subsurface_get_conf(char *name, pref_type_t type)
wchar_t *wstring = NULL, *wname = NULL;
char *utf8_string;
switch (type) {
case PREF_BOOL:
return get_from_registry(hkey, name) ? (void *) 1 : NULL;
case PREF_STRING:
wname = (wchar_t *)g_utf8_to_utf16(name, -1, NULL, NULL, NULL);
if (!wname)
return NULL;
@ -115,8 +119,11 @@ const void *subsurface_get_conf(char *name, pref_type_t type)
return NULL;
}
return utf8_string;
}
return NULL; /* we shouldn't get here */
}
int subsurface_get_conf_bool(char *name)
{
return get_from_registry(hkey, name);
}
void subsurface_flush_conf(void)