cli: don't interpret value as index too early for nmc_setting_remove_property_option()

Not all implementations support having the value being an index.
For example, the implementations that are done via DEFINE_REMOVER_OPTION() macro.

The meaning of the "value" string must not be determined by
nmc_setting_remove_property_option(). It's up to the implementation
to decide whether to allow an index and how to interpret it.
This commit is contained in:
Thomas Haller 2019-03-18 09:23:37 +01:00
parent d3cfe20598
commit cb5a81399a
5 changed files with 37 additions and 57 deletions

View file

@ -4004,12 +4004,7 @@ set_property (NMClient *client,
* - or option name: remove item with the option name
*/
if (value) {
unsigned long idx;
if (nmc_string_to_uint (value, TRUE, 0, G_MAXUINT32, &idx))
nmc_setting_remove_property_option (setting, property_name, NULL, idx, &local);
else
nmc_setting_remove_property_option (setting, property_name, value, 0, &local);
nmc_setting_remove_property_option (setting, property_name, value, &local);
if (local) {
g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT,
_("Error: failed to remove a value from %s.%s: %s."),
@ -6996,17 +6991,9 @@ property_edit_submenu (NmCli *nmc,
case NMC_EDITOR_SUB_CMD_REMOVE:
if (cmd_property_arg) {
unsigned long val_int = G_MAXUINT32;
gs_free char *option = NULL;
if (!nmc_string_to_uint (cmd_property_arg, TRUE, 0, G_MAXUINT32, &val_int)) {
option = g_strdup (cmd_property_arg);
g_strstrip (option);
}
if (!nmc_setting_remove_property_option (curr_setting, prop_name,
option,
(guint32) val_int,
if (!nmc_setting_remove_property_option (curr_setting,
prop_name,
cmd_property_arg,
&tmp_err)) {
g_print (_("Error: %s\n"), tmp_err->message);
g_clear_error (&tmp_err);

View file

@ -618,26 +618,17 @@ nmc_setting_reset_property (NMSetting *setting, const char *prop, GError **error
return FALSE;
}
/*
* Generic function for removing items for collection-type properties.
*
* If 'option' is not NULL, it tries to remove it, otherwise 'idx' is used.
* For single-value properties (not having specialized remove function) this
* function does nothing and just returns TRUE.
*
* Returns: TRUE on success; FALSE on failure and sets error
*/
gboolean
nmc_setting_remove_property_option (NMSetting *setting,
const char *prop,
const char *option,
guint32 idx,
const char *value,
GError **error)
{
const NMMetaPropertyInfo *property_info;
g_return_val_if_fail (NM_IS_SETTING (setting), FALSE);
g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
g_return_val_if_fail (!error || !*error, FALSE);
g_return_val_if_fail (value, FALSE);
if ((property_info = nm_meta_property_info_find_by_setting (setting, prop))) {
if (property_info->property_type->remove_fcn) {
@ -645,8 +636,7 @@ nmc_setting_remove_property_option (NMSetting *setting,
nmc_meta_environment,
nmc_meta_environment_arg,
setting,
option,
idx,
value,
error);
}
}

View file

@ -52,8 +52,7 @@ gboolean nmc_setting_reset_property (NMSetting *setting,
GError **error);
gboolean nmc_setting_remove_property_option (NMSetting *setting,
const char *prop,
const char *option,
guint32 idx,
const char *value,
GError **error);
void nmc_property_set_default_value (NMSetting *setting, const char *prop);

View file

@ -628,7 +628,7 @@ _env_warn_fcn (const NMMetaEnvironment *environment,
const NMMetaPropertyInfo *property_info, const NMMetaEnvironment *environment, gpointer environment_user_data, NMSetting *setting, const char *value, GError **error
#define ARGS_REMOVE_FCN \
const NMMetaPropertyInfo *property_info, const NMMetaEnvironment *environment, gpointer environment_user_data, NMSetting *setting, const char *value, guint32 idx, GError **error
const NMMetaPropertyInfo *property_info, const NMMetaEnvironment *environment, gpointer environment_user_data, NMSetting *setting, const char *value, GError **error
#define ARGS_COMPLETE_FCN \
const NMMetaPropertyInfo *property_info, const NMMetaEnvironment *environment, gpointer environment_user_data, const NMMetaOperationContext *operation_context, const char *text, char ***out_to_free
@ -1669,9 +1669,11 @@ vpn_data_item (const char *key, const char *value, gpointer user_data)
static gboolean \
def_func (ARGS_REMOVE_FCN) \
{ \
guint32 num; \
gint64 idx; \
gint64 num; \
\
if (!value) { \
idx = _nm_utils_ascii_str_to_int64 (value, 10, 0, G_MAXINT64, -1); \
if (idx != -1) { \
num = num_func (s_macro (setting)); \
if (idx < num) \
rem_func_idx (s_macro (setting), idx); \
@ -4158,31 +4160,36 @@ _remove_vlan_xgress_priority_map (const NMMetaEnvironment *environment,
NMSetting *setting,
const NMMetaPropertyInfo *property_info,
const char *value,
guint32 idx,
NMVlanPriorityMap map_type,
GError **error)
{
gs_strfreev char **prio_map = NULL;
guint32 num;
gint64 idx;
gsize i;
if (value) {
gs_strfreev char **prio_map = NULL;
gsize i;
prio_map = _parse_vlan_priority_maps (value, map_type, TRUE, error);
if (!prio_map)
return FALSE;
for (i = 0; prio_map[i]; i++) {
nm_setting_vlan_remove_priority_str_by_value (NM_SETTING_VLAN (setting),
map_type,
prio_map[i]);
}
idx = _nm_utils_ascii_str_to_int64 (value, 10, 0, G_MAXINT64, -1);
if (idx != -1) {
/* FIXME: this is ugly, if the caller only provides one number, we accept it as
* index to remove.
* The ugly-ness comes from the fact, that
* " -vlan.ingress-priority-map 0,1,2"
* cannot be used to remove the first 3 elements. */
num = nm_setting_vlan_get_num_priorities (NM_SETTING_VLAN (setting), map_type);
if (idx < (gint64) num)
nm_setting_vlan_remove_priority (NM_SETTING_VLAN (setting), map_type, idx);
return TRUE;
}
num = nm_setting_vlan_get_num_priorities (NM_SETTING_VLAN (setting), map_type);
if (idx < num)
nm_setting_vlan_remove_priority (NM_SETTING_VLAN (setting), map_type, idx);
prio_map = _parse_vlan_priority_maps (value, map_type, TRUE, error);
if (!prio_map)
return FALSE;
for (i = 0; prio_map[i]; i++) {
nm_setting_vlan_remove_priority_str_by_value (NM_SETTING_VLAN (setting),
map_type,
prio_map[i]);
}
return TRUE;
}
@ -4194,7 +4201,6 @@ _remove_fcn_vlan_ingress_priority_map (ARGS_REMOVE_FCN)
setting,
property_info,
value,
idx,
NM_VLAN_INGRESS_MAP,
error);
}
@ -4207,7 +4213,6 @@ _remove_fcn_vlan_egress_priority_map (ARGS_REMOVE_FCN)
setting,
property_info,
value,
idx,
NM_VLAN_EGRESS_MAP,
error);
}

View file

@ -215,8 +215,7 @@ struct _NMMetaPropertyType {
const NMMetaEnvironment *environment,
gpointer environment_user_data,
NMSetting *setting,
const char *option,
guint32 idx,
const char *value,
GError **error);
const char *const*(*values_fcn) (const NMMetaPropertyInfo *property_info,