cli: allow dynamic results from values_fcn() and describe_fcn() property functions

Mostly these strings are static. In same cases they are generated
however. Instead of caching them in a static variable, return them
to the caller.

And belatedly fix invoking describe_fcn().

3 files changed, 89 insertions, 88 deletions

diff --git a/clients/cli/connections.c b/clients/cli/connections.c
index b750ec97b2..6421f7fd07 100644
--- a/clients/cli/connections.c
+++ b/clients/cli/connections.c
@@ -5701,26 +5701,23 @@ should_complete_vpn_uuids (const char *prompt, const char *line)
 }
 
 static const char *const*
-get_allowed_property_values (void)
+get_allowed_property_values (char ***out_to_free)
 {
-	NMSetting *setting;
-	char *property;
+	gs_unref_object NMSetting *setting = NULL;
+	gs_free char *property = NULL;
 	const char *const*avals = NULL;
 
 	get_setting_and_property (rl_prompt, rl_line_buffer, &setting, &property);
 	if (setting && property)
-		avals = nmc_setting_get_property_allowed_values (setting, property);
-
-	if (setting)
-		g_object_unref (setting);
-	g_free (property);
-
+		avals = nmc_setting_get_property_allowed_values (setting, property, out_to_free);
 	return avals;
 }
 
 static gboolean
 should_complete_property_values (const char *prompt, const char *line, gboolean *multi)
 {
+	gs_strfreev char **to_free = NULL;
+
 	/* properties allowing multiple values */
 	const char *multi_props[] = {
 		/* '802-1x' properties */
@@ -5736,10 +5733,9 @@ should_complete_property_values (const char *prompt, const char *line, gboolean
 		NULL
 	};
 	_get_and_check_property (prompt, line, NULL, multi_props, multi);
-	return get_allowed_property_values () != NULL;
+	return !!get_allowed_property_values (&to_free);
 }
 
-//FIXME: this helper should go to libnm later
 static gboolean
 _setting_property_is_boolean (NMSetting *setting, const char *property_name)
 {
@@ -5775,13 +5771,13 @@ should_complete_boolean (const char *prompt, const char *line)
 static char *
 gen_property_values (const char *text, int state)
 {
-	char *ret = NULL;
+	gs_strfreev char **to_free = NULL;
 	const char *const*avals;
 
-	avals = get_allowed_property_values ();
-	if (avals)
-		ret = nmc_rl_gen_func_basic (text, state, avals);
-	return ret;
+	avals = get_allowed_property_values (&to_free);
+	if (!avals)
+		return NULL;
+	return nmc_rl_gen_func_basic (text, state, avals);
 }
 
 /* from readline */
@@ -6657,13 +6653,16 @@ property_edit_submenu (NmCli *nmc,
 			 * single values:  : both SET and ADD sets the new value
 			 */
 			if (!cmd_property_arg) {
-				const char *const*avals = nmc_setting_get_property_allowed_values (curr_setting, prop_name);
+				gs_strfreev char **to_free = NULL;
+				const char *const*avals;
 
+				avals = nmc_setting_get_property_allowed_values (curr_setting, prop_name, &to_free);
 				if (avals) {
-					char *avals_str = nmc_util_strv_for_display (avals, FALSE);
+					gs_free char *avals_str = NULL;
+
+					avals_str = nmc_util_strv_for_display (avals, FALSE);
 					g_print (_("Allowed values for '%s' property: %s\n"),
 					         prop_name, avals_str);
-					g_free (avals_str);
 				}
 				prop_val_user = nmc_readline (_("Enter '%s' value: "), prop_name);
 			} else
@@ -7068,8 +7067,9 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t
 			/* Set property value */
 			if (!cmd_arg) {
 				if (menu_ctx.level == 1) {
+					gs_strfreev char **avals_to_free = NULL;
+					gs_free char *prop_val_user = NULL;
 					const char *prop_name;
-					char *prop_val_user = NULL;
 					const char *const*avals;
 					GError *tmp_err = NULL;
 
@@ -7079,12 +7079,13 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t
 					if (!prop_name)
 						break;
 
-					avals = nmc_setting_get_property_allowed_values (menu_ctx.curr_setting, prop_name);
+					avals = nmc_setting_get_property_allowed_values (menu_ctx.curr_setting, prop_name, &avals_to_free);
 					if (avals) {
-						char *avals_str = nmc_util_strv_for_display (avals, FALSE);
+						gs_free char *avals_str = NULL;
+
+						avals_str = nmc_util_strv_for_display (avals, FALSE);
 						g_print (_("Allowed values for '%s' property: %s\n"),
 						         prop_name, avals_str);
-						g_free (avals_str);
 					}
 					prop_val_user = nmc_readline (_("Enter '%s' value: "), prop_name);
 
@@ -7137,12 +7138,16 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t
 
 				/* Ask for value */
 				if (!cmd_arg_v) {
-					const char *const*avals = nmc_setting_get_property_allowed_values (ss, prop_name);
+					gs_strfreev char **avals_to_free = NULL;
+					const char *const*avals;
+
+					avals = nmc_setting_get_property_allowed_values (ss, prop_name, &avals_to_free);
 					if (avals) {
-						char *avals_str = nmc_util_strv_for_display (avals, FALSE);
+						gs_free char *avals_str = NULL;
+
+						avals_str = nmc_util_strv_for_display (avals, FALSE);
 						g_print (_("Allowed values for '%s' property: %s\n"),
 						         prop_name, avals_str);
-						g_free (avals_str);
 					}
 					cmd_arg_v = nmc_readline (_("Enter '%s' value: "), prop_name);
 				}
diff --git a/clients/cli/settings.c b/clients/cli/settings.c
index d4f6efb498..5d1760adde 100644
--- a/clients/cli/settings.c
+++ b/clients/cli/settings.c
@@ -47,7 +47,7 @@ static char *secret_flags_to_string (guint32 flags, NmcPropertyGetType get_type)
 /*****************************************************************************/
 
 #define ARGS_DESCRIBE_FCN \
-	const NmcSettingInfo *setting_info, const NmcPropertyInfo *property_info
+	const NmcSettingInfo *setting_info, const NmcPropertyInfo *property_info, char **out_to_free
 
 #define ARGS_GET_FCN \
 	const NmcSettingInfo *setting_info, const NmcPropertyInfo *property_info, NMSetting *setting, NmcPropertyGetType get_type, gboolean show_secrets
@@ -59,7 +59,7 @@ static char *secret_flags_to_string (guint32 flags, NmcPropertyGetType get_type)
 	const NmcSettingInfo *setting_info, const NmcPropertyInfo *property_info, NMSetting *setting, const char *value, guint32 idx, GError **error
 
 #define ARGS_VALUES_FCN \
-	const NmcSettingInfo *setting_info, const NmcPropertyInfo *property_info
+	const NmcSettingInfo *setting_info, const NmcPropertyInfo *property_info, char ***out_to_free
 
 static char *
 _get_fcn_name (ARGS_GET_FCN)
@@ -312,23 +312,16 @@ _set_fcn_gobject_secret_flags (ARGS_SET_FCN)
 static const char *const*
 _values_fcn_gobject_enum (ARGS_VALUES_FCN)
 {
-	static GHashTable *cache = NULL;
-	const char **v;
+	char **v, **w;
+	bool has_minmax =    property_info->property_typ_data->subtype.gobject_enum.min
+	                  || property_info->property_typ_data->subtype.gobject_enum.max;
 
-	if (G_UNLIKELY (!cache))
-		cache = g_hash_table_new (NULL, NULL);
-
-	v = g_hash_table_lookup (cache, property_info);
-	if (!v) {
-		bool has_minmax =    property_info->property_typ_data->subtype.gobject_enum.min
-		                  || property_info->property_typ_data->subtype.gobject_enum.max;
-
-		v = nm_utils_enum_get_values (             property_info->property_typ_data->subtype.gobject_enum.get_gtype (),
-		                              has_minmax ? property_info->property_typ_data->subtype.gobject_enum.min : G_MININT,
-		                              has_minmax ? property_info->property_typ_data->subtype.gobject_enum.max : G_MAXINT);
-		g_hash_table_insert (cache, (gpointer) property_info, v);
-	}
-	return (const char *const*) v;
+	v = (char **) nm_utils_enum_get_values (             property_info->property_typ_data->subtype.gobject_enum.get_gtype (),
+	                                        has_minmax ? property_info->property_typ_data->subtype.gobject_enum.min : G_MININT,
+	                                        has_minmax ? property_info->property_typ_data->subtype.gobject_enum.max : G_MAXINT);
+	for (w = v; w && *w; w++)
+		*w = g_strdup (*w);
+	return (const char *const*) (*out_to_free = v);
 }
 
 /*****************************************************************************/
@@ -1480,29 +1473,26 @@ DEFINE_REMOVER_OPTION (_remove_fcn_bond_options,
 static const char *
 _describe_fcn_bond_options (ARGS_DESCRIBE_FCN)
 {
-	static char *desc = NULL;
+	gs_free char *options_str = NULL;
 	const char **valid_options;
-	char *options_str;
+	char *s;
 
-	if (G_UNLIKELY (desc == NULL)) {
-		valid_options = nm_setting_bond_get_valid_options (NULL);
-		options_str = g_strjoinv (", ", (char **) valid_options);
+	valid_options = nm_setting_bond_get_valid_options (NULL);
+	options_str = g_strjoinv (", ", (char **) valid_options);
 
-		desc = g_strdup_printf (_("Enter a list of bonding options formatted as:\n"
-		                          "  option = <value>, option = <value>,... \n"
-		                          "Valid options are: %s\n"
-		                          "'mode' can be provided as a name or a number:\n"
-		                          "balance-rr    = 0\n"
-		                          "active-backup = 1\n"
-		                          "balance-xor   = 2\n"
-		                          "broadcast     = 3\n"
-		                          "802.3ad       = 4\n"
-		                          "balance-tlb   = 5\n"
-		                          "balance-alb   = 6\n\n"
-		                          "Example: mode=2,miimon=120\n"), options_str);
-		g_free (options_str);
-	}
-	return desc;
+	s = g_strdup_printf (_("Enter a list of bonding options formatted as:\n"
+	                       "  option = <value>, option = <value>,... \n"
+	                       "Valid options are: %s\n"
+	                       "'mode' can be provided as a name or a number:\n"
+	                       "balance-rr    = 0\n"
+	                       "active-backup = 1\n"
+	                       "balance-xor   = 2\n"
+	                       "broadcast     = 3\n"
+	                       "802.3ad       = 4\n"
+	                       "balance-tlb   = 5\n"
+	                       "balance-alb   = 6\n\n"
+	                       "Example: mode=2,miimon=120\n"), options_str);
+	return (*out_to_free = s);
 }
 
 static const char *const*
@@ -3641,22 +3631,19 @@ _values_fcn__wired_s390_options (ARGS_VALUES_FCN)
 static const char *
 _describe_fcn_wired_s390_options (ARGS_DESCRIBE_FCN)
 {
-	static char *desc = NULL;
+	gs_free char *options_str = NULL;
 	const char **valid_options;
-	char *options_str;
+	char *s;
 
-	if (G_UNLIKELY (desc == NULL)) {
-		valid_options = nm_setting_wired_get_valid_s390_options (NULL);
+	valid_options = nm_setting_wired_get_valid_s390_options (NULL);
 
-		options_str = g_strjoinv (", ", (char **) valid_options);
+	options_str = g_strjoinv (", ", (char **) valid_options);
 
-		desc = g_strdup_printf (_("Enter a list of S/390 options formatted as:\n"
-		                          "  option = <value>, option = <value>,...\n"
-		                          "Valid options are: %s\n"),
-		                        options_str);
-		g_free (options_str);
-	}
-	return desc;
+	s = g_strdup_printf (_("Enter a list of S/390 options formatted as:\n"
+	                       "  option = <value>, option = <value>,...\n"
+	                       "Valid options are: %s\n"),
+	                       options_str);
+	return (*out_to_free = s);
 }
 
 
@@ -4655,17 +4642,17 @@ nmc_setting_get_valid_properties (NMSetting *setting)
 	return valid_props;
 }
 
-/*
- * Return allowed values for 'prop' as a string.
- */
 const char *const*
-nmc_setting_get_property_allowed_values (NMSetting *setting, const char *prop)
+nmc_setting_get_property_allowed_values (NMSetting *setting, const char *prop, char ***out_to_free)
 {
 
 	const NmcSettingInfo *setting_info;
 	const NmcPropertyInfo *property_info;
 
 	g_return_val_if_fail (NM_IS_SETTING (setting), FALSE);
+	g_return_val_if_fail (out_to_free, FALSE);
+
+	*out_to_free = NULL;
 
 	if ((property_info = _meta_find_property_info_by_setting (setting, prop, &setting_info))) {
 		if (property_info->is_name) {
@@ -4673,7 +4660,8 @@ nmc_setting_get_property_allowed_values (NMSetting *setting, const char *prop)
 			 * For the moment, skip it from get_property_val(). */
 		} else if (property_info->property_type->values_fcn) {
 			return property_info->property_type->values_fcn (setting_info,
-			                                                 property_info);
+			                                                 property_info,
+			                                                 out_to_free);
 		} else if (property_info->property_typ_data && property_info->property_typ_data->values_static)
 			return property_info->property_typ_data->values_static;
 	}
@@ -4694,6 +4682,7 @@ nmc_setting_get_property_allowed_values (NMSetting *setting, const char *prop)
 char *
 nmc_setting_get_property_desc (NMSetting *setting, const char *prop)
 {
+	gs_free char *desc_to_free = NULL;
 	const char *setting_desc = NULL;
 	const char *setting_desc_title = "";
 	const char *nmcli_desc = NULL;
@@ -4709,11 +4698,18 @@ nmc_setting_get_property_desc (NMSetting *setting, const char *prop)
 		setting_desc_title = _("[NM property description]");
 
 	if ((property_info = _meta_find_property_info_by_setting (setting, prop, &setting_info))) {
+		const char *desc = NULL;
+
 		if (property_info->is_name) {
 			/* Traditionally, the "name" property was not handled here.
 			 * For the moment, skip it from get_property_val(). */
-		} else if (property_info->describe_message) {
-			nmcli_desc = _(property_info->describe_message);
+		} else if (property_info->property_type->describe_fcn) {
+			desc = property_info->property_type->describe_fcn (setting_info, property_info, &desc_to_free);
+		} else
+			desc = property_info->describe_message;
+
+		if (desc) {
+			nmcli_desc = _(desc);
 			nmcli_desc_title = _("[nmcli specific description]");
 			nmcli_nl = "\n";
 		}
diff --git a/clients/cli/settings.h b/clients/cli/settings.h
index ae63ccf138..76d2a3bbd9 100644
--- a/clients/cli/settings.h
+++ b/clients/cli/settings.h
@@ -45,9 +45,9 @@ typedef struct _NmcPropertyTypData NmcPropertyTypData;
 
 struct _NmcPropertyType {
 
-	/* FIXME: the function should return an allocated string. */
 	const char *(*describe_fcn) (const NmcSettingInfo *setting_info,
-	                             const NmcPropertyInfo *property_info);
+	                             const NmcPropertyInfo *property_info,
+	                             char **out_to_free);
 
 	char *(*get_fcn) (const NmcSettingInfo *setting_info,
 	                  const NmcPropertyInfo *property_info,
@@ -66,9 +66,9 @@ struct _NmcPropertyType {
 	                        guint32 idx,
 	                        GError **error);
 
-	/* FIXME: the function should return an allocated string. */
 	const char *const*(*values_fcn) (const NmcSettingInfo *setting_info,
-	                                 const NmcPropertyInfo *property_info);
+	                                 const NmcPropertyInfo *property_info,
+	                                 char ***out_to_free);
 };
 
 struct _NmcPropertyTypData {
@@ -130,7 +130,7 @@ void nmc_setting_connection_connect_handlers (NMSettingConnection *setting, NMCo
 
 char      **nmc_setting_get_valid_properties (NMSetting *setting);
 char       *nmc_setting_get_property_desc (NMSetting *setting, const char *prop);
-const char *const*nmc_setting_get_property_allowed_values (NMSetting *setting, const char *prop);
+const char *const*nmc_setting_get_property_allowed_values (NMSetting *setting, const char *prop, char ***out_to_free);
 char       *nmc_setting_get_property (NMSetting *setting,
                                       const char *prop,
                                       GError **error);