Re: crashing vpn properties (patch)

[Casey Harkins <caseyharkins gmail com>]

Dan Williams wrote:
> On Mon, 2007-12-10 at 16:23 -0600, Casey Harkins wrote:
> > The crashes when working with vpn properties (not just openvpn) are the 
> > result of using g_free() as the key_destroy_func. The keys being used 
> > for the properties hash table are #define'ed constants and shouldn't be 
> > free'ed. Attached is a one-liner that should fix this.
> >
> > This should resolve RH bug #320941.
>
> Actually, whenever anything stuffs stuff into that table it needs to
> g_strdup() the key.  See update_one_secret() and copy_hash().
> Otherwise, you'll leak the keys when freeing the hash.  So anything that
> uses NMSettingVPNProperties (like the properties dialogs) also need to
> g_strdup() the key.  I probably missed that when fixing up the VPN
> properties hash.

Makes sense. Attached is a patch fixing this for the openvpn properties 
dialog. I'll take a look at the other vpn plugins as well (I know vpnc 
was crashing in my limited testing as well).

-casey

Index: properties/nm-openvpn.c
===================================================================
--- properties/nm-openvpn.c	(revision 3160)
+++ properties/nm-openvpn.c	(working copy)
@@ -297,31 +297,31 @@
 	use_cipher             = gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (impl->w_use_cipher));
 	use_ta                 = gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (impl->w_use_ta));
 
-	g_hash_table_insert (properties, NM_OPENVPN_KEY_CONNECTION_TYPE,
+	g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_CONNECTION_TYPE),
 					 int_to_gvalue (gtk_combo_box_get_active (GTK_COMBO_BOX (impl->w_connection_type))));
 
-	g_hash_table_insert (properties, NM_OPENVPN_KEY_TAP_DEV, bool_to_gvalue (use_tap));
-	g_hash_table_insert (properties, NM_OPENVPN_KEY_REMOTE, str_to_gvalue (remote));
-	g_hash_table_insert (properties, NM_OPENVPN_KEY_PORT, uint_to_gvalue ((guint) atoi (port)));
-	g_hash_table_insert (properties, NM_OPENVPN_KEY_PROTO_TCP, bool_to_gvalue (use_tcp));
-	g_hash_table_insert (properties, NM_OPENVPN_KEY_CA, str_to_gvalue (ca));
-	g_hash_table_insert (properties, NM_OPENVPN_KEY_CERT, str_to_gvalue (cert));
-	g_hash_table_insert (properties, NM_OPENVPN_KEY_KEY, str_to_gvalue (key));
-	g_hash_table_insert (properties, NM_OPENVPN_KEY_COMP_LZO, bool_to_gvalue (use_lzo));
-	g_hash_table_insert (properties, NM_OPENVPN_KEY_SHARED_KEY, str_to_gvalue (shared_key));
-	g_hash_table_insert (properties, NM_OPENVPN_KEY_LOCAL_IP, str_to_gvalue (local_ip));
-	g_hash_table_insert (properties, NM_OPENVPN_KEY_REMOTE_IP, str_to_gvalue (remote_ip));
-	g_hash_table_insert (properties, NM_OPENVPN_KEY_USERNAME, str_to_gvalue (username));
+	g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_TAP_DEV), bool_to_gvalue (use_tap));
+	g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_REMOTE), str_to_gvalue (remote));
+	g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_PORT), uint_to_gvalue ((guint) atoi (port)));
+	g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_PROTO_TCP), bool_to_gvalue (use_tcp));
+	g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_CA), str_to_gvalue (ca));
+	g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_CERT), str_to_gvalue (cert));
+	g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_KEY), str_to_gvalue (key));
+	g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_COMP_LZO), bool_to_gvalue (use_lzo));
+	g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_SHARED_KEY), str_to_gvalue (shared_key));
+	g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_LOCAL_IP), str_to_gvalue (local_ip));
+	g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_REMOTE_IP), str_to_gvalue (remote_ip));
+	g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_USERNAME), str_to_gvalue (username));
 
 	if (use_cipher) {
 		const gchar *cipher = gtk_combo_box_get_active_text (impl->w_cipher);
 		if (cipher != NULL)
-			g_hash_table_insert (properties, NM_OPENVPN_KEY_CIPHER, str_to_gvalue (cipher));
+			g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_CIPHER), str_to_gvalue (cipher));
 	}
 	if (use_ta) {
 		const gchar* dir;
 
-		g_hash_table_insert (properties, NM_OPENVPN_KEY_TA,
+		g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_TA),
 						 str_to_gvalue (gtk_entry_get_text (impl->w_ta)));
 
 		if (gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (impl->w_ta_dir_zero)))
@@ -331,7 +331,7 @@
 		else
 			dir = "";
 
-		g_hash_table_insert (properties, NM_OPENVPN_KEY_TA_DIR, str_to_gvalue (dir));
+		g_hash_table_insert (properties, g_strdup(NM_OPENVPN_KEY_TA_DIR), str_to_gvalue (dir));
 	}
 }