From d9d5124997f1fa550b80be74dc151d161b901273 Mon Sep 17 00:00:00 2001 From: Bastien Nocera Date: Wed, 13 Aug 2014 13:46:03 +0200 Subject: [PATCH] network: Don't stop validating when we get an error The previous code would stop validating when an error occurred which meant that when a page contained multiple errors, only the first one(s) to be checked would appear in red. Now, all the errors will appear in red. https://bugzilla.gnome.org/show_bug.cgi?id=734472 --- .../connection-editor/ce-page-ethernet.c | 22 ++++--- .../network/connection-editor/ce-page-ip4.c | 66 ++++++++++++------- .../network/connection-editor/ce-page-ip6.c | 66 ++++++++++++------- .../network/connection-editor/ce-page-wifi.c | 36 +++++----- .../wireless-security/eap-method-fast.c | 16 +++-- .../wireless-security/eap-method-leap.c | 15 +++-- .../wireless-security/eap-method-simple.c | 15 +++-- .../wireless-security/eap-method-tls.c | 42 ++++++++---- panels/network/wireless-security/ws-leap.c | 15 +++-- 9 files changed, 185 insertions(+), 108 deletions(-) diff --git a/panels/network/connection-editor/ce-page-ethernet.c b/panels/network/connection-editor/ce-page-ethernet.c index 3fd1ace69..81542d480 100644 --- a/panels/network/connection-editor/ce-page-ethernet.c +++ b/panels/network/connection-editor/ce-page-ethernet.c @@ -164,27 +164,33 @@ validate (CEPage *page, gboolean invalid = FALSE; GByteArray *ignore; GtkWidget *entry; + gboolean ret = TRUE; entry = gtk_bin_get_child (GTK_BIN (self->device_mac)); if (entry) { ignore = ce_page_entry_to_mac (GTK_ENTRY (entry), ARPHRD_ETHER, &invalid); if (invalid) { widget_set_error (entry); - return FALSE; + ret = FALSE; + } else { + if (ignore) + g_byte_array_free (ignore, TRUE); + widget_unset_error (entry); } - if (ignore) - g_byte_array_free (ignore, TRUE); - widget_unset_error (entry); } ignore = ce_page_entry_to_mac (self->cloned_mac, ARPHRD_ETHER, &invalid); if (invalid) { widget_set_error (GTK_WIDGET (self->cloned_mac)); - return FALSE; + ret = FALSE; + } else { + if (ignore) + g_byte_array_free (ignore, TRUE); + widget_unset_error (GTK_WIDGET (self->cloned_mac)); } - if (ignore) - g_byte_array_free (ignore, TRUE); - widget_unset_error (GTK_WIDGET (self->cloned_mac)); + + if (!ret) + return ret; ui_to_setting (self); diff --git a/panels/network/connection-editor/ce-page-ip4.c b/panels/network/connection-editor/ce-page-ip4.c index a61ff5649..643a7f23e 100644 --- a/panels/network/connection-editor/ce-page-ip4.c +++ b/panels/network/connection-editor/ce-page-ip4.c @@ -676,7 +676,6 @@ parse_netmask (const char *str, guint32 *prefix) static gboolean ui_to_setting (CEPageIP4 *page) { - gboolean valid = FALSE; const gchar *method; gboolean ignore_auto_dns; gboolean ignore_auto_routes; @@ -685,6 +684,7 @@ ui_to_setting (CEPageIP4 *page) GArray *dns_servers = NULL; GPtrArray *routes = NULL; GList *children, *l; + gboolean ret = TRUE; if (!gtk_switch_get_active (page->enabled)) { method = NM_SETTING_IP4_CONFIG_METHOD_DISABLED; @@ -735,21 +735,27 @@ ui_to_setting (CEPageIP4 *page) if (inet_pton (AF_INET, text_address, &tmp_addr) <= 0) { widget_set_error (GTK_WIDGET (entry)); - goto out; + ret = FALSE; + } else { + widget_unset_error (GTK_WIDGET (entry)); } - widget_unset_error (GTK_WIDGET (entry)); if (!parse_netmask (text_netmask, &prefix)) { widget_set_error (g_object_get_data (G_OBJECT (row), "network")); - goto out; + ret = FALSE; + } else { + widget_unset_error (g_object_get_data (G_OBJECT (row), "network")); } - widget_unset_error (g_object_get_data (G_OBJECT (row), "network")); if (text_gateway && *text_gateway && inet_pton (AF_INET, text_gateway, &tmp_gateway) <= 0) { widget_set_error (g_object_get_data (G_OBJECT (row), "gateway")); - goto out; + ret = FALSE; + } else { + widget_unset_error (g_object_get_data (G_OBJECT (row), "gateway")); } - widget_unset_error (g_object_get_data (G_OBJECT (row), "gateway")); + + if (!ret) + continue; addr = g_array_sized_new (FALSE, TRUE, sizeof (guint32), 3); g_array_append_val (addr, tmp_addr.s_addr); @@ -788,11 +794,11 @@ ui_to_setting (CEPageIP4 *page) if (inet_pton (AF_INET, text, &tmp_addr) <= 0) { widget_set_error (GTK_WIDGET (entry)); - goto out; + ret = FALSE; + } else { + widget_unset_error (GTK_WIDGET (entry)); + g_array_append_val (dns_servers, tmp_addr.s_addr); } - widget_unset_error (GTK_WIDGET (entry)); - - g_array_append_val (dns_servers, tmp_addr.s_addr); } g_list_free (children); @@ -826,24 +832,26 @@ ui_to_setting (CEPageIP4 *page) if (inet_pton (AF_INET, text_address, &tmp_addr) <= 0) { widget_set_error (GTK_WIDGET (entry)); - goto out; + ret = FALSE; + } else { + widget_unset_error (GTK_WIDGET (entry)); + address = tmp_addr.s_addr; } - widget_unset_error (GTK_WIDGET (entry)); - - address = tmp_addr.s_addr; if (!parse_netmask (text_netmask, &netmask)) { widget_set_error (GTK_WIDGET (g_object_get_data (G_OBJECT (row), "netmask"))); - goto out; + ret = FALSE; + } else { + widget_unset_error (GTK_WIDGET (g_object_get_data (G_OBJECT (row), "netmask"))); } - widget_unset_error (GTK_WIDGET (g_object_get_data (G_OBJECT (row), "netmask"))); if (inet_pton (AF_INET, text_gateway, &tmp_addr) <= 0) { widget_set_error (GTK_WIDGET (g_object_get_data (G_OBJECT (row), "gateway"))); - goto out; + ret = FALSE; + } else { + widget_unset_error (GTK_WIDGET (g_object_get_data (G_OBJECT (row), "gateway"))); + gateway = tmp_addr.s_addr; } - widget_unset_error (GTK_WIDGET (g_object_get_data (G_OBJECT (row), "gateway"))); - gateway = tmp_addr.s_addr; metric = 0; if (*text_metric) { @@ -851,10 +859,16 @@ ui_to_setting (CEPageIP4 *page) metric = strtoul (text_metric, NULL, 10); if (errno) { widget_set_error (GTK_WIDGET (g_object_get_data (G_OBJECT (row), "metric"))); - goto out; + ret = FALSE; + } else { + widget_unset_error (GTK_WIDGET (g_object_get_data (G_OBJECT (row), "metric"))); } + } else { + widget_unset_error (GTK_WIDGET (g_object_get_data (G_OBJECT (row), "metric"))); } - widget_unset_error (GTK_WIDGET (g_object_get_data (G_OBJECT (row), "metric"))); + + if (!ret) + continue; route = g_array_sized_new (FALSE, TRUE, sizeof (guint32), 4); g_array_append_val (route, address); @@ -869,6 +883,10 @@ ui_to_setting (CEPageIP4 *page) g_ptr_array_free (routes, TRUE); routes = NULL; } + + if (!ret) + goto out; + ignore_auto_dns = !gtk_switch_get_active (page->auto_dns); ignore_auto_routes = !gtk_switch_get_active (page->auto_routes); never_default = gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (page->never_default)); @@ -883,8 +901,6 @@ ui_to_setting (CEPageIP4 *page) NM_SETTING_IP4_CONFIG_NEVER_DEFAULT, never_default, NULL); - valid = TRUE; - out: if (addresses) g_ptr_array_free (addresses, TRUE); @@ -895,7 +911,7 @@ out: if (routes) g_ptr_array_free (routes, TRUE); - return valid; + return ret; } static gboolean diff --git a/panels/network/connection-editor/ce-page-ip6.c b/panels/network/connection-editor/ce-page-ip6.c index d12db9215..077a5d53e 100644 --- a/panels/network/connection-editor/ce-page-ip6.c +++ b/panels/network/connection-editor/ce-page-ip6.c @@ -653,12 +653,12 @@ connect_ip6_page (CEPageIP6 *page) static gboolean ui_to_setting (CEPageIP6 *page) { - gboolean valid = FALSE; const gchar *method; gboolean ignore_auto_dns; gboolean ignore_auto_routes; gboolean never_default; GList *children, *l; + gboolean ret = TRUE; if (!gtk_switch_get_active (page->enabled)) { method = NM_SETTING_IP6_CONFIG_METHOD_IGNORE; @@ -713,26 +713,34 @@ ui_to_setting (CEPageIP6 *page) if (inet_pton (AF_INET6, text_address, &tmp_addr) <= 0) { widget_set_error (GTK_WIDGET (entry)); - goto out; + ret = FALSE; + } else { + widget_unset_error (GTK_WIDGET (entry)); } - widget_unset_error (GTK_WIDGET (entry)); prefix = strtoul (text_prefix, &end, 10); if (!end || *end || prefix == 0 || prefix > 128) { widget_set_error (g_object_get_data (G_OBJECT (row), "prefix")); - goto out; + ret = FALSE; + } else { + widget_unset_error (g_object_get_data (G_OBJECT (row), "prefix")); } - widget_unset_error (g_object_get_data (G_OBJECT (row), "prefix")); if (text_gateway && *text_gateway) { if (inet_pton (AF_INET6, text_gateway, &tmp_gateway) <= 0) { widget_set_error (g_object_get_data (G_OBJECT (row), "gateway")); - goto out; + ret = FALSE; + } else { + if (!IN6_IS_ADDR_UNSPECIFIED (&tmp_gateway)) + have_gateway = TRUE; + widget_unset_error (g_object_get_data (G_OBJECT (row), "gateway")); } - if (!IN6_IS_ADDR_UNSPECIFIED (&tmp_gateway)) - have_gateway = TRUE; + } else { + widget_unset_error (g_object_get_data (G_OBJECT (row), "gateway")); } - widget_unset_error (g_object_get_data (G_OBJECT (row), "gateway")); + + if (!ret) + continue; addr = nm_ip6_address_new (); nm_ip6_address_set_address (addr, &tmp_addr); @@ -764,11 +772,11 @@ ui_to_setting (CEPageIP6 *page) if (inet_pton (AF_INET6, text, &tmp_addr) <= 0) { widget_set_error (GTK_WIDGET (entry)); - goto out; + ret = FALSE; + } else { + widget_unset_error (GTK_WIDGET (entry)); + nm_setting_ip6_config_add_dns (page->setting, &tmp_addr); } - widget_unset_error (GTK_WIDGET (entry)); - - nm_setting_ip6_config_add_dns (page->setting, &tmp_addr); } g_list_free (children); @@ -806,22 +814,25 @@ ui_to_setting (CEPageIP6 *page) if (inet_pton (AF_INET6, text_address, &dest) <= 0) { widget_set_error (GTK_WIDGET (entry)); - goto out; + ret = FALSE; + } else { + widget_unset_error (GTK_WIDGET (entry)); } - widget_unset_error (GTK_WIDGET (entry)); prefix = strtoul (text_prefix, &end, 10); if (!end || *end || prefix == 0 || prefix > 128) { widget_set_error (g_object_get_data (G_OBJECT (row), "prefix")); - goto out; + ret = FALSE; + } else { + widget_unset_error (g_object_get_data (G_OBJECT (row), "prefix")); } - widget_unset_error (g_object_get_data (G_OBJECT (row), "prefix")); if (inet_pton (AF_INET6, text_gateway, &gateway) <= 0) { widget_set_error (g_object_get_data (G_OBJECT (row), "gateway")); - goto out; + ret = FALSE; + } else { + widget_unset_error (g_object_get_data (G_OBJECT (row), "gateway")); } - widget_unset_error (g_object_get_data (G_OBJECT (row), "gateway")); metric = 0; if (*text_metric) { @@ -829,10 +840,16 @@ ui_to_setting (CEPageIP6 *page) metric = strtoul (text_metric, NULL, 10); if (errno) { widget_set_error (g_object_get_data (G_OBJECT (row), "metric")); - goto out; + ret = FALSE; + } else { + widget_unset_error (g_object_get_data (G_OBJECT (row), "metric")); } + } else { + widget_unset_error (g_object_get_data (G_OBJECT (row), "metric")); } - widget_unset_error (g_object_get_data (G_OBJECT (row), "metric")); + + if (!ret) + continue; route = nm_ip6_route_new (); nm_ip6_route_set_dest (route, &dest); @@ -844,6 +861,9 @@ ui_to_setting (CEPageIP6 *page) } g_list_free (children); + if (!ret) + goto out; + ignore_auto_dns = !gtk_switch_get_active (page->auto_dns); ignore_auto_routes = !gtk_switch_get_active (page->auto_routes); never_default = gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (page->never_default)); @@ -855,11 +875,9 @@ ui_to_setting (CEPageIP6 *page) NM_SETTING_IP6_CONFIG_NEVER_DEFAULT, never_default, NULL); - valid = TRUE; - out: - return valid; + return ret; } static gboolean diff --git a/panels/network/connection-editor/ce-page-wifi.c b/panels/network/connection-editor/ce-page-wifi.c index 81d5e34eb..ddd8d9101 100644 --- a/panels/network/connection-editor/ce-page-wifi.c +++ b/panels/network/connection-editor/ce-page-wifi.c @@ -189,39 +189,45 @@ validate (CEPage *page, GtkWidget *entry; GByteArray *ignore; gboolean invalid; - gboolean success; gchar *security; NMSettingWireless *setting; + gboolean ret = TRUE; entry = gtk_bin_get_child (GTK_BIN (gtk_builder_get_object (page->builder, "combo_bssid"))); ignore = ce_page_entry_to_mac (GTK_ENTRY (entry), ARPHRD_ETHER, &invalid); if (invalid) { widget_set_error (entry); - return FALSE; + ret = FALSE; + } else { + if (ignore) + g_byte_array_free (ignore, TRUE); + widget_unset_error (entry); } - if (ignore) - g_byte_array_free (ignore, TRUE); - widget_unset_error (entry); entry = gtk_bin_get_child (GTK_BIN (gtk_builder_get_object (page->builder, "combo_mac"))); ignore = ce_page_entry_to_mac (GTK_ENTRY (entry), ARPHRD_ETHER, &invalid); if (invalid) { widget_set_error (entry); - return FALSE; + ret = FALSE; + } else { + if (ignore) + g_byte_array_free (ignore, TRUE); + widget_unset_error (entry); } - if (ignore) - g_byte_array_free (ignore, TRUE); - widget_unset_error (entry); entry = GTK_WIDGET (gtk_builder_get_object (page->builder, "entry_cloned_mac")); ignore = ce_page_entry_to_mac (GTK_ENTRY (entry), ARPHRD_ETHER, &invalid); if (invalid) { widget_set_error (entry); - return FALSE; + ret = FALSE; + } else { + if (ignore) + g_byte_array_free (ignore, TRUE); + widget_unset_error (entry); } - if (ignore) - g_byte_array_free (ignore, TRUE); - widget_unset_error (entry); + + if (!ret) + return ret; ui_to_setting (CE_PAGE_WIFI (page)); @@ -229,11 +235,11 @@ validate (CEPage *page, setting = CE_PAGE_WIFI (page)->setting; security = g_strdup (nm_setting_wireless_get_security (setting)); g_object_set (setting, NM_SETTING_WIRELESS_SEC, NULL, NULL); - success = nm_setting_verify (NM_SETTING (setting), NULL, error); + ret = nm_setting_verify (NM_SETTING (setting), NULL, error); g_object_set (setting, NM_SETTING_WIRELESS_SEC, security, NULL); g_free (security); - return success; + return ret; } static void diff --git a/panels/network/wireless-security/eap-method-fast.c b/panels/network/wireless-security/eap-method-fast.c index 351305db8..20bbf2f21 100644 --- a/panels/network/wireless-security/eap-method-fast.c +++ b/panels/network/wireless-security/eap-method-fast.c @@ -62,7 +62,7 @@ validate (EAPMethod *parent) EAPMethod *eap = NULL; const char *file; gboolean provisioning; - gboolean valid = FALSE; + gboolean ret = TRUE; widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_fast_pac_provision_checkbutton")); g_assert (widget); @@ -72,9 +72,10 @@ validate (EAPMethod *parent) file = gtk_file_chooser_get_filename (GTK_FILE_CHOOSER (widget)); if (!provisioning && !file) { widget_set_error (widget); - return FALSE; + ret = FALSE; + } else { + widget_unset_error (widget); } - widget_unset_error (widget); widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_fast_inner_auth_combo")); g_assert (widget); @@ -82,9 +83,14 @@ validate (EAPMethod *parent) gtk_combo_box_get_active_iter (GTK_COMBO_BOX (widget), &iter); gtk_tree_model_get (model, &iter, I_METHOD_COLUMN, &eap, -1); g_assert (eap); - valid = eap_method_validate (eap); + if (!eap_method_validate (eap)) { + widget_set_error (widget); + ret = FALSE; + } else { + widget_unset_error (widget); + } eap_method_unref (eap); - return valid; + return ret; } static void diff --git a/panels/network/wireless-security/eap-method-leap.c b/panels/network/wireless-security/eap-method-leap.c index e07ca276c..bcfc2050a 100644 --- a/panels/network/wireless-security/eap-method-leap.c +++ b/panels/network/wireless-security/eap-method-leap.c @@ -52,26 +52,29 @@ validate (EAPMethod *parent) { GtkWidget *widget; const char *text; + gboolean ret = TRUE; widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_leap_username_entry")); g_assert (widget); text = gtk_entry_get_text (GTK_ENTRY (widget)); if (!text || !strlen (text)) { widget_set_error (widget); - return FALSE; + ret = FALSE; + } else { + widget_unset_error (widget); } - widget_unset_error (widget); widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_leap_password_entry")); g_assert (widget); text = gtk_entry_get_text (GTK_ENTRY (widget)); - if (!text || !strlen (text)) { + if (!text || *text == '\0') { widget_set_error (widget); - return FALSE; + ret = FALSE; + } else { + widget_unset_error (widget); } - widget_unset_error (widget); - return TRUE; + return ret; } static void diff --git a/panels/network/wireless-security/eap-method-simple.c b/panels/network/wireless-security/eap-method-simple.c index 521950b56..f3b843d05 100644 --- a/panels/network/wireless-security/eap-method-simple.c +++ b/panels/network/wireless-security/eap-method-simple.c @@ -55,22 +55,24 @@ validate (EAPMethod *parent) { GtkWidget *widget; const char *text; + gboolean ret = TRUE; widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_simple_username_entry")); g_assert (widget); text = gtk_entry_get_text (GTK_ENTRY (widget)); if (!text || !strlen (text)) { widget_set_error (widget); - return FALSE; + ret = FALSE; + } else { + widget_unset_error (widget); } - widget_unset_error (widget); /* Check if the password should always be requested */ widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_password_always_ask")); g_assert (widget); if (gtk_toggle_button_get_active (GTK_TOGGLE_BUTTON (widget))) { widget_unset_error (GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_simple_password_entry"))); - return TRUE; + return ret; } widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_simple_password_entry")); @@ -78,11 +80,12 @@ validate (EAPMethod *parent) text = gtk_entry_get_text (GTK_ENTRY (widget)); if (!text || !strlen (text)) { widget_set_error (widget); - return FALSE; + ret = FALSE; + } else { + widget_unset_error (widget); } - widget_unset_error (widget); - return TRUE; + return ret; } static void diff --git a/panels/network/wireless-security/eap-method-tls.c b/panels/network/wireless-security/eap-method-tls.c index f74c455c4..2069ccb3e 100644 --- a/panels/network/wireless-security/eap-method-tls.c +++ b/panels/network/wireless-security/eap-method-tls.c @@ -59,41 +59,57 @@ validate (EAPMethod *parent) NMSetting8021xCKFormat format = NM_SETTING_802_1X_CK_FORMAT_UNKNOWN; GtkWidget *widget; const char *password, *identity; + gboolean ret = TRUE; widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_tls_identity_entry")); g_assert (widget); identity = gtk_entry_get_text (GTK_ENTRY (widget)); if (!identity || !strlen (identity)) { widget_set_error (widget); - return FALSE; + ret = FALSE; + } else { + widget_unset_error (widget); } - widget_unset_error (widget); - if (!eap_method_validate_filepicker (parent->builder, "eap_tls_ca_cert_button", TYPE_CA_CERT, NULL, NULL)) - return FALSE; + widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_tls_ca_cert_button")); + if (!eap_method_validate_filepicker (parent->builder, "eap_tls_ca_cert_button", TYPE_CA_CERT, NULL, NULL)) { + widget_set_error (widget); + ret = FALSE; + } else { + widget_unset_error (widget); + } widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_tls_private_key_password_entry")); g_assert (widget); password = gtk_entry_get_text (GTK_ENTRY (widget)); if (!password || !strlen (password)) { widget_set_error (widget); - return FALSE; + ret = FALSE; + } else { + widget_unset_error (widget); } - widget_unset_error (widget); + widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_tls_private_key_button")); if (!eap_method_validate_filepicker (parent->builder, "eap_tls_private_key_button", TYPE_PRIVATE_KEY, password, - &format)) - return FALSE; - - if (format != NM_SETTING_802_1X_CK_FORMAT_PKCS12) { - if (!eap_method_validate_filepicker (parent->builder, "eap_tls_user_cert_button", TYPE_CLIENT_CERT, NULL, NULL)) - return FALSE; + &format)) { + widget_set_error (widget); + ret = FALSE; } - return TRUE; + if (format != NM_SETTING_802_1X_CK_FORMAT_PKCS12) { + widget = GTK_WIDGET (gtk_builder_get_object (parent->builder, "eap_tls_user_cert_button")); + if (!eap_method_validate_filepicker (parent->builder, "eap_tls_user_cert_button", TYPE_CLIENT_CERT, NULL, NULL)) { + widget_set_error (widget); + ret = FALSE; + } else { + widget_unset_error (widget); + } + } + + return ret; } static void diff --git a/panels/network/wireless-security/ws-leap.c b/panels/network/wireless-security/ws-leap.c index 5ba632e14..370a55ab3 100644 --- a/panels/network/wireless-security/ws-leap.c +++ b/panels/network/wireless-security/ws-leap.c @@ -49,26 +49,29 @@ validate (WirelessSecurity *parent, const GByteArray *ssid) { GtkWidget *entry; const char *text; + gboolean ret = TRUE; entry = GTK_WIDGET (gtk_builder_get_object (parent->builder, "leap_username_entry")); g_assert (entry); text = gtk_entry_get_text (GTK_ENTRY (entry)); if (!text || !strlen (text)) { widget_set_error (entry); - return FALSE; + ret = FALSE; + } else { + widget_unset_error (entry); } - widget_unset_error (entry); entry = GTK_WIDGET (gtk_builder_get_object (parent->builder, "leap_password_entry")); g_assert (entry); text = gtk_entry_get_text (GTK_ENTRY (entry)); - if (!text || !strlen (text)) { + if (!text || *text == '\0') { widget_set_error (entry); - return FALSE; + ret = FALSE; + } else { + widget_unset_error (entry); } - widget_unset_error (entry); - return TRUE; + return ret; } static void