diff options
author | Thomas Haller <thaller@redhat.com> | 2019-04-17 15:40:32 +0200 |
---|---|---|
committer | Beniamino Galvani <bgalvani@redhat.com> | 2019-04-18 09:53:24 +0200 |
commit | 05a547133b3e98c6c08f52df2923cfacc4ca8400 (patch) | |
tree | 26e28318f0eee78ed25ecce6a0120e0e8c699c6f | |
parent | 6ac953e9b327c77f23752d90876d6dacfc44eeb7 (diff) |
libnm: minor refactoring of _nm_utils_bridge_vlan_verify_list()
- if there is only one vlan in the list, then we can return success
early. That is, because one NMBridgeVlan instance is always valid
due to the way how users must use the API to construct the element.
- the implementation for check_normalizable is only correct, if there
are no duplicate or overlapping ranges. Assert for that. In fact,
all callers first check for errors and then for normalizable errors.
- avoid duplicate calls to nm_bridge_vlan_get_vid_range(). There are
duplicate assertions that we don't need.
- only check for pvid once per range.
- combine calls to g_hash_table_contains() and g_hash_table_add().
(cherry picked from commit a358da096f7b2d195dc3b526a7d21efa9ade55b9)
-rw-r--r-- | libnm-core/nm-utils.c | 48 |
1 files changed, 27 insertions, 21 deletions
diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index ea333dc61c..28bc24b1e1 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -6883,28 +6883,33 @@ _nm_utils_bridge_vlan_verify_list (GPtrArray *vlans, gs_unref_hashtable GHashTable *h = NULL; gboolean pvid_found = FALSE; - if (!vlans || !vlans->len) + if ( !vlans + || vlans->len <= 1) return TRUE; if (check_normalizable) { + guint16 vid_prev_end, vid_start, vid_end; + + nm_assert (_nm_utils_bridge_vlan_verify_list (vlans, FALSE, NULL, setting, property)); + + nm_bridge_vlan_get_vid_range (vlans->pdata[0], NULL, &vid_prev_end); for (i = 1; i < vlans->len; i++) { - NMBridgeVlan *vlan_prev = vlans->pdata[i - 1]; - NMBridgeVlan *vlan = vlans->pdata[i]; - guint16 vid_prev, vid; + const NMBridgeVlan *vlan = vlans->pdata[i]; - nm_bridge_vlan_get_vid_range (vlan_prev, &vid_prev, NULL); - nm_bridge_vlan_get_vid_range (vlan, &vid, NULL); + nm_bridge_vlan_get_vid_range (vlan, &vid_start, &vid_end); - if (vid_prev > vid) { + if (vid_prev_end > vid_start) { g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, _("Bridge VLANs %d and %d are not sorted by ascending vid"), - vid_prev, - vid); + vid_prev_end, + vid_start); g_prefix_error (error, "%s.%s: ", setting, property); return FALSE; } + + vid_prev_end = vid_end; } return TRUE; } @@ -6915,8 +6920,9 @@ _nm_utils_bridge_vlan_verify_list (GPtrArray *vlans, guint16 v, vid_start, vid_end; nm_bridge_vlan_get_vid_range (vlan, &vid_start, &vid_end); + for (v = vid_start; v <= vid_end; v++) { - if (g_hash_table_contains (h, GUINT_TO_POINTER (v))) { + if (!nm_g_hash_table_add (h, GUINT_TO_POINTER (v))) { g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_INVALID_PROPERTY, @@ -6924,19 +6930,19 @@ _nm_utils_bridge_vlan_verify_list (GPtrArray *vlans, g_prefix_error (error, "%s.%s: ", setting, property); return FALSE; } + } - if (nm_bridge_vlan_is_pvid (vlan)) { - if (pvid_found) { - g_set_error_literal (error, - NM_CONNECTION_ERROR, - NM_CONNECTION_ERROR_INVALID_PROPERTY, - _("only one VLAN can be the PVID")); - g_prefix_error (error, "%s.%s: ", setting, property); - return FALSE; - } - pvid_found = TRUE; + if (nm_bridge_vlan_is_pvid (vlan)) { + if ( vid_start != vid_end + || pvid_found) { + g_set_error_literal (error, + NM_CONNECTION_ERROR, + NM_CONNECTION_ERROR_INVALID_PROPERTY, + _("only one VLAN can be the PVID")); + g_prefix_error (error, "%s.%s: ", setting, property); + return FALSE; } - g_hash_table_add (h, GUINT_TO_POINTER (v)); + pvid_found = TRUE; } } |