From ef71c5cca1f43b09fe90e52950a176bb4cee2ab2 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 20 May 2011 18:58:35 -0500 Subject: [PATCH] libnm-util: make VPN secret and data iterators change-safe Let callbacks add/remove data items and secrets during iteration. --- libnm-util/nm-setting-vpn.c | 47 +++++++++++++++++++--- libnm-util/tests/test-general.c | 70 +++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 6 deletions(-) diff --git a/libnm-util/nm-setting-vpn.c b/libnm-util/nm-setting-vpn.c index d70ff547c3..f70363eee2 100644 --- a/libnm-util/nm-setting-vpn.c +++ b/libnm-util/nm-setting-vpn.c @@ -161,23 +161,55 @@ nm_setting_vpn_remove_data_item (NMSettingVPN *setting, const char *key) g_hash_table_remove (NM_SETTING_VPN_GET_PRIVATE (setting)->data, key); } +static void +foreach_item_helper (GHashTable *hash, + NMVPNIterFunc func, + gpointer user_data) +{ + GList *keys, *liter; + GSList *copied = NULL, *siter; + + g_return_if_fail (hash != NULL); + + /* Grab keys and copy them so that the callback func can modify + * the hash table items if it wants to. + */ + keys = g_hash_table_get_keys (hash); + for (liter = keys; liter; liter = g_list_next (liter)) + copied = g_slist_prepend (copied, g_strdup (liter->data)); + copied = g_slist_reverse (copied); + g_list_free (keys); + + for (siter = copied; siter; siter = g_slist_next (siter)) { + gpointer value; + + value = g_hash_table_lookup (hash, siter->data); + func (siter->data, value, user_data); + } + + g_slist_foreach (copied, (GFunc) g_free, NULL); + g_slist_free (copied); +} + /** * nm_setting_vpn_foreach_data_item: * @setting: a #NMSettingVPN * @func: (scope call): an user provided function * @user_data: data to be passed to @func * - * Iterates all data items stored in this setting + * Iterates all data items stored in this setting. It is safe to add, remove, + * and modify data items inside @func, though any additions or removals made + * during iteration will not be part of the iteration. */ void nm_setting_vpn_foreach_data_item (NMSettingVPN *setting, NMVPNIterFunc func, gpointer user_data) { + g_return_if_fail (setting != NULL); g_return_if_fail (NM_IS_SETTING_VPN (setting)); - g_hash_table_foreach (NM_SETTING_VPN_GET_PRIVATE (setting)->data, - (GHFunc) func, user_data); + foreach_item_helper (NM_SETTING_VPN_GET_PRIVATE (setting)->data, func, user_data); } void @@ -217,17 +249,19 @@ nm_setting_vpn_remove_secret (NMSettingVPN *setting, const char *key) * @func: (scope call): an user provided function * @user_data: data to be passed to @func * - * Iterates all secrets stored in this setting. + * Iterates all secrets stored in this setting. It is safe to add, remove, + * and modify secrets inside @func, though any additions or removals made during + * iteration will not be part of the iteration. */ void nm_setting_vpn_foreach_secret (NMSettingVPN *setting, NMVPNIterFunc func, gpointer user_data) { + g_return_if_fail (setting != NULL); g_return_if_fail (NM_IS_SETTING_VPN (setting)); - g_hash_table_foreach (NM_SETTING_VPN_GET_PRIVATE (setting)->secrets, - (GHFunc) func, user_data); + foreach_item_helper (NM_SETTING_VPN_GET_PRIVATE (setting)->secrets, func, user_data); } static gboolean @@ -423,6 +457,7 @@ destroy_one_secret (gpointer data) char *secret = (char *) data; /* Don't leave the secret lying around in memory */ +g_message ("%s: destroying %s", __func__, secret); memset (secret, 0, strlen (secret)); g_free (secret); } diff --git a/libnm-util/tests/test-general.c b/libnm-util/tests/test-general.c index 3295c41a2f..87a50d6703 100644 --- a/libnm-util/tests/test-general.c +++ b/libnm-util/tests/test-general.c @@ -195,6 +195,75 @@ test_setting_vpn_update_secrets (void) g_object_unref (connection); } +#define TO_DEL_NUM 50 +typedef struct { + NMSettingVPN *s_vpn; + char *to_del[TO_DEL_NUM]; + guint called; +} IterInfo; + +static void +del_iter_func (const char *key, const char *value, gpointer user_data) +{ + IterInfo *info = user_data; + int i; + + /* Record how many times this function gets called; it should get called + * exactly as many times as there are keys in the hash table, regardless + * of what keys we delete from the table. + */ + info->called++; + + /* During the iteration, remove a bunch of stuff from the table */ + if (info->called == 1) { + for (i = 0; i < TO_DEL_NUM; i++) + nm_setting_vpn_remove_data_item (info->s_vpn, info->to_del[i]); + } +} + +static void +test_setting_vpn_modify_during_foreach (void) +{ + NMSettingVPN *s_vpn; + IterInfo info; + char *key, *val; + int i, u = 0; + + s_vpn = (NMSettingVPN *) nm_setting_vpn_new (); + g_assert (s_vpn); + + for (i = 0; i < TO_DEL_NUM * 2; i++) { + key = g_strdup_printf ("adsfasdfadf%d", i); + val = g_strdup_printf ("42263236236awt%d", i); + nm_setting_vpn_add_data_item (s_vpn, key, val); + + /* Cache some keys to delete */ + if (i % 2) + info.to_del[u++] = g_strdup (key); + + g_free (key); + g_free (val); + } + + /* Iterate over current table keys */ + info.s_vpn = s_vpn; + info.called = 0; + nm_setting_vpn_foreach_data_item (s_vpn, del_iter_func, &info); + + /* Make sure all the things we removed during iteration are really gone */ + for (i = 0; i < TO_DEL_NUM; i++) { + g_assert_cmpstr (nm_setting_vpn_get_data_item (s_vpn, info.to_del[i]), ==, NULL); + g_free (info.to_del[i]); + } + + /* And make sure the foreach callback was called the same number of times + * as there were keys in the table at the beginning of the foreach. + */ + g_assert_cmpint (info.called, ==, TO_DEL_NUM * 2); + + g_object_unref (s_vpn); +} + #define OLD_DBUS_TYPE_G_IP6_ADDRESS (dbus_g_type_get_struct ("GValueArray", DBUS_TYPE_G_UCHAR_ARRAY, G_TYPE_UINT, G_TYPE_INVALID)) #define OLD_DBUS_TYPE_G_ARRAY_OF_IP6_ADDRESS (dbus_g_type_get_collection ("GPtrArray", OLD_DBUS_TYPE_G_IP6_ADDRESS)) @@ -1151,6 +1220,7 @@ int main (int argc, char **argv) /* The tests */ test_setting_vpn_items (); test_setting_vpn_update_secrets (); + test_setting_vpn_modify_during_foreach (); test_setting_ip6_config_old_address_array (); test_setting_gsm_apn_spaces (); test_setting_gsm_apn_bad_chars ();