From: Andrej Kacian <ticho@claws-mail.org>
Date: Sun, 9 Jul 2017 16:27:20 +0000 (+0200)
Subject: Fix a long-standing memory leak in account prefs.
X-Git-Tag: 3.16.0~124
X-Git-Url: http://git.claws-mail.org/?p=claws.git;a=commitdiff_plain;h=8c3760e3461510a1889195d2d027ebd4db6ca5bd

Fix a long-standing memory leak in account prefs.

All P_STRING type prefs with non-NULL default values
were being leaked due to overly complicated account
config loading, where we would first initialize
a fresh PrefAccount, then load prefs from config, copying
data between the static tmp_ac_prefs and our dynamically
allocated PrefAccount several times.

Instead of adding another round of data copying from/to
tmp_ac_prefs for purpose of free()ing the P_STRING prefs,
this commit changes prefs_account_read_config() into
prefs_account_new_from_config(), a
Existing accounts are now created with simple:
  ac_prefs = prefs_account_new_from_config(...);
instead of previous:
  ac_prefs = prefs_account_new();
  prefs_account_read_from_config(ac_prefs, ...);

The new function sets up contents of tmp_ac_prefs,
and then its contents are only copied to the newly
allocated PrefsAccount once.
---

diff --git a/src/account.c b/src/account.c
index 962505682..a152ef690 100644
--- a/src/account.c
+++ b/src/account.c
@@ -223,8 +223,7 @@ void account_read_config_all(void)
 	/* read config data from file */
 	cur_account = NULL;
 	for (cur = ac_label_list; cur != NULL; cur = cur->next) {
-		ac_prefs = prefs_account_new();
-		prefs_account_read_config(ac_prefs, (gchar *)cur->data);
+		ac_prefs = prefs_account_new_from_config((gchar *)cur->data);
 		account_list = g_list_append(account_list, ac_prefs);
 		if (ac_prefs->is_default)
 			cur_account = ac_prefs;
diff --git a/src/prefs_account.c b/src/prefs_account.c
index 589d63d1d..513ba6bfa 100644
--- a/src/prefs_account.c
+++ b/src/prefs_account.c
@@ -3574,20 +3574,31 @@ PrefsAccount *prefs_account_new(void)
 	return ac_prefs;
 }
 
-void prefs_account_read_config(PrefsAccount *ac_prefs, const gchar *label)
+PrefsAccount *prefs_account_new_from_config(const gchar *label)
 {
 	const gchar *p = label;
 	gchar *rcpath;
 	gint id;
 	gchar **strv, **cur;
 	gsize len;
+	PrefsAccount *ac_prefs;
+
+	cm_return_val_if_fail(label != NULL, NULL);
 
-	cm_return_if_fail(ac_prefs != NULL);
-	cm_return_if_fail(label != NULL);
+	ac_prefs = g_new0(PrefsAccount, 1);
 
+	/* Load default values to tmp_ac_prefs first, ... */
 	memset(&tmp_ac_prefs, 0, sizeof(PrefsAccount));
-	tmp_ac_prefs.privacy_prefs = ac_prefs->privacy_prefs;
+	prefs_set_default(basic_param);
+	prefs_set_default(receive_param);
+	prefs_set_default(send_param);
+	prefs_set_default(compose_param);
+	prefs_set_default(templates_param);
+	prefs_set_default(privacy_param);
+	prefs_set_default(ssl_param);
+	prefs_set_default(advanced_param);
 
+	/* ... overriding them with values from stored config file. */
 	rcpath = g_strconcat(get_rc_dir(), G_DIR_SEPARATOR_S, ACCOUNT_RC, NULL);
 	prefs_read_config(basic_param, label, rcpath, NULL);
 	prefs_read_config(receive_param, label, rcpath, NULL);
@@ -3600,11 +3611,14 @@ void prefs_account_read_config(PrefsAccount *ac_prefs, const gchar *label)
 	g_free(rcpath);
 
 	*ac_prefs = tmp_ac_prefs;
+
 	while (*p && !g_ascii_isdigit(*p)) p++;
 	id = atoi(p);
 	if (id < 0) g_warning("wrong account id: %d", id);
 	ac_prefs->account_id = id;
 
+	/* Now parse privacy_prefs. */
+	ac_prefs->privacy_prefs = g_hash_table_new(g_str_hash, g_str_equal);
 	if (privacy_prefs != NULL) {
 		strv = g_strsplit(privacy_prefs, ",", 0);
 		for (cur = strv; *cur != NULL; cur++) {
@@ -3627,6 +3641,8 @@ void prefs_account_read_config(PrefsAccount *ac_prefs, const gchar *label)
 		privacy_prefs = NULL;
 	}
 
+	/* For older configurations, move stored passwords into the
+	 * password store. */
 	gboolean passwords_migrated = FALSE;
 
 	if (ac_prefs->passwd != NULL && strlen(ac_prefs->passwd) > 1) {
@@ -3652,14 +3668,16 @@ void prefs_account_read_config(PrefsAccount *ac_prefs, const gchar *label)
 		passwords_migrated = TRUE;
 	}
 
-	/* Write out password store to file immediately after their move
-	 * from accountrc there. */
+	/* Write out password store to file immediately, to prevent
+	 * their loss. */
 	if (passwords_migrated)
 		passwd_store_write_config();
 
 	ac_prefs->receive_in_progress = FALSE;
 
 	prefs_custom_header_read_config(ac_prefs);
+
+	return ac_prefs;
 }
 
 static void create_privacy_prefs(gpointer key, gpointer _value, gpointer user_data)
diff --git a/src/prefs_account.h b/src/prefs_account.h
index 83f6efad7..fd4dc1a61 100644
--- a/src/prefs_account.h
+++ b/src/prefs_account.h
@@ -211,10 +211,9 @@ struct _PrefsAccount
 
 void prefs_account_init			(void);
 
-PrefsAccount *prefs_account_new		(void);
+PrefsAccount *prefs_account_new			(void);
+PrefsAccount *prefs_account_new_from_config	(const gchar	*label);
 
-void prefs_account_read_config		(PrefsAccount	*ac_prefs,
-					 const gchar	*label);
 void prefs_account_write_config_all	(GList		*account_list);
 
 void prefs_account_free			(PrefsAccount	*ac_prefs);