platform: assert against valid ifindex and remove duplicate assertions

We want that all code paths assert strictly and gracefully.

That means, if we have function nm_platform_link_get() which calls
nm_platform_link_get_obj(), then we don't need to assert the same things
twice. Don't have the calling function assert itself, if it is obvious
that the first thing that it does, is calling a function that itself
asserts the same conditions.

On the other hand, it simply indicates a bug passing a non-positive
ifindex to any of these platform functions. No longer let
nm_platform_link_get_obj() handle negative ifindex gracefully. Instead,
let it directly pass it to nmp_cache_lookup_link(), which eventually
does a g_return_val_if_fail() check. This quite possible enables
assertions on a lot of code paths. But note that g_return_val_if_fail()
is graceful and does not lead to a crash (unless G_DEBUG=fatal-criticals
is set for debugging).

5 files changed, 14 insertions, 60 deletions

diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
index 6721f774db..0e0acf3114 100644
--- a/src/platform/nm-platform.c
+++ b/src/platform/nm-platform.c
@@ -756,9 +756,6 @@ nm_platform_link_get_obj (NMPlatform *self,
 
 	_CHECK_SELF (self, klass, NULL);
 
-	if (ifindex <= 0)
-		return NULL;
-
 	obj_cache = nmp_cache_lookup_link (nm_platform_get_cache (self), ifindex);
 	if (   !obj_cache
 	    || (   visible_only
@@ -1058,8 +1055,6 @@ nm_platform_link_get_name (NMPlatform *self, int ifindex)
 {
 	const NMPlatformLink *pllink;
 
-	_CHECK_SELF (self, klass, NULL);
-
 	pllink = nm_platform_link_get (self, ifindex);
 	return pllink ? pllink->name : NULL;
 }
@@ -1077,8 +1072,6 @@ nm_platform_link_get_type (NMPlatform *self, int ifindex)
 {
 	const NMPlatformLink *pllink;
 
-	_CHECK_SELF (self, klass, NM_LINK_TYPE_NONE);
-
 	pllink = nm_platform_link_get (self, ifindex);
 	return pllink ? pllink->type : NM_LINK_TYPE_NONE;
 }
@@ -1097,10 +1090,7 @@ nm_platform_link_get_type_name (NMPlatform *self, int ifindex)
 {
 	const NMPObject *obj;
 
-	_CHECK_SELF (self, klass, NULL);
-
 	obj = nm_platform_link_get_obj (self, ifindex, TRUE);
-
 	if (!obj)
 		return NULL;
 
@@ -1221,11 +1211,6 @@ nm_platform_link_get_ifi_flags (NMPlatform *self,
 {
 	const NMPlatformLink *pllink;
 
-	_CHECK_SELF (self, klass, -EINVAL);
-
-	if (ifindex <= 0)
-		return -EINVAL;
-
 	/* include invisible links (only in netlink, not udev). */
 	pllink = NMP_OBJECT_CAST_LINK (nm_platform_link_get_obj (self, ifindex, FALSE));
 	if (!pllink)
@@ -1264,8 +1249,6 @@ nm_platform_link_is_connected (NMPlatform *self, int ifindex)
 {
 	const NMPlatformLink *pllink;
 
-	_CHECK_SELF (self, klass, FALSE);
-
 	pllink = nm_platform_link_get (self, ifindex);
 	return pllink ? pllink->connected : FALSE;
 }
@@ -1331,10 +1314,6 @@ nm_platform_link_get_udev_device (NMPlatform *self, int ifindex)
 {
 	const NMPObject *obj_cache;
 
-	_CHECK_SELF (self, klass, FALSE);
-
-	g_return_val_if_fail (ifindex >= 0, NULL);
-
 	obj_cache = nm_platform_link_get_obj (self, ifindex, FALSE);
 	return obj_cache ? obj_cache->_link.udev.device : NULL;
 }
@@ -1355,10 +1334,6 @@ nm_platform_link_get_user_ipv6ll_enabled (NMPlatform *self, int ifindex)
 {
 	const NMPlatformLink *pllink;
 
-	_CHECK_SELF (self, klass, FALSE);
-
-	g_return_val_if_fail (ifindex >= 0, FALSE);
-
 	pllink = nm_platform_link_get (self, ifindex);
 	if (pllink && pllink->inet6_addr_gen_mode_inv)
 		return _nm_platform_uint8_inv (pllink->inet6_addr_gen_mode_inv) == NM_IN6_ADDR_GEN_MODE_NONE;
@@ -1424,12 +1399,7 @@ nm_platform_link_get_address (NMPlatform *self, int ifindex, size_t *length)
 {
 	const NMPlatformLink *pllink;
 
-	_CHECK_SELF (self, klass, NULL);
-
-	g_return_val_if_fail (ifindex > 0, NULL);
-
 	pllink = nm_platform_link_get (self, ifindex);
-
 	if (   !pllink
 	    || pllink->addr.len <= 0) {
 		NM_SET_OUT (length, 0);
@@ -1649,8 +1619,6 @@ nm_platform_link_get_mtu (NMPlatform *self, int ifindex)
 {
 	const NMPlatformLink *pllink;
 
-	_CHECK_SELF (self, klass, 0);
-
 	pllink = nm_platform_link_get (self, ifindex);
 	return pllink ? pllink->mtu : 0;
 }
@@ -1833,10 +1801,6 @@ nm_platform_link_get_master (NMPlatform *self, int slave)
 {
 	const NMPlatformLink *pllink;
 
-	_CHECK_SELF (self, klass, 0);
-
-	g_return_val_if_fail (slave >= 0, FALSE);
-
 	pllink = nm_platform_link_get (self, slave);
 	return pllink ? pllink->master : 0;
 }
@@ -1882,15 +1846,11 @@ nm_platform_link_get_lnk (NMPlatform *self, int ifindex, NMLinkType link_type, c
 {
 	const NMPObject *obj;
 
-	_CHECK_SELF (self, klass, FALSE);
-
-	NM_SET_OUT (out_link, NULL);
-
-	g_return_val_if_fail (ifindex > 0, NULL);
-
 	obj = nm_platform_link_get_obj (self, ifindex, TRUE);
-	if (!obj)
+	if (!obj) {
+		NM_SET_OUT (out_link, NULL);
 		return NULL;
+	}
 
 	NM_SET_OUT (out_link, &obj->link);
 
@@ -2215,12 +2175,11 @@ gboolean
 nm_platform_link_6lowpan_get_properties (NMPlatform *self, int ifindex, int *out_parent)
 {
 	const NMPlatformLink *plink;
-	_CHECK_SELF (self, klass, FALSE);
 
 	plink = nm_platform_link_get (self, ifindex);
-
 	if (!plink)
 		return FALSE;
+
 	if (plink->type != NM_LINK_TYPE_6LOWPAN)
 		return FALSE;
 
@@ -2823,12 +2782,11 @@ nm_platform_link_veth_get_properties (NMPlatform *self, int ifindex, int *out_pe
 {
 	const NMPlatformLink *plink;
 	int peer_ifindex;
-	_CHECK_SELF (self, klass, FALSE);
 
 	plink = nm_platform_link_get (self, ifindex);
-
 	if (!plink)
 		return FALSE;
+
 	if (plink->type != NM_LINK_TYPE_VETH)
 		return FALSE;
 
@@ -2886,14 +2844,11 @@ nm_platform_link_tun_get_properties (NMPlatform *self,
 	gint64 group;
 	gint64 flags;
 
-	_CHECK_SELF (self, klass, FALSE);
-
-	g_return_val_if_fail (ifindex > 0, FALSE);
-
 	/* we consider also invisible links (those that are not yet in udev). */
 	plobj = nm_platform_link_get_obj (self, ifindex, FALSE);
 	if (!plobj)
 		return FALSE;
+
 	if (NMP_OBJECT_CAST_LINK (plobj)->type != NM_LINK_TYPE_TUN)
 		return FALSE;
 
diff --git a/src/platform/tests/test-cleanup.c b/src/platform/tests/test-cleanup.c
index 12d9181208..962ae16b63 100644
--- a/src/platform/tests/test-cleanup.c
+++ b/src/platform/tests/test-cleanup.c
@@ -128,8 +128,7 @@ _nmtstp_init_tests (int *argc, char ***argv)
 void
 _nmtstp_setup_tests (void)
 {
-	nm_platform_link_delete (NM_PLATFORM_GET, nm_platform_link_get_ifindex (NM_PLATFORM_GET, DEVICE_NAME));
-	g_assert (!nm_platform_link_get_by_ifname (NM_PLATFORM_GET, DEVICE_NAME));
+	nmtstp_link_delete (NM_PLATFORM_GET, -1, -1, DEVICE_NAME, FALSE);
 
 	g_test_add_func ("/internal", test_cleanup_internal);
 	/* FIXME: add external cleanup check */
diff --git a/src/platform/tests/test-common.c b/src/platform/tests/test-common.c
index 46f0a28b3a..8aea3e7d95 100644
--- a/src/platform/tests/test-common.c
+++ b/src/platform/tests/test-common.c
@@ -2076,7 +2076,7 @@ main (int argc, char **argv)
 
 	result = g_test_run ();
 
-	nm_platform_link_delete (NM_PLATFORM_GET, nm_platform_link_get_ifindex (NM_PLATFORM_GET, DEVICE_NAME));
+	nmtstp_link_delete (NM_PLATFORM_GET, -1, -1, DEVICE_NAME, FALSE);
 
 	g_object_unref (NM_PLATFORM_GET);
 	return result;
diff --git a/src/platform/tests/test-common.h b/src/platform/tests/test-common.h
index 140866e86a..c49fc0b87e 100644
--- a/src/platform/tests/test-common.h
+++ b/src/platform/tests/test-common.h
@@ -350,8 +350,8 @@ _nmtstp_env1_wrapper_setup (const NmtstTestData *test_data)
 
 	_LOGT ("TEST[%s]: setup", test_data->testpath);
 
-	nm_platform_link_delete (NM_PLATFORM_GET, nm_platform_link_get_ifindex (NM_PLATFORM_GET, DEVICE_NAME));
-	g_assert (!nm_platform_link_get_by_ifname (NM_PLATFORM_GET, DEVICE_NAME));
+	nmtstp_link_delete (NM_PLATFORM_GET, -1, -1, DEVICE_NAME, FALSE);
+
 	g_assert_cmpint (nm_platform_link_dummy_add (NM_PLATFORM_GET, DEVICE_NAME, NULL), ==, NM_PLATFORM_ERROR_SUCCESS);
 
 	*p_ifindex = nm_platform_link_get_ifindex (NM_PLATFORM_GET, DEVICE_NAME);
diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c
index 6177624c45..fd72a1aa70 100644
--- a/src/platform/tests/test-link.c
+++ b/src/platform/tests/test-link.c
@@ -443,7 +443,7 @@ test_software (NMLinkType link_type, const char *link_typename)
 	accept_signal (link_removed);
 
 	/* Delete again */
-	g_assert (!nm_platform_link_delete (NM_PLATFORM_GET, nm_platform_link_get_ifindex (NM_PLATFORM_GET, DEVICE_NAME)));
+	g_assert (nm_platform_link_get_ifindex (NM_PLATFORM_GET, DEVICE_NAME) <= 0);
 	g_assert (!nm_platform_link_delete (NM_PLATFORM_GET, ifindex));
 
 	/* VLAN: Delete parent */
@@ -2829,9 +2829,9 @@ _nmtstp_init_tests (int *argc, char ***argv)
 void
 _nmtstp_setup_tests (void)
 {
-	nm_platform_link_delete (NM_PLATFORM_GET, nm_platform_link_get_ifindex (NM_PLATFORM_GET, DEVICE_NAME));
-	nm_platform_link_delete (NM_PLATFORM_GET, nm_platform_link_get_ifindex (NM_PLATFORM_GET, SLAVE_NAME));
-	nm_platform_link_delete (NM_PLATFORM_GET, nm_platform_link_get_ifindex (NM_PLATFORM_GET, PARENT_NAME));
+	nmtstp_link_delete (NM_PLATFORM_GET, -1, -1, DEVICE_NAME, FALSE);
+	nmtstp_link_delete (NM_PLATFORM_GET, -1, -1, SLAVE_NAME, FALSE);
+	nmtstp_link_delete (NM_PLATFORM_GET, -1, -1, PARENT_NAME, FALSE);
 	g_assert (!nm_platform_link_get_by_ifname (NM_PLATFORM_GET, DEVICE_NAME));
 	g_assert (!nm_platform_link_get_by_ifname (NM_PLATFORM_GET, SLAVE_NAME));
 	g_assert (!nm_platform_link_get_by_ifname (NM_PLATFORM_GET, PARENT_NAME));