colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

platform: assert against valid ifindex and remove duplicate assertions

Browse Source 
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).
This commit is contained in:
Thomas Haller
2018-11-29 12:33:14 +01:00
parent da39a0ada3
commit 945c904f95
5 changed files with 14 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

57
src/platform/nm-platform.c
View File

@@ -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;

3
src/platform/tests/test-cleanup.c
View File

@@ -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 */

2
src/platform/tests/test-common.c
View File

@@ -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;

4
src/platform/tests/test-common.h
View File

@@ -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);

8
src/platform/tests/test-link.c
View File

@@ -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));