libnm-core: clean up NMConnectionError

Rename NM_CONNECTION_ERROR_UNKNOWN to NM_CONNECTION_ERROR_FAILED,
following GError best practices.

Replace NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND ("no
NMSettingConnection") with a more generic
NM_CONNECTION_ERROR_MISSING_SETTING. Use that new code in a few places
that had previously been using NM_CONNECTION_ERROR_SETTING_NOT_FOUND,
which was supposed to mean "the setting that you asked about doesn't
exist", not "the connection is invalid because it's missing a required
setting".

Clarify that NM_CONNECTION_ERROR_INVALID_SETTING can be used for any
"invalid or inappropriate NMSetting", not just a "conflicting" one.
(But fix a case in nm_connection_update_secrets() that was returning
INVALID_SETTING when it should have been return-if-failing instead.)

For both MISSING_SETTING and INVALID_SETTING, always prefix the error
message with "setting-name: ", just like we do with the various
NMSetting MISSING_PROPERTY and INVALID_PROPERTY errors. And make sure
that the error message is marked for localization.

Drop NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID, which is pretty
pointless; it was only used in the case where connection.type was the
name of a valid setting type that is not a base setting type. Instead,
just return NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY for
connection.type in this case (which is what the code already did when
connection.type was completely unrecognized).
This commit is contained in:
Dan Winship 2014-10-11 15:02:16 -04:00
parent a7b1ee77db
commit aeb3d093f6
9 changed files with 69 additions and 86 deletions

View file

@ -312,10 +312,11 @@ nm_connection_replace_settings (NMConnection *connection,
type = nm_setting_lookup_type (setting_name);
if (type == G_TYPE_INVALID) {
g_set_error (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_SETTING,
"unknown setting name '%s'", setting_name);
g_set_error_literal (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_SETTING,
_("unknown setting name"));
g_prefix_error (error, "%s: ", setting_name);
g_variant_unref (setting_dict);
g_slist_free_full (settings, g_object_unref);
return FALSE;
@ -749,8 +750,9 @@ _nm_connection_verify (NMConnection *connection, GError **error)
if (!s_con) {
g_set_error_literal (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND,
"connection setting not found");
NM_CONNECTION_ERROR_MISSING_SETTING,
_("setting not found"));
g_prefix_error (error, "%s: ", NM_SETTING_CONNECTION_SETTING_NAME);
goto EXIT;
}
@ -811,21 +813,23 @@ _nm_connection_verify (NMConnection *connection, GError **error)
if ((normalizable_error_type == NM_SETTING_VERIFY_SUCCESS ||
(normalizable_error_type == NM_SETTING_VERIFY_NORMALIZABLE)) && (s_ip4 || s_ip6)) {
g_clear_error (&normalizable_error);
g_set_error (&normalizable_error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_SETTING,
"slave connection cannot have an IP%c setting",
s_ip4 ? '4' : '6');
g_set_error_literal (&normalizable_error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_SETTING,
_("setting not allowed in slave connection"));
g_prefix_error (&normalizable_error, "%s: ",
s_ip4 ? NM_SETTING_IP4_CONFIG_SETTING_NAME : NM_SETTING_IP6_CONFIG_SETTING_NAME);
/* having a slave with IP config *was* and is a verify() error. */
normalizable_error_type = NM_SETTING_VERIFY_NORMALIZABLE_ERROR;
}
} else {
if (normalizable_error_type == NM_SETTING_VERIFY_SUCCESS && (!s_ip4 || !s_ip6)) {
g_set_error (&normalizable_error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_SETTING_NOT_FOUND,
"connection needs an IP%c setting",
!s_ip4 ? '4' : '6');
g_set_error_literal (&normalizable_error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_MISSING_SETTING,
_("setting is required for non-slave connections"));
g_prefix_error (&normalizable_error, "%s: ",
!s_ip4 ? NM_SETTING_IP4_CONFIG_SETTING_NAME : NM_SETTING_IP6_CONFIG_SETTING_NAME);
/* having a master without IP config was not a verify() error, accept
* it for backward compatibility. */
normalizable_error_type = NM_SETTING_VERIFY_NORMALIZABLE;
@ -888,7 +892,7 @@ nm_connection_normalize (NMConnection *connection,
if (success == NM_SETTING_VERIFY_ERROR && error && !*error) {
g_set_error_literal (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_UNKNOWN,
NM_CONNECTION_ERROR_FAILED,
_("Unexpected failure to verify the connection"));
g_return_val_if_reached (FALSE);
}
@ -919,7 +923,7 @@ nm_connection_normalize (NMConnection *connection,
if (error && !*error) {
g_set_error_literal (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_UNKNOWN,
NM_CONNECTION_ERROR_FAILED,
_("Unexpected failure to normalize the connection"));
}
g_return_val_if_reached (FALSE);
@ -969,12 +973,13 @@ nm_connection_update_secrets (NMConnection *connection,
if (error)
g_return_val_if_fail (*error == NULL, FALSE);
full_connection = g_variant_is_of_type (secrets, NM_VARIANT_TYPE_CONNECTION);
g_return_val_if_fail (setting_name != NULL || full_connection, FALSE);
/* Empty @secrets means success */
if (g_variant_n_children (secrets) == 0)
return TRUE;
full_connection = g_variant_is_of_type (secrets, NM_VARIANT_TYPE_CONNECTION);
if (setting_name) {
/* Update just one setting's secrets */
setting = nm_connection_get_setting_by_name (connection, setting_name);
@ -1009,14 +1014,6 @@ nm_connection_update_secrets (NMConnection *connection,
if (success_detail == NM_SETTING_UPDATE_SECRET_SUCCESS_MODIFIED)
updated = TRUE;
} else {
if (!full_connection) {
g_set_error_literal (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_INVALID_SETTING,
_("Update secrets expects a full connection, instead only a setting is provided."));
return FALSE;
}
/* check first, whether all the settings exist... */
g_variant_iter_init (&iter, secrets);
while (g_variant_iter_next (&iter, "{&s@a{sv}}", &key, NULL)) {

View file

@ -72,28 +72,24 @@ G_BEGIN_DECLS
/**
* NMConnectionError:
* @NM_CONNECTION_ERROR_UNKNOWN: unknown or unclassified error
* @NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND: the #NMConnection object
* did not contain the required #NMSettingConnection object, which must be
* present for all connections
* @NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID: the 'type' property of the
* 'connection' setting did not point to a valid connection base type; ie
* it was not a hardware-related setting like #NMSettingWired or
* #NMSettingWireless.
* @NM_CONNECTION_ERROR_FAILED: unknown or unclassified error
* @NM_CONNECTION_ERROR_SETTING_NOT_FOUND: the #NMConnection object
* did not contain the specified #NMSetting object
*@NM_CONNECTION_ERROR_INVALID_SETTING: the #NMConnection object contains
* a conflicting setting object
* @NM_CONNECTION_ERROR_MISSING_SETTING: the #NMConnection object is missing an
* #NMSetting which is required for its configuration. The error message will
* always be prefixed with "<setting-name>: ", where "<setting-name>" is the
* name of the setting that is missing.
* @NM_CONNECTION_ERROR_INVALID_SETTING: the #NMConnection object contains an
* invalid or inappropriate #NMSetting. The error message will always be
* prefixed with "<setting-name>: ", where "<setting-name>" is the name of the
* setting that is invalid.
*
* Describes errors that may result from operations involving a #NMConnection.
*
**/
typedef enum
{
NM_CONNECTION_ERROR_UNKNOWN = 0, /*< nick=UnknownError >*/
NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND, /*< nick=ConnectionSettingNotFound >*/
NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID, /*< nick=ConnectionTypeInvalid >*/
*/
typedef enum {
NM_CONNECTION_ERROR_FAILED = 0, /*< nick=Failed >*/
NM_CONNECTION_ERROR_SETTING_NOT_FOUND, /*< nick=SettingNotFound >*/
NM_CONNECTION_ERROR_MISSING_SETTING, /*< nick=MissingSetting >*/
NM_CONNECTION_ERROR_INVALID_SETTING, /*< nick=InvalidSetting >*/
} NMConnectionError;

View file

@ -162,7 +162,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
s_con = NM_SETTING_CONNECTION (_nm_setting_find_in_list_required (all_settings,
NM_SETTING_CONNECTION_SETTING_NAME,
error, NULL, NULL));
error));
if (!s_con)
return FALSE;

View file

@ -840,7 +840,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
}
base_type = nm_setting_lookup_type (priv->type);
if (base_type == G_TYPE_INVALID) {
if (base_type == G_TYPE_INVALID || !_nm_setting_type_is_base_type (base_type)) {
g_set_error (error,
NM_SETTING_CONNECTION_ERROR,
NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY,
@ -850,16 +850,6 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
return FALSE;
}
if (!_nm_setting_type_is_base_type (base_type)) {
g_set_error (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID,
_("connection type '%s' is not a valid base type"),
priv->type);
g_prefix_error (error, "%s.%s: ", NM_SETTING_CONNECTION_SETTING_NAME, NM_SETTING_CONNECTION_TYPE);
return FALSE;
}
/* Make sure the corresponding 'type' item is present */
if ( all_settings
&& !nm_setting_find_in_list (all_settings, priv->type)) {

View file

@ -95,9 +95,7 @@ NMSetting *nm_setting_find_in_list (GSList *settings_list, const char *setting_n
NMSetting * _nm_setting_find_in_list_required (GSList *all_settings,
const char *setting_name,
GError **error,
const char *error_prefix_setting_name,
const char *error_prefix_property_name);
GError **error);
NMSettingVerifyResult _nm_setting_verify_required_virtual_interface_name (GSList *all_settings,
GError **error);

View file

@ -105,7 +105,7 @@ verify (NMSetting *setting, GSList *all_settings, GError **error)
s_con = NM_SETTING_CONNECTION (_nm_setting_find_in_list_required (all_settings,
NM_SETTING_CONNECTION_SETTING_NAME,
error, NULL, NULL));
error));
if (!s_con)
return FALSE;

View file

@ -1743,28 +1743,21 @@ nm_setting_to_string (NMSetting *setting)
NMSetting *
_nm_setting_find_in_list_required (GSList *all_settings,
const char *setting_name,
GError **error,
const char *error_prefix_setting_name,
const char *error_prefix_property_name)
GError **error)
{
NMSetting *setting;
g_return_val_if_fail (!error || !*error, NULL);
g_return_val_if_fail (all_settings, NULL);
g_return_val_if_fail (setting_name, NULL);
g_return_val_if_fail (!error_prefix_setting_name == !error_prefix_property_name, NULL);
setting = nm_setting_find_in_list (all_settings, setting_name);
if (!setting) {
g_set_error (error,
NM_CONNECTION_ERROR,
!strcmp (setting_name, NM_SETTING_CONNECTION_SETTING_NAME)
? NM_CONNECTION_ERROR_CONNECTION_SETTING_NOT_FOUND
: NM_CONNECTION_ERROR_SETTING_NOT_FOUND,
_("Missing '%s' setting"),
setting_name);
if (error_prefix_setting_name)
g_prefix_error (error, "%s.%s: ", error_prefix_setting_name, error_prefix_property_name);
g_set_error_literal (error,
NM_CONNECTION_ERROR,
NM_CONNECTION_ERROR_MISSING_SETTING,
_("missing setting"));
g_prefix_error (error, "%s: ", setting_name);
}
return setting;
}

View file

@ -1965,7 +1965,8 @@ test_connection_bad_base_types (void)
nm_connection_add_setting (connection, setting);
success = nm_connection_verify (connection, &error);
g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID);
g_assert_error (error, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY);
g_assert (g_str_has_prefix (error->message, "connection.type: "));
g_assert (success == FALSE);
g_object_unref (connection);
g_clear_error (&error);
@ -1979,7 +1980,8 @@ test_connection_bad_base_types (void)
nm_connection_add_setting (connection, setting);
success = nm_connection_verify (connection, &error);
g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID);
g_assert_error (error, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY);
g_assert (g_str_has_prefix (error->message, "connection.type: "));
g_assert (success == FALSE);
g_object_unref (connection);
g_clear_error (&error);
@ -1993,7 +1995,8 @@ test_connection_bad_base_types (void)
nm_connection_add_setting (connection, setting);
success = nm_connection_verify (connection, &error);
g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID);
g_assert_error (error, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY);
g_assert (g_str_has_prefix (error->message, "connection.type: "));
g_assert (success == FALSE);
g_object_unref (connection);
g_clear_error (&error);
@ -2005,7 +2008,8 @@ test_connection_bad_base_types (void)
nm_connection_add_setting (connection, setting);
success = nm_connection_verify (connection, &error);
g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID);
g_assert_error (error, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY);
g_assert (g_str_has_prefix (error->message, "connection.type: "));
g_assert (success == FALSE);
g_object_unref (connection);
g_clear_error (&error);
@ -2017,7 +2021,8 @@ test_connection_bad_base_types (void)
nm_connection_add_setting (connection, setting);
success = nm_connection_verify (connection, &error);
g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_CONNECTION_TYPE_INVALID);
g_assert_error (error, NM_SETTING_CONNECTION_ERROR, NM_SETTING_CONNECTION_ERROR_INVALID_PROPERTY);
g_assert (g_str_has_prefix (error->message, "connection.type: "));
g_assert (success == FALSE);
g_object_unref (connection);
g_clear_error (&error);

View file

@ -607,10 +607,10 @@ test_update_secrets_whole_connection_empty_hash (void)
GError *error = NULL;
gboolean success;
/* Test that updating secrets with an empty hash returns success */
/* Test that updating secrets with an empty connection hash returns success */
connection = wifi_connection_new ();
secrets = g_variant_new_array (G_VARIANT_TYPE ("{sv}"), NULL, 0);
secrets = g_variant_new_array (G_VARIANT_TYPE ("{sa{sv}}"), NULL, 0);
success = nm_connection_update_secrets (connection, NULL, secrets, &error);
g_assert_no_error (error);
g_assert (success == TRUE);
@ -715,25 +715,29 @@ test_update_secrets_null_setting_name_with_setting_hash (void)
secrets = build_wep_secrets (wepkey);
g_test_expect_message ("libnm", G_LOG_LEVEL_CRITICAL,
"*nm_connection_update_secrets*setting_name != NULL || full_connection*");
success = nm_connection_update_secrets (connection, NULL, secrets, &error);
g_assert_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_SETTING);
g_test_assert_expected_messages ();
g_assert_no_error (error);
g_assert (!success);
g_variant_unref (secrets);
g_object_unref (connection);
}
int main (int argc, char **argv)
NMTST_DEFINE ();
int
main (int argc, char **argv)
{
GError *error = NULL;
char *base;
#if !GLIB_CHECK_VERSION (2, 35, 0)
g_type_init ();
#endif
if (!nm_utils_init (&error))
FAIL ("nm-utils-init", "failed to initialize libnm: %s", error->message);
nmtst_init (&argc, &argv, TRUE);
/* The tests */
test_need_tls_secrets_path ();