From 9a229241f96ce48f77baa442b1b6aeac9d8489cb Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 1 Aug 2019 17:24:55 +0200
Subject: [PATCH 01/21] shared: fix non-serious bug with bogus condition in
 assertion in nm_key_file_db_ref()

---
 shared/nm-glib-aux/nm-keyfile-aux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/shared/nm-glib-aux/nm-keyfile-aux.c b/shared/nm-glib-aux/nm-keyfile-aux.c
index 0257bcca7f..989c773f23 100644
--- a/shared/nm-glib-aux/nm-keyfile-aux.c
+++ b/shared/nm-glib-aux/nm-keyfile-aux.c
@@ -125,7 +125,7 @@ nm_key_file_db_ref (NMKeyFileDB *self)
 
 	g_return_val_if_fail (_IS_KEY_FILE_DB (self, FALSE, TRUE), NULL);
 
-	nm_assert (self->ref_count <= G_MAXUINT);
+	nm_assert (self->ref_count < G_MAXUINT);
 	self->ref_count++;
 	return self;
 }

From 2ea3c237235312f75498bd55deca128036029b31 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 1 Aug 2019 17:28:58 +0200
Subject: [PATCH 02/21] core/trivial: fix whitespace

---
 src/platform/wifi/nm-wifi-utils-nl80211.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/platform/wifi/nm-wifi-utils-nl80211.c b/src/platform/wifi/nm-wifi-utils-nl80211.c
index 164dada4e9..c4a8bd8a7d 100644
--- a/src/platform/wifi/nm-wifi-utils-nl80211.c
+++ b/src/platform/wifi/nm-wifi-utils-nl80211.c
@@ -284,8 +284,8 @@ nl80211_get_wake_on_wlan_handler (struct nl_msg *msg, void *arg)
 	struct genlmsghdr *gnlh = nlmsg_data (nlmsg_hdr (msg));
 
 	nla_parse_arr (attrs,
-	               genlmsg_attrdata(gnlh, 0),
-	               genlmsg_attrlen(gnlh, 0),
+	               genlmsg_attrdata (gnlh, 0),
+	               genlmsg_attrlen (gnlh, 0),
 	               NULL);
 
 	if (!attrs[NL80211_ATTR_WOWLAN_TRIGGERS])
