From 0825ec34fde7fc35db54aff110d678ebce2dfb52 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 9 Aug 2019 09:43:15 +0200 Subject: [PATCH] cli: add NMMetaAccessorModifier enum instead of using "char" type The enum values are unique throughout the source code so they can easier be searched (e.g. with grep), compared to '\0'. It is often interesting where a certain modifier is used, so searching the source code is important to give relevant results. Also, the modifier is really an enum and we shouldn't misuse char type. If that would be a good idea in general, we wouldn't need any enums at all. But we use them for good reasons. --- clients/cli/connections.c | 196 ++++++++++++++++++-------- clients/cli/settings.c | 8 +- clients/cli/settings.h | 2 +- clients/common/nm-meta-setting-desc.c | 35 +++-- clients/common/nm-meta-setting-desc.h | 8 +- 5 files changed, 175 insertions(+), 74 deletions(-) diff --git a/clients/cli/connections.c b/clients/cli/connections.c index b8905cc78..260844fae 100644 --- a/clients/cli/connections.c +++ b/clients/cli/connections.c @@ -3321,6 +3321,21 @@ get_valid_settings_array (const char *con_type) return NULL; } +static char * +_construct_property_name (const char *setting_name, + const char *property_name, + NMMetaAccessorModifier modifier) +{ + return g_strdup_printf ("%s%s.%s\n", + ( modifier == NM_META_ACCESSOR_MODIFIER_ADD + ? "+" + : ( modifier == NM_META_ACCESSOR_MODIFIER_DEL + ? "-" + : "")), + setting_name, + property_name); +} + /* get_valid_properties_string: * @array: base properties for the current connection type * @array_slv: slave properties (or ipv4/ipv6 ones) for the current connection type @@ -3338,7 +3353,7 @@ get_valid_settings_array (const char *con_type) static char * get_valid_properties_string (const NMMetaSettingValidPartItem *const*array, const NMMetaSettingValidPartItem *const*array_slv, - char modifier, + NMMetaAccessorModifier modifier, const char *prefix, const char *postfix) { @@ -3380,32 +3395,26 @@ get_valid_properties_string (const NMMetaSettingValidPartItem *const*array, /* Search the array with the arguments of the current property */ for (j = 0; j < setting_info->properties_num; j++) { - char *new; + gs_free char *ss1 = NULL; const char *arg_name; arg_name = setting_info->properties[j]->property_name; /* If required, expand the alias too */ - if (!postfix && setting_info->alias) { - if (modifier) - g_string_append_c (str, modifier); - new = g_strdup_printf ("%s.%s\n", - setting_info->alias, - arg_name); - g_string_append (str, new); - g_free (new); + if ( !postfix + && setting_info->alias) { + gs_free char *ss2 = NULL; + + ss2 = _construct_property_name (setting_info->alias, arg_name, modifier); + g_string_append (str, ss2); } - if (postfix && !g_str_has_prefix (arg_name, postfix)) + if ( postfix + && !g_str_has_prefix (arg_name, postfix)) continue; - if (modifier) - g_string_append_c (str, modifier); - new = g_strdup_printf ("%s.%s\n", - prop_name, - arg_name); - g_string_append (str, new); - g_free (new); + ss1 = _construct_property_name (prop_name, arg_name, modifier); + g_string_append (str, ss1); } } } @@ -4005,7 +4014,7 @@ set_property (NMClient *client, const char *setting_name, const char *property, const char *value, - char modifier, + NMMetaAccessorModifier modifier, GError **error) { gs_free char *property_name = NULL; @@ -4013,7 +4022,9 @@ set_property (NMClient *client, NMSetting *setting; nm_assert (setting_name && setting_name[0]); - nm_assert (NM_IN_SET (modifier, '\0', '+', '-')); + nm_assert (NM_IN_SET (modifier, NM_META_ACCESSOR_MODIFIER_SET, + NM_META_ACCESSOR_MODIFIER_ADD, + NM_META_ACCESSOR_MODIFIER_DEL)); setting = nm_connection_get_setting_by_name (connection, setting_name); if (!setting) { @@ -4033,14 +4044,15 @@ set_property (NMClient *client, if (!nmc_setting_set_property (client, setting, property_name, - ( (modifier == '-' && !value) - ? '\0' + ( ( modifier == NM_META_ACCESSOR_MODIFIER_DEL + && !value) + ? NM_META_ACCESSOR_MODIFIER_SET : modifier), value, &local)) { g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, _("Error: failed to %s %s.%s: %s."), - ( modifier != '-' + ( modifier != NM_META_ACCESSOR_MODIFIER_DEL ? "modify" : "remove a value from"), setting_name, @@ -4070,8 +4082,15 @@ set_option (NmCli *nmc, NMConnection *connection, const NMMetaAbstractInfo *abst if (option && option->check_and_set) { return option->check_and_set (nmc, connection, option, value, error); } else if (value) { - return set_property (nmc->client, connection, setting_name, property_name, - value, inf_flags & NM_META_PROPERTY_INF_FLAG_MULTI ? '+' : '\0', error); + return set_property (nmc->client, + connection, + setting_name, + property_name, + value, + inf_flags & NM_META_PROPERTY_INF_FLAG_MULTI + ? NM_META_ACCESSOR_MODIFIER_ADD + : NM_META_ACCESSOR_MODIFIER_SET, + error); } else if (inf_flags & NM_META_PROPERTY_INF_FLAG_REQD) { g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT, _("Error: '%s' is mandatory."), option_name); @@ -4192,9 +4211,13 @@ set_connection_type (NmCli *nmc, NMConnection *con, const OptionInfo *option, co } if (slave_type) { - if (!set_property (nmc->client, con, NM_SETTING_CONNECTION_SETTING_NAME, - NM_SETTING_CONNECTION_SLAVE_TYPE, slave_type, - '\0', error)) { + if (!set_property (nmc->client, + con, + NM_SETTING_CONNECTION_SETTING_NAME, + NM_SETTING_CONNECTION_SLAVE_TYPE, + slave_type, + NM_META_ACCESSOR_MODIFIER_SET, + error)) { return FALSE; } enable_options (NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_MASTER, master); @@ -4209,7 +4232,13 @@ set_connection_type (NmCli *nmc, NMConnection *con, const OptionInfo *option, co NM_SETTING_CONNECTION_INTERFACE_NAME); } - if (!set_property (nmc->client, con, option->setting_info->general->setting_name, option->property, value, '\0', error)) + if (!set_property (nmc->client, + con, + option->setting_info->general->setting_name, + option->property, + value, + NM_META_ACCESSOR_MODIFIER_SET, + error)) return FALSE; if (!con_settings (con, &type_settings, &slv_settings, error)) @@ -4238,7 +4267,13 @@ set_connection_iface (NmCli *nmc, NMConnection *con, const OptionInfo *option, c } } - return set_property (nmc->client, con, option->setting_info->general->setting_name, option->property, value, '\0', error); + return set_property (nmc->client, + con, + option->setting_info->general->setting_name, + option->property, + value, + NM_META_ACCESSOR_MODIFIER_SET, + error); } static gboolean @@ -4261,13 +4296,23 @@ set_connection_master (NmCli *nmc, NMConnection *con, const OptionInfo *option, connections = nm_client_get_connections (nmc->client); value = normalized_master_for_slave (connections, value, slave_type, &slave_type); - if (!set_property (nmc->client, con, NM_SETTING_CONNECTION_SETTING_NAME, - NM_SETTING_CONNECTION_SLAVE_TYPE, slave_type, - '\0', error)) { + if (!set_property (nmc->client, + con, + NM_SETTING_CONNECTION_SETTING_NAME, + NM_SETTING_CONNECTION_SLAVE_TYPE, + slave_type, + NM_META_ACCESSOR_MODIFIER_SET, + error)) { return FALSE; } - return set_property (nmc->client, con, option->setting_info->general->setting_name, option->property, value, '\0', error); + return set_property (nmc->client, + con, + option->setting_info->general->setting_name, + option->property, + value, + NM_META_ACCESSOR_MODIFIER_SET, + error); } static gboolean @@ -4381,7 +4426,13 @@ set_bluetooth_type (NmCli *nmc, NMConnection *con, const OptionInfo *option, con return FALSE; } - return set_property (nmc->client, con, option->setting_info->general->setting_name, option->property, value, '\0', error); + return set_property (nmc->client, + con, + option->setting_info->general->setting_name, + option->property, + value, + NM_META_ACCESSOR_MODIFIER_SET, + error); } static gboolean @@ -4400,8 +4451,13 @@ set_ip4_address (NmCli *nmc, NMConnection *con, const OptionInfo *option, const NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP4_CONFIG_METHOD_MANUAL, NULL); } - return set_property (nmc->client, con, option->setting_info->general->setting_name, option->property, value, - '+', error); + return set_property (nmc->client, + con, + option->setting_info->general->setting_name, + option->property, + value, + NM_META_ACCESSOR_MODIFIER_ADD, + error); } static gboolean @@ -4420,8 +4476,13 @@ set_ip6_address (NmCli *nmc, NMConnection *con, const OptionInfo *option, const NM_SETTING_IP_CONFIG_METHOD, NM_SETTING_IP6_CONFIG_METHOD_MANUAL, NULL); } - return set_property (nmc->client, con, option->setting_info->general->setting_name, option->property, value, - '+', error); + return set_property (nmc->client, + con, + option->setting_info->general->setting_name, + option->property, + value, + NM_META_ACCESSOR_MODIFIER_ADD, + error); } /*****************************************************************************/ @@ -4493,7 +4554,7 @@ option_relevant (NMConnection *connection, const NMMetaAbstractInfo *abstract_in static void complete_property_name (NmCli *nmc, NMConnection *connection, - char modifier, + NMMetaAccessorModifier modifier, const char *prefix, const char *postfix) { @@ -4516,7 +4577,7 @@ complete_property_name (NmCli *nmc, NMConnection *connection, if (word_list) g_print ("%s", word_list); - if (modifier != '\0') + if (modifier != NM_META_ACCESSOR_MODIFIER_SET) return; for (s = 0; s < _NM_META_SETTING_TYPE_NUM; s++) { @@ -4658,7 +4719,7 @@ nmc_read_connection_properties (NmCli *nmc, gs_strfreev char **strv = NULL; const NMMetaSettingValidPartItem *const*type_settings; const NMMetaSettingValidPartItem *const*slv_settings; - char modifier = '\0'; + NMMetaAccessorModifier modifier; if (!con_settings (connection, &type_settings, &slv_settings, error)) return FALSE; @@ -4673,8 +4734,11 @@ nmc_read_connection_properties (NmCli *nmc, return FALSE; } - if (option[0] == '+' || option[0] == '-') - modifier = *option; + switch (option[0]) { + case '+': modifier = NM_META_ACCESSOR_MODIFIER_ADD; break; + case '-': modifier = NM_META_ACCESSOR_MODIFIER_DEL; break; + default: modifier = NM_META_ACCESSOR_MODIFIER_SET; break; + } strv = g_strsplit (option, ".", 2); if (g_strv_length (strv) == 2) { @@ -4683,7 +4747,7 @@ nmc_read_connection_properties (NmCli *nmc, char *setting = strv[0]; const char *setting_name; - if (modifier) + if (modifier != NM_META_ACCESSOR_MODIFIER_SET) setting++; if (*argc == 1 && nmc->complete) @@ -4769,7 +4833,7 @@ nmc_read_connection_properties (NmCli *nmc, } if (!chosen) { - if (modifier) + if (modifier != NM_META_ACCESSOR_MODIFIER_SET) option++; if (*argc == 1 && nmc->complete) complete_property_name (nmc, connection, modifier, option, NULL); @@ -6981,8 +7045,8 @@ property_edit_submenu (NmCli *nmc, curr_setting, prop_name, (cmdsub == NMC_EDITOR_SUB_CMD_SET) - ? '\0' - : '+', + ? NM_META_ACCESSOR_MODIFIER_SET + : NM_META_ACCESSOR_MODIFIER_ADD, prop_val_user, &tmp_err); if (!set_result) { @@ -6998,7 +7062,12 @@ property_edit_submenu (NmCli *nmc, _("Edit '%s' value: "), prop_name); - if (!nmc_setting_set_property (nmc->client, curr_setting, prop_name, '\0', prop_val_user, &tmp_err)) { + if (!nmc_setting_set_property (nmc->client, + curr_setting, + prop_name, + NM_META_ACCESSOR_MODIFIER_SET, + prop_val_user, + &tmp_err)) { g_print (_("Error: failed to set '%s' property: %s\n"), prop_name, tmp_err->message); g_clear_error (&tmp_err); } @@ -7009,8 +7078,8 @@ property_edit_submenu (NmCli *nmc, curr_setting, prop_name, ( cmd_property_arg - ? '-' - : '\0'), + ? NM_META_ACCESSOR_MODIFIER_DEL + : NM_META_ACCESSOR_MODIFIER_SET), cmd_property_arg, &tmp_err)) { g_print (_("Error: %s\n"), tmp_err->message); @@ -7364,7 +7433,12 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t _("Enter '%s' value: "), prop_name); - if (!nmc_setting_set_property (nmc->client, menu_ctx.curr_setting, prop_name, '+', prop_val_user, &tmp_err)) { + if (!nmc_setting_set_property (nmc->client, + menu_ctx.curr_setting, + prop_name, + NM_META_ACCESSOR_MODIFIER_ADD, + prop_val_user, + &tmp_err)) { g_print (_("Error: failed to set '%s' property: %s\n"), prop_name, tmp_err->message); g_clear_error (&tmp_err); } @@ -7428,7 +7502,9 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t if (!nmc_setting_set_property (nmc->client, ss, prop_name, - cmd_arg_v ? '+' : '\0', + cmd_arg_v + ? NM_META_ACCESSOR_MODIFIER_ADD + : NM_META_ACCESSOR_MODIFIER_SET, cmd_arg_v, &tmp_err)) { g_print (_("Error: failed to set '%s' property: %s\n"), @@ -7527,7 +7603,12 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t if (!prop_name) break; - if (!nmc_setting_set_property (nmc->client, menu_ctx.curr_setting, prop_name, '\0', NULL, &tmp_err)) { + if (!nmc_setting_set_property (nmc->client, + menu_ctx.curr_setting, + prop_name, + NM_META_ACCESSOR_MODIFIER_SET, + NULL, + &tmp_err)) { g_print (_("Error: failed to remove value of '%s': %s\n"), prop_name, tmp_err->message); g_clear_error (&tmp_err); @@ -7577,7 +7658,12 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t prop_name = is_property_valid (ss, cmd_arg_p, &tmp_err); if (prop_name) { - if (!nmc_setting_set_property (nmc->client, ss, prop_name, '\0', NULL, &tmp_err)) { + if (!nmc_setting_set_property (nmc->client, + ss, + prop_name, + NM_META_ACCESSOR_MODIFIER_SET, + NULL, + &tmp_err)) { g_print (_("Error: failed to remove value of '%s': %s\n"), prop_name, tmp_err->message); diff --git a/clients/cli/settings.c b/clients/cli/settings.c index 91aaf28e3..fec36a89f 100644 --- a/clients/cli/settings.c +++ b/clients/cli/settings.c @@ -532,7 +532,7 @@ gboolean nmc_setting_set_property (NMClient *client, NMSetting *setting, const char *prop, - char modifier, + NMMetaAccessorModifier modifier, const char *value, GError **error) { @@ -542,14 +542,14 @@ nmc_setting_set_property (NMClient *client, 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 (NM_IN_SET (modifier, '\0', '-', '+'), FALSE); + g_return_val_if_fail (NM_IN_SET (modifier, NM_META_ACCESSOR_MODIFIER_SET, NM_META_ACCESSOR_MODIFIER_DEL, NM_META_ACCESSOR_MODIFIER_ADD), FALSE); if (!(property_info = nm_meta_property_info_find_by_setting (setting, prop))) goto out_fail_read_only; if (!property_info->property_type->set_fcn) goto out_fail_read_only; - if ( modifier == '-' + if ( modifier == NM_META_ACCESSOR_MODIFIER_DEL && !property_info->property_type->set_supports_remove) { /* The property is a plain property. It does not support '-'. * @@ -572,7 +572,7 @@ nmc_setting_set_property (NMClient *client, } } - if ( NM_IN_SET (modifier, '+', '-') + if ( NM_IN_SET (modifier, NM_META_ACCESSOR_MODIFIER_ADD, NM_META_ACCESSOR_MODIFIER_DEL) && ( !value || !value[0])) { /* nothing to do. */ diff --git a/clients/cli/settings.h b/clients/cli/settings.h index 1ff936859..62e854661 100644 --- a/clients/cli/settings.h +++ b/clients/cli/settings.h @@ -45,7 +45,7 @@ char *nmc_setting_get_property_parsable (NMSetting *setting, gboolean nmc_setting_set_property (NMClient *client, NMSetting *setting, const char *prop, - char modifier, + NMMetaAccessorModifier modifier, const char *val, GError **error); diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c index d5014441c..bdeb2026e 100644 --- a/clients/common/nm-meta-setting-desc.c +++ b/clients/common/nm-meta-setting-desc.c @@ -638,7 +638,7 @@ _env_warn_fcn (const NMMetaEnvironment *environment, const NMMetaPropertyInfo *property_info, const NMMetaEnvironment *environment, gpointer environment_user_data, NMSetting *setting, NMMetaAccessorGetType get_type, NMMetaAccessorGetFlags get_flags, NMMetaAccessorGetOutFlags *out_flags, gboolean *out_is_default, gpointer *out_to_free #define ARGS_SET_FCN \ - const NMMetaPropertyInfo *property_info, const NMMetaEnvironment *environment, gpointer environment_user_data, NMSetting *setting, char modifier, const char *value, GError **error + const NMMetaPropertyInfo *property_info, const NMMetaEnvironment *environment, gpointer environment_user_data, NMSetting *setting, NMMetaAccessorModifier modifier, 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, GError **error @@ -653,43 +653,52 @@ _env_warn_fcn (const NMMetaEnvironment *environment, const NMMetaSettingInfoEditor *setting_info, NMSetting *setting, NMMetaAccessorSettingInitType init_type static gboolean -_SET_FCN_DO_RESET_DEFAULT (const NMMetaPropertyInfo *property_info, char modifier, const char *value) +_SET_FCN_DO_RESET_DEFAULT (const NMMetaPropertyInfo *property_info, NMMetaAccessorModifier modifier, const char *value) { nm_assert (property_info); nm_assert (!property_info->property_type->set_supports_remove); - nm_assert (NM_IN_SET (modifier, '\0', '+')); - nm_assert (value || modifier == '\0'); + nm_assert (NM_IN_SET (modifier, NM_META_ACCESSOR_MODIFIER_SET, + NM_META_ACCESSOR_MODIFIER_ADD)); + nm_assert ( value + || modifier == NM_META_ACCESSOR_MODIFIER_SET); return value == NULL; } static gboolean -_SET_FCN_DO_RESET_DEFAULT_WITH_SUPPORTS_REMOVE (const NMMetaPropertyInfo *property_info, char modifier, const char *value) +_SET_FCN_DO_RESET_DEFAULT_WITH_SUPPORTS_REMOVE (const NMMetaPropertyInfo *property_info, NMMetaAccessorModifier modifier, const char *value) { nm_assert (property_info); nm_assert (property_info->property_type->set_supports_remove); - nm_assert (NM_IN_SET (modifier, '\0', '+', '-')); - nm_assert (value || modifier == '\0'); + nm_assert (NM_IN_SET (modifier, NM_META_ACCESSOR_MODIFIER_SET, + NM_META_ACCESSOR_MODIFIER_ADD, + NM_META_ACCESSOR_MODIFIER_DEL)); + nm_assert ( value + || modifier == NM_META_ACCESSOR_MODIFIER_SET); return value == NULL; } static gboolean -_SET_FCN_DO_SET_ALL (char modifier, const char *value) +_SET_FCN_DO_SET_ALL (NMMetaAccessorModifier modifier, const char *value) { - nm_assert (NM_IN_SET (modifier, '\0', '+', '-')); + nm_assert (NM_IN_SET (modifier, NM_META_ACCESSOR_MODIFIER_SET, + NM_META_ACCESSOR_MODIFIER_ADD, + NM_META_ACCESSOR_MODIFIER_DEL)); nm_assert (value); - return modifier == '\0'; + return modifier == NM_META_ACCESSOR_MODIFIER_SET; } static gboolean -_SET_FCN_DO_REMOVE (char modifier, const char *value) +_SET_FCN_DO_REMOVE (NMMetaAccessorModifier modifier, const char *value) { - nm_assert (NM_IN_SET (modifier, '\0', '+', '-')); + nm_assert (NM_IN_SET (modifier, NM_META_ACCESSOR_MODIFIER_SET, + NM_META_ACCESSOR_MODIFIER_ADD, + NM_META_ACCESSOR_MODIFIER_DEL)); nm_assert (value); - return modifier == '-'; + return modifier == NM_META_ACCESSOR_MODIFIER_DEL; } #define RETURN_UNSUPPORTED_GET_TYPE() \ diff --git a/clients/common/nm-meta-setting-desc.h b/clients/common/nm-meta-setting-desc.h index 27ae4254b..cb4fe4b25 100644 --- a/clients/common/nm-meta-setting-desc.h +++ b/clients/common/nm-meta-setting-desc.h @@ -129,6 +129,12 @@ typedef enum { _NM_META_COLOR_NUM } NMMetaColor; +typedef enum { + NM_META_ACCESSOR_MODIFIER_SET, + NM_META_ACCESSOR_MODIFIER_ADD, + NM_META_ACCESSOR_MODIFIER_DEL, +} NMMetaAccessorModifier; + typedef enum { NM_META_ACCESSOR_GET_TYPE_PRETTY, NM_META_ACCESSOR_GET_TYPE_PARSABLE, @@ -210,7 +216,7 @@ struct _NMMetaPropertyType { const NMMetaEnvironment *environment, gpointer environment_user_data, NMSetting *setting, - char modifier, + NMMetaAccessorModifier modifier, const char *value, GError **error);