@@ -363,7 +363,7 @@ wifi_nl80211_set_wake_on_wlan (NMWifiUtils *data, NMSettingWirelessWakeOnWLan wo
 	if (NM_FLAGS_HAS (wowl, NM_SETTING_WIRELESS_WAKE_ON_WLAN_RFKILL_RELEASE))
 		NLA_PUT_FLAG (msg, NL80211_WOWLAN_TRIG_RFKILL_RELEASE);
 
-	nla_nest_end(msg, triggers);
+	nla_nest_end (msg, triggers);
 
 	err = nl80211_send_and_recv (self, msg, NULL, NULL);
 

From 210d7eb5282ca7c21d5d1fd5b10ee37d297fddf2 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 1 Aug 2019 17:38:10 +0200
Subject: [PATCH 03/21] ifcfg-rh: drop unreachable code in make_wpa_setting()

This triggers a coverity warning because we above already
check that not all relevant keys are NULL together.

Work around warning by modifying the code.
---
 src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
index 900a3fc1de..2e0221420b 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
@@ -3629,7 +3629,10 @@ make_wpa_setting (shvarFile *ifcfg,
 	wpa_sae = nm_streq0 (v, "SAE");
 	wpa_eap = nm_streq0 (v, "WPA-EAP");
 	ieee8021x = nm_streq0 (v, "IEEE8021X");
-	if (!wpa_psk && !wpa_sae && !wpa_eap && !ieee8021x)
+	if (   !wpa_psk
+	    && !wpa_sae
+	    && !wpa_eap
+	    && !ieee8021x)
 		return NULL; /* Not WPA or Dynamic WEP */
 
 	/* WPS */
@@ -3693,7 +3696,9 @@ make_wpa_setting (shvarFile *ifcfg,
 			g_object_set (wsec, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, "sae", NULL);
 		else
 			g_assert_not_reached ();
-	} else if (wpa_eap || ieee8021x) {
+	} else {
+		nm_assert (wpa_eap || ieee8021x);
+
 		/* Adhoc mode is mutually exclusive with any 802.1x-based authentication */
 		if (adhoc) {
 			g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
@@ -3710,10 +3715,6 @@ make_wpa_setting (shvarFile *ifcfg,
 
 			g_object_set (wsec, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, lower, NULL);
 		}
-	} else {
-		g_set_error (error, NM_SETTINGS_ERROR, NM_SETTINGS_ERROR_INVALID_CONNECTION,
-		             "Unknown wireless KEY_MGMT type '%s'", v);
-		return NULL;
 	}
 
 	i_val = NM_SETTING_WIRELESS_SECURITY_PMF_DEFAULT;

From 43575513ca91993c5a5995d492361c477b05a99d Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 1 Aug 2019 17:40:39 +0200
Subject: [PATCH 04/21] ifcfg-rh: drop g_assert_not_reached() that clearly
 cannot be reached

Use nm_assert() which is disabled in production builds.
---
 src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
index 2e0221420b..9c3ae10af8 100644
--- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
+++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c
@@ -3692,10 +3692,10 @@ make_wpa_setting (shvarFile *ifcfg,
 			g_object_set (wsec, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, "wpa-none", NULL);
 		else if (wpa_psk)
 			g_object_set (wsec, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, "wpa-psk", NULL);
-		else if (wpa_sae)
+		else {
+			nm_assert (wpa_sae);
 			g_object_set (wsec, NM_SETTING_WIRELESS_SECURITY_KEY_MGMT, "sae", NULL);
-		else
-			g_assert_not_reached ();
+		}
 	} else {
 		nm_assert (wpa_eap || ieee8021x);
 

From 026739eb9faf45ade38cf450430c26d638445453 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 1 Aug 2019 17:45:43 +0200
Subject: [PATCH 05/21] core: fix coverity warning about memset() non-char
 value in assertion

  CID 202432 (#1 of 1): Memset fill truncated (NO_EFFECT)
  bad_memset: Argument -559030611 in memset loses precision in memset(priv->connections_cached_list, -559030611, 8UL * (priv->connections_len + 1U)).
---
 src/settings/nm-settings.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c
index 430d277693..9a17d4774c 100644
--- a/src/settings/nm-settings.c
+++ b/src/settings/nm-settings.c
@@ -2866,7 +2866,7 @@ _clear_connections_cached_list (NMSettingsPrivate *priv)
 	 * it. That is a bug, this code just tries to make it blow up
 	 * more eagerly. */
 	memset (priv->connections_cached_list,
-	        0xdeaddead,
+	        0x43,
 	        sizeof (NMSettingsConnection *) * (priv->connections_len + 1));
 #endif
 

From 243458836adcdb4652bcb5871b53e6f65b510985 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 1 Aug 2019 17:48:48 +0200
Subject: [PATCH 06/21] platform: avoid coverity warning about not checking
 nla_nest_start() result

Usually we check the result of nla_nest_start(). Also, in most cases where this
function would return %NULL, it's an actual bug. That is, because our netlink
message is allocated with a large buffer, and in most cases we append there a well
known, small amount of data.

To make coverity happy, handle the case and assert.
---
 src/platform/wifi/nm-wifi-utils-nl80211.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/platform/wifi/nm-wifi-utils-nl80211.c b/src/platform/wifi/nm-wifi-utils-nl80211.c
index c4a8bd8a7d..2f30b05a2e 100644
--- a/src/platform/wifi/nm-wifi-utils-nl80211.c
+++ b/src/platform/wifi/nm-wifi-utils-nl80211.c
@@ -347,6 +347,8 @@ wifi_nl80211_set_wake_on_wlan (NMWifiUtils *data, NMSettingWirelessWakeOnWLan wo
 		return FALSE;
 
 	triggers = nla_nest_start (msg, NL80211_ATTR_WOWLAN_TRIGGERS);
+	if (!triggers)
+		goto nla_put_failure;
 
 	if (NM_FLAGS_HAS (wowl, NM_SETTING_WIRELESS_WAKE_ON_WLAN_ANY))
 		NLA_PUT_FLAG (msg, NL80211_WOWLAN_TRIG_ANY);

From 990a7bee9d35c19420fe87c44293e5c125cde2e5 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Thu, 1 Aug 2019 17:48:48 +0200
Subject: [PATCH 07/21] platform: drop checks for failure of
 nl80211_alloc_msg()

nl80211_alloc_msg() just allocates some memory, using glib's allocators.
Hence it cannot fail, and we don't need to check for that.

Drop the unnecessary %NULL checks.
---
 src/platform/wifi/nm-wifi-utils-nl80211.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/src/platform/wifi/nm-wifi-utils-nl80211.c b/src/platform/wifi/nm-wifi-utils-nl80211.c
index 2f30b05a2e..84a93f4f6b 100644
--- a/src/platform/wifi/nm-wifi-utils-nl80211.c
+++ b/src/platform/wifi/nm-wifi-utils-nl80211.c
@@ -343,8 +343,6 @@ wifi_nl80211_set_wake_on_wlan (NMWifiUtils *data, NMSettingWirelessWakeOnWLan wo
 		return TRUE;
 
 	msg = nl80211_alloc_msg (self, NL80211_CMD_SET_WOWLAN, 0);
-	if (!msg)
-		return FALSE;
 
 	triggers = nla_nest_start (msg, NL80211_ATTR_WOWLAN_TRIGGERS);
 	if (!triggers)
@@ -636,14 +634,12 @@ nl80211_get_ap_info (NMWifiUtilsNl80211 *self,
 		return;
 
 	msg = nl80211_alloc_msg (self, NL80211_CMD_GET_STATION, 0);
-	if (msg) {
-		NLA_PUT (msg, NL80211_ATTR_MAC, ETH_ALEN, bss_info.bssid);
+	NLA_PUT (msg, NL80211_ATTR_MAC, ETH_ALEN, bss_info.bssid);
 
-		nl80211_send_and_recv (self, msg, nl80211_station_handler, sta_info);
-		if (!sta_info->signal_valid) {
-			/* Fall back to bss_info signal quality (both are in percent) */
-			sta_info->signal = bss_info.beacon_signal;
-		}
+	nl80211_send_and_recv (self, msg, nl80211_station_handler, sta_info);
+	if (!sta_info->signal_valid) {
+		/* Fall back to bss_info signal quality (both are in percent) */
+		sta_info->signal = bss_info.beacon_signal;
 	}
 
 	return;

From bee0b20e3f04f93fe2ae906141c8973110b94667 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 2 Aug 2019 08:29:18 +0200
Subject: [PATCH 08/21] cli: use gs_free_error in nmcli's "connections.c"

---
 clients/cli/connections.c | 60 +++++++++++++++++++++------------------
 1 file changed, 32 insertions(+), 28 deletions(-)

diff --git a/clients/cli/connections.c b/clients/cli/connections.c
index 806e3504c1..c88fbd2abb 100644
--- a/clients/cli/connections.c
+++ b/clients/cli/connections.c
@@ -1298,17 +1298,16 @@ got_secrets (GObject *source_object, GAsyncResult *res, gpointer user_data)
 {
 	NMRemoteConnection *remote = NM_REMOTE_CONNECTION (source_object);
 	GetSecretsData *data = user_data;
-	GVariant *secrets;
-	GError *error = NULL;
+	gs_unref_variant GVariant *secrets = NULL;
 
 	secrets = nm_remote_connection_get_secrets_finish (remote, res, NULL);
 	if (secrets) {
+		gs_free_error GError *error = NULL;
+
 		if (!nm_connection_update_secrets (data->local, NULL, secrets, &error) && error) {
 			g_printerr (_("Error updating secrets for %s: %s\n"),
 			            data->setting_name, error->message);
-			g_clear_error (&error);
 		}
-		g_variant_unref (secrets);
 	}
 
 	g_main_loop_quit (data->loop);
@@ -2764,6 +2763,7 @@ parse_passwords (const char *passwd_file, GError **error)
 
 		g_hash_table_insert (pwds_hash, pwd_spec, g_strdup (pwd));
 	}
+
 	return g_steal_pointer (&pwds_hash);
 }
 
@@ -2783,22 +2783,24 @@ nmc_activate_connection (NmCli *nmc,
 	NMDevice *device = NULL;
 	const char *spec_object = NULL;
 	gboolean device_found;
-	GError *local = NULL;
 
 	g_return_val_if_fail (nmc, FALSE);
 	g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
 
-	if (connection && (ifname || ap || nsp)) {
+	if (   connection
+	    && (   ifname
+	        || ap
+	        || nsp)) {
+		gs_free_error GError *local = NULL;
+
 		device_found = find_device_for_connection (nmc, connection, ifname, ap, nsp, &device, &spec_object, &local);
 
 		/* Virtual connection may not have their interfaces created yet */
 		if (!device_found && !nm_connection_is_virtual (connection)) {
 			g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_CON_ACTIVATION,
 			             "%s", local->message);
-			g_clear_error (&local);
 			return FALSE;
 		}
-		g_clear_error (&local);
 	} else if (ifname) {
 		device = nm_client_get_device_by_iface (nmc->client, ifname);
 		if (!device) {
@@ -2813,11 +2815,10 @@ nmc_activate_connection (NmCli *nmc,
 	}
 
 	/* Parse passwords given in passwords file */
-	pwds_hash = parse_passwords (pwds, &local);
-	if (local) {
-		g_propagate_error (error, local);
+	pwds_hash = parse_passwords (pwds, error);
+	if (!pwds_hash)
 		return FALSE;
-	}
+
 	if (nmc->pwds_hash)
 		g_hash_table_destroy (nmc->pwds_hash);
 	nmc->pwds_hash = pwds_hash;
@@ -7573,18 +7574,19 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t
 					}
 				} else {
 					gs_free char *prop_name = NULL;
-					GError *tmp_err = NULL;
+					gs_free GError *tmp_err = NULL;
 
 					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)) {
-							g_print (_("Error: failed to remove value of '%s': %s\n"), prop_name,
+							g_print (_("Error: failed to remove value of '%s': %s\n"),
+							         prop_name,
 							         tmp_err->message);
-							g_clear_error (&tmp_err);
 						}
 					} else {
-						/* If the string is not a property, try it as a setting */
 						NMSetting *s_tmp;
+
+						/* If the string is not a property, try it as a setting */
 						s_tmp = is_setting_valid (connection,
 						                          valid_settings_main,
 						                          valid_settings_slave,
@@ -7592,16 +7594,17 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t
 						if (s_tmp) {
 							/* Remove setting from the connection */
 							connection_remove_setting (connection, s_tmp);
+
 							/* coverity[copy_paste_error] - suppress Coverity COPY_PASTE_ERROR defect */
 							if (ss == menu_ctx.curr_setting) {
 								/* If we removed the setting we are in, go up */
 								menu_switch_to_level0 (&nmc->nmc_config, &menu_ctx, BASE_PROMPT);
 								nmc_tab_completion.setting = NULL;  /* for TAB completion */
 							}
-						} else
+						} else {
 							g_print (_("Error: %s properties, nor it is a setting name.\n"),
 							         tmp_err->message);
-						g_clear_error (&tmp_err);
+						}
 					}
 				}
 			}
@@ -7661,7 +7664,7 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t
 					/* Show description for all properties */
 					print_setting_description (ss);
 				} else {
-					GError *tmp_err = NULL;
+					gs_free_error GError *tmp_err = NULL;
 					gs_free char *prop_name = NULL;
 
 					prop_name = is_property_valid (ss, cmd_arg_p, &tmp_err);
@@ -7678,11 +7681,11 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t
 						                          cmd_arg_p);
 						if (s_tmp)
 							print_setting_description (s_tmp);
-						else
+						else {
 							g_print (_("Error: invalid property: %s, "
 							           "neither a valid setting name.\n"),
 							         tmp_err->message);
-						g_clear_error (&tmp_err);
+						}
 					}
 				}
 			}
@@ -7769,19 +7772,21 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t
 
 			if (   menu_ctx.curr_setting
 			    && (!cmd_arg || strcmp (cmd_arg, "all") != 0)) {
-				GError *tmp_err = NULL;
-				(void) nm_setting_verify (menu_ctx.curr_setting, NULL, &tmp_err);
+				gs_free_error GError *tmp_err = NULL;
+
+				nm_setting_verify (menu_ctx.curr_setting, NULL, &tmp_err);
 				g_print (_("Verify setting '%s': %s\n"),
 				         nm_setting_get_name (menu_ctx.curr_setting),
 				         tmp_err ? tmp_err->message : "OK");
-				g_clear_error (&tmp_err);
 			} else {
-				GError *tmp_err = NULL;
-				gboolean valid, modified;
+				gs_free_error GError *tmp_err = NULL;
 				gboolean fixed = TRUE;
+				gboolean modified;
+				gboolean valid;
 
 				valid = nm_connection_verify (connection, &tmp_err);
-				if (!valid && (g_strcmp0 (cmd_arg, "fix") == 0)) {
+				if (   !valid
+				    && nm_streq0 (cmd_arg, "fix")) {
 					/* Try to fix normalizable errors */
 					g_clear_error (&tmp_err);
 					fixed = nm_connection_normalize (connection, NULL, &modified, &tmp_err);
@@ -7790,7 +7795,6 @@ editor_menu_main (NmCli *nmc, NMConnection *connection, const char *connection_t
 				         tmp_err ? tmp_err->message : "OK");
 				if (!fixed)
 					g_print (_("The error cannot be fixed automatically.\n"));
-				g_clear_error (&tmp_err);
 			}
 			break;
 

From ec982ceb8ebc9a2f9c3ee14f89735bce818ee948 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 2 Aug 2019 08:33:32 +0200
Subject: [PATCH 09/21] cli: fix dereferncing NULL pointer in parse_passwords()
 with empty file

Warned by coverity.
---
 clients/cli/connections.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/clients/cli/connections.c b/clients/cli/connections.c
index c88fbd2abb..b8905cc78a 100644
--- a/clients/cli/connections.c
+++ b/clients/cli/connections.c
@@ -2711,7 +2711,6 @@ parse_passwords (const char *passwd_file, GError **error)
 	if (!passwd_file)
 		return g_steal_pointer (&pwds_hash);
 
-	/* Read the passwords file */
 	if (!g_file_get_contents (passwd_file, &contents, &len, &local_err)) {
 		g_set_error (error, NMCLI_ERROR, NMC_RESULT_ERROR_USER_INPUT,
 		             _("failed to read passwd-file '%s': %s"),
@@ -2721,7 +2720,7 @@ parse_passwords (const char *passwd_file, GError **error)
 	}
 
 	strv = nm_utils_strsplit_set (contents, "\r\n");
-	for (iter = strv; *iter; iter++) {
+	for (iter = strv; strv && *iter; iter++) {
 		gs_free char *iter_s = g_strdup (*iter);
 
 		pwd = strchr (iter_s, ':');

From af4a41cc4c1cf17bf9d04477cb6c2ebed5daa6ee Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 2 Aug 2019 08:44:12 +0200
Subject: [PATCH 10/21] cli: fix type for loop variable in
 _get_fcn_vlan_xgress_priority_map()

Coverity correctly points out that nm_setting_vlan_get_num_priorities() can return
a negative value (-1 on assertion). Handle that by using the right integer type.
---
 clients/common/nm-meta-setting-desc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clients/common/nm-meta-setting-desc.c b/clients/common/nm-meta-setting-desc.c
index c1a5c2a7fe..5c71868d06 100644
--- a/clients/common/nm-meta-setting-desc.c
+++ b/clients/common/nm-meta-setting-desc.c
@@ -3755,7 +3755,7 @@ _get_fcn_vlan_xgress_priority_map (ARGS_GET_FCN)
 	NMVlanPriorityMap map_type = _vlan_priority_map_type_from_property_info (property_info);
 	NMSettingVlan *s_vlan = NM_SETTING_VLAN (setting);
 	GString *str = NULL;
-	guint32 i, num;
+	gint32 i, num;
 
 	RETURN_UNSUPPORTED_GET_TYPE ();
 

From b5793b74ca096990f138475820e519f5a3fc69b2 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 2 Aug 2019 08:46:38 +0200
Subject: [PATCH 11/21] libnm: fix assertions in NMSettingVlan's priority API

Most of these functions did not ever return failure. The functions
were assertin that the input was valid (and then returned a special
value). But they did not fail under regular conditions.

Fix the gtk-doc for some of these to not claim to be able to fail.

For some (like nm_setting_vlan_add_priority_str() and
nm_setting_vlan_get_priority()), actually let them fail for valid
input (instead of asserting).
---
 libnm-core/nm-setting-vlan.c | 41 ++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/libnm-core/nm-setting-vlan.c b/libnm-core/nm-setting-vlan.c
index a7debbf1d7..d69a894621 100644
--- a/libnm-core/nm-setting-vlan.c
+++ b/libnm-core/nm-setting-vlan.c
@@ -221,7 +221,7 @@ check_replace_duplicate_priority (GSList *list, guint32 from, guint32 to)
  * the Linux SKB priorities to 802.1p priorities.
  *
  * Returns: %TRUE if the entry was successfully added to the list, or it
- * overwrote the old value, %FALSE if error
+ * overwrote the old value, %FALSE if @str is not a valid mapping.
  */
 gboolean
 nm_setting_vlan_add_priority_str (NMSettingVlan *setting,
@@ -235,11 +235,11 @@ nm_setting_vlan_add_priority_str (NMSettingVlan *setting,
 	g_return_val_if_fail (map == NM_VLAN_INGRESS_MAP || map == NM_VLAN_EGRESS_MAP, FALSE);
 	g_return_val_if_fail (str && str[0], FALSE);
 
-	list = get_map (setting, map);
-
 	item = priority_map_new_from_str (map, str);
 	if (!item)
-		g_return_val_if_reached (FALSE);
+		return FALSE;
+
+	list = get_map (setting, map);
 
 	/* Duplicates get replaced */
 	if (check_replace_duplicate_priority (list, item->from, item->to)) {
@@ -264,7 +264,7 @@ nm_setting_vlan_add_priority_str (NMSettingVlan *setting,
  * #NMSettingVlan:ingress_priority_map or #NMSettingVlan:egress_priority_map
  * properties of this setting.
  *
- * Returns: return the number of ingress/egress priority entries, -1 if error
+ * Returns: return the number of ingress/egress priority entries.
  **/
 gint32
 nm_setting_vlan_get_num_priorities (NMSettingVlan *setting, NMVlanPriorityMap map)
@@ -280,13 +280,13 @@ nm_setting_vlan_get_num_priorities (NMSettingVlan *setting, NMVlanPriorityMap ma
  * @setting: the #NMSettingVlan
  * @map: the type of priority map
  * @idx: the zero-based index of the ingress/egress priority map entry
- * @out_from: (out): on return the value of the priority map's 'from' item
- * @out_to: (out): on return the value of priority map's 'to' item
+ * @out_from: (out) (allow-none): on return the value of the priority map's 'from' item
+ * @out_to: (out) (allow-none): on return the value of priority map's 'to' item
  *
  * Retrieve one of the entries of the #NMSettingVlan:ingress_priority_map
  * or #NMSettingVlan:egress_priority_map properties of this setting.
  *
- * Returns: %TRUE if a priority map was returned, %FALSE if error
+ * Returns: returns %TRUE if @idx is in range. Otherwise %FALSE.
  **/
 gboolean
 nm_setting_vlan_get_priority (NMSettingVlan *setting,
@@ -295,21 +295,23 @@ nm_setting_vlan_get_priority (NMSettingVlan *setting,
                               guint32 *out_from,
                               guint32 *out_to)
 {
-	GSList *list = NULL;
-	NMVlanQosMapping *item = NULL;
+	NMVlanQosMapping *item;
+	GSList *list;
 
 	g_return_val_if_fail (NM_IS_SETTING_VLAN (setting), FALSE);
-	g_return_val_if_fail (map == NM_VLAN_INGRESS_MAP || map == NM_VLAN_EGRESS_MAP, FALSE);
-	g_return_val_if_fail (out_from != NULL, FALSE);
-	g_return_val_if_fail (out_to != NULL, FALSE);
+	g_return_val_if_fail (NM_IN_SET (map, NM_VLAN_INGRESS_MAP, NM_VLAN_EGRESS_MAP), FALSE);
 
 	list = get_map (setting, map);
-	g_return_val_if_fail (idx < g_slist_length (list), FALSE);
-
 	item = g_slist_nth_data (list, idx);
-	g_assert (item);
-	*out_from = item->from;
-	*out_to = item->to;
+
+	if (!item) {
+		NM_SET_OUT (out_from, 0);
+		NM_SET_OUT (out_to, 0);
+		return FALSE;
+	}
+
+	NM_SET_OUT (out_from, item->from);
+	NM_SET_OUT (out_to, item->to);
 	return TRUE;
 }
 
@@ -331,8 +333,7 @@ nm_setting_vlan_get_priority (NMSettingVlan *setting,
  * If @map is #NM_VLAN_EGRESS_MAP then @from is the Linux SKB priority value and
  * @to is the outgoing 802.1q VLAN Priority Code Point (PCP) value.
  *
- * Returns: %TRUE if the new priority mapping was successfully added to the
- * list, %FALSE if error
+ * Returns: %TRUE.
  */
 gboolean
 nm_setting_vlan_add_priority (NMSettingVlan *setting,

From f61e274df973b515b17d958ac877fa78a3a2c2b0 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 2 Aug 2019 08:52:26 +0200
Subject: [PATCH 12/21] libnm: try to avoid coverity warning in assertion()

Coverity thinks that this could be NULL sometimes. Try to check for that
to shut up the warning.
---
 libnm-core/nm-setting-vpn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libnm-core/nm-setting-vpn.c b/libnm-core/nm-setting-vpn.c
index 56098392b5..fc9c5184da 100644
--- a/libnm-core/nm-setting-vpn.c
+++ b/libnm-core/nm-setting-vpn.c
@@ -277,7 +277,7 @@ foreach_item_helper (NMSettingVpn *self,
 	}
 
 	for (i = 0; i < len; i++) {
-		nm_assert (keys[i]);
+		nm_assert (keys && keys[i]);
 		keys[i] = g_strdup (keys[i]);
 	}
 	nm_assert (!keys[i]);

From 186d559d6320f159988a7155f9049c714510c6be Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 2 Aug 2019 08:55:30 +0200
Subject: [PATCH 13/21] libnm: avoid NM_CONST_MAX() in enum definition for
 NMTeamAttribute

This confuses coverity. Just use MAX(). MAX() is usually not preferred
as it evaluates the arguments more than once. But in this case, it is of
course fine.

  CID 202433 (#1 of 1): Unrecoverable parse warning (PARSE_ERROR)1.
  expr_not_constant: expression must have a constant value
---
 libnm-core/nm-team-utils.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libnm-core/nm-team-utils.h b/libnm-core/nm-team-utils.h
index 7da42a3f43..c4631bec9a 100644
--- a/libnm-core/nm-team-utils.h
+++ b/libnm-core/nm-team-utils.h
@@ -61,7 +61,7 @@ typedef enum {
 	NM_TEAM_ATTRIBUTE_PORT_LACP_KEY,
 	_NM_TEAM_ATTRIBUTE_PORT_NUM,
 
-	_NM_TEAM_ATTRIBUTE_NUM = NM_CONST_MAX (_NM_TEAM_ATTRIBUTE_MASTER_NUM, _NM_TEAM_ATTRIBUTE_PORT_NUM),
+	_NM_TEAM_ATTRIBUTE_NUM = MAX (_NM_TEAM_ATTRIBUTE_MASTER_NUM, _NM_TEAM_ATTRIBUTE_PORT_NUM),
 
 } NMTeamAttribute;
 

From 8988a12ade61c23d7442076495e4c6492aa8ccc1 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 2 Aug 2019 09:01:32 +0200
Subject: [PATCH 14/21] core: assert for valid arguments in
 sort_captured_addresses() and _addresses_sort_cmp()

Coverity thinks that the arguments could be %NULL. Add an assertion,
hoping to silence coverity.
---
 src/nm-ip4-config.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c
index 86c8292506..36e75bb2d2 100644
--- a/src/nm-ip4-config.c
+++ b/src/nm-ip4-config.c
@@ -544,6 +544,9 @@ _addresses_sort_cmp (gconstpointer a, gconstpointer b, gpointer user_data)
 	const NMPlatformIP4Address *a2 = NMP_OBJECT_CAST_IP4_ADDRESS (*((const NMPObject **) b));
 	guint32 n1, n2;
 
+	nm_assert (a1);
+	nm_assert (a2);
+
 	/* Sort by address type. For example link local will
 	 * be sorted *after* a global address. */
 	p1 = _addresses_sort_cmp_get_prio (a1->address);
@@ -577,6 +580,9 @@ sort_captured_addresses (const CList *lst_a, const CList *lst_b, gconstpointer u
 	const NMPlatformIP4Address *addr_a = NMP_OBJECT_CAST_IP4_ADDRESS (c_list_entry (lst_a, NMDedupMultiEntry, lst_entries)->obj);
 	const NMPlatformIP4Address *addr_b = NMP_OBJECT_CAST_IP4_ADDRESS (c_list_entry (lst_b, NMDedupMultiEntry, lst_entries)->obj);
 
+	nm_assert (addr_a);
+	nm_assert (addr_b);
+
 	/* Primary addresses first */
 	return NM_FLAGS_HAS (addr_a->n_ifa_flags, IFA_F_SECONDARY) -
 	       NM_FLAGS_HAS (addr_b->n_ifa_flags, IFA_F_SECONDARY);

From 5b9a848a82a756bfa01799a4f32d52af1490ec94 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 2 Aug 2019 09:09:45 +0200
Subject: [PATCH 15/21] device/adsl: restore brfd value on error in
 br2684_assign_vcc()

Warned by coverity: we assert above that brfd is -1, so we must always
restore it to -1 in the error case.

Technically, not a problem because socket() is documented to return
only -1 on error already. Apparently coverity does not believe that.
---
 src/devices/adsl/nm-device-adsl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/devices/adsl/nm-device-adsl.c b/src/devices/adsl/nm-device-adsl.c
index 59c87851d1..536d7cccb7 100644
--- a/src/devices/adsl/nm-device-adsl.c
+++ b/src/devices/adsl/nm-device-adsl.c
@@ -155,6 +155,7 @@ br2684_assign_vcc (NMDeviceAdsl *self, NMSettingAdsl *s_adsl)
 	if (priv->brfd < 0) {
 		errsv = errno;
 		_LOGE (LOGD_ADSL, "failed to open ATM control socket (%d)", errsv);
+		priv->brfd = -1;
 		return FALSE;
 	}
 

From 458a2edbb24061bfd9532594e447da2e23819092 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 2 Aug 2019 09:16:34 +0200
Subject: [PATCH 16/21] device/wireguard: fix explicit_bzero() call on peers
 buffer in link_config()

Correctly warned by coverity.
---
 src/devices/nm-device-wireguard.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/devices/nm-device-wireguard.c b/src/devices/nm-device-wireguard.c
index f4da539356..d08f1afd47 100644
--- a/src/devices/nm-device-wireguard.c
+++ b/src/devices/nm-device-wireguard.c
@@ -1467,7 +1467,7 @@ link_config (NMDeviceWireGuard *self,
 	                                       plpeers_len,
 	                                       wg_change_flags);
 
-	nm_explicit_bzero (plpeers, sizeof (plpeers) * plpeers_len);
+	nm_explicit_bzero (plpeers, sizeof (plpeers[0]) * plpeers_len);
 
 	if (r < 0) {
 		NM_SET_OUT (out_failure_reason, NM_DEVICE_STATE_REASON_CONFIG_FAILED);

From e001424ae280ac71d23f5ca40787a190fd35f612 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 2 Aug 2019 09:19:03 +0200
Subject: [PATCH 17/21] libnm/keyfile: silence "Identical code for different
 branches" complaint in _read_setting_wireguard_peer()

That both branches of the "if" do the same, looks suspicious to Coverity.
Work around it by not doing it.
---
 libnm-core/nm-keyfile.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/libnm-core/nm-keyfile.c b/libnm-core/nm-keyfile.c
index 85a9ea11cb..b0c7a13478 100644
--- a/libnm-core/nm-keyfile.c
+++ b/libnm-core/nm-keyfile.c
@@ -3394,10 +3394,9 @@ _read_setting_wireguard_peer (KeyfileReaderInfo *info)
 		return;
 
 	if (!nm_wireguard_peer_is_valid (peer, TRUE, TRUE, &error)) {
-		if (!handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN,
-		                  _("peer '%s' is invalid: %s"),
-		                  info->group, error->message))
-			return;
+		handle_warn (info, key, NM_KEYFILE_WARN_SEVERITY_WARN,
+		             _("peer '%s' is invalid: %s"),
+		             info->group, error->message);
 		return;
 	}
 

From 4596d7793cee22228e68d102fa199ed1736a2050 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 2 Aug 2019 09:24:01 +0200
Subject: [PATCH 18/21] initrd: avoid coverity warning in parse_ip() about
 "Dereference before null check"

get_word() only moves the "argument" pointer forward. It never sets it
to %NULL. Also, above we already dereference argument, so Coverity thinks
that this NULL check indicates a bug.

Drop it to silence Coverity.
---
 src/initrd/nmi-cmdline-reader.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/initrd/nmi-cmdline-reader.c b/src/initrd/nmi-cmdline-reader.c
index ccdd1f2992..7a3af8d661 100644
--- a/src/initrd/nmi-cmdline-reader.c
+++ b/src/initrd/nmi-cmdline-reader.c
@@ -244,7 +244,7 @@ parse_ip (GHashTable *connections, const char *sysfs_dir, char *argument)
 			dns[0] = tmp;
 			dns[1] = get_word (&argument, ':');
 			dns_addr_family[1] = guess_ip_address_family (dns[1]);
-			if (argument && *argument)
+			if (*argument)
 				_LOGW (LOGD_CORE, "Ignoring extra: '%s'.", argument);
 		} else {
 			mtu = tmp;

From e6fa3ce2dfd45ef3556b31b8f51c88a78c4e4ce4 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 2 Aug 2019 09:27:52 +0200
Subject: [PATCH 19/21] shared: explicitly ignore return value of
 g_utf8_validate()

Coverity doesn't like us ignoring the return value, although
we really only care about the "p" output pointer.

Try casting the result to (void), maybe that silences Coverity.
---
 shared/nm-glib-aux/nm-shared-utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c
index c8a253a668..b2ec547092 100644
--- a/shared/nm-glib-aux/nm-shared-utils.c
+++ b/shared/nm-glib-aux/nm-shared-utils.c
@@ -1939,7 +1939,7 @@ nm_utils_buf_utf8safe_escape (gconstpointer buf, gssize buflen, NMUtilsStrUtf8Sa
 			break;
 
 		s = &p[1];
-		g_utf8_validate (s, buflen, &p);
+		(void) g_utf8_validate (s, buflen, &p);
 	} while (TRUE);
 
 	*to_free = g_string_free (gstr, FALSE);

From d76df4c1398324170f306fc3f28b7910d6972ccc Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 2 Aug 2019 09:30:33 +0200
Subject: [PATCH 20/21] shared: try avoid coverity warning in
 _nm_utils_user_data_unpack()

Coverity says

  CID 202453 (#1 of 1): Wrong sizeof argument (SIZEOF_MISMATCH)suspicious_sizeof:
  Passing argument user_data of type gconstpointer and argument (gsize)nargs * 8UL /* sizeof (gconstpointer) */ to function g_slice_free1 is suspicious.

Let's pass instead the "data" pointer. It's the same, but maybe that
avoids the warning.
---
 shared/nm-glib-aux/nm-shared-utils.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c
index b2ec547092..33749d3a1c 100644
--- a/shared/nm-glib-aux/nm-shared-utils.c
+++ b/shared/nm-glib-aux/nm-shared-utils.c
@@ -2878,7 +2878,7 @@ _nm_utils_user_data_unpack (gpointer user_data, int nargs, ...)
 	}
 	va_end (ap);
 
-	g_slice_free1 (((gsize) nargs) * sizeof (gconstpointer), user_data);
+	g_slice_free1 (((gsize) nargs) * sizeof (gconstpointer), data);
 }
 
 /*****************************************************************************/

From 8fb954b81d6f6ebc5f0b4fda3efa6f02f4c06e8a Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Fri, 2 Aug 2019 09:37:46 +0200
Subject: [PATCH 21/21] shared: refactor nm_utils_g_slist_strlist_cmp() to
 avoid dead-code warning from Coverity

Coverity sees that "return 0" cannot be reached. Refactor the code,
to avoid the warning.
---
 shared/nm-glib-aux/nm-shared-utils.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/shared/nm-glib-aux/nm-shared-utils.c b/shared/nm-glib-aux/nm-shared-utils.c
index 33749d3a1c..9bfd5ae2b8 100644
--- a/shared/nm-glib-aux/nm-shared-utils.c
+++ b/shared/nm-glib-aux/nm-shared-utils.c
@@ -2834,10 +2834,15 @@ nm_utils_g_slist_find_str (const GSList *list,
 int
 nm_utils_g_slist_strlist_cmp (const GSList *a, const GSList *b)
 {
-	for (; a && b; a = a->next, b = b->next)
+	while (TRUE) {
+		if (!a)
+			return !b ? 0 : -1;
+		if (!b)
+			return 1;
 		NM_CMP_DIRECT_STRCMP0 (a->data, b->data);
-	NM_CMP_SELF (a, b);
-	return 0;
+		a = a->next;
+		b = b->next;
+	}
 }
 
 /*****************************************************************************/