Discussion:
[PATCH] unit: IP pools exist as long as they are referenced
(too old to reply)
Patrik Flykt
2015-10-23 11:59:25 UTC
Permalink
An IP pool exists after it has been created until it is unreferenced.
Make no assumptions that the ip pool address range will not be
re-assigned after it has ben unreferenced.
---

This behavior became obvious after changing the ip pool allocation
with the previous patches. Nevertheless, the unit test assumes things
that cannot be guaranteed and needs fixing.

unit/test-ippool.c | 70 ++++++++++++++++++++++++++----------------------------
1 file changed, 34 insertions(+), 36 deletions(-)

diff --git a/unit/test-ippool.c b/unit/test-ippool.c
index e8d077a..17fac9d 100644
--- a/unit/test-ippool.c
+++ b/unit/test-ippool.c
@@ -253,7 +253,7 @@ static void test_case_4(void)

static void test_case_5(void)
{
- struct connman_ippool *pool;
+ struct connman_ippool *pool1, *pool2;
const char *gateway;
const char *broadcast;
const char *subnet_mask;
@@ -271,14 +271,14 @@ static void test_case_5(void)
g_assert(flag == 0);

/* pool should return 192.168.0.1 now */
- pool = __connman_ippool_create(26, 1, 100, collision_cb, &flag);
- g_assert(pool);
+ pool1 = __connman_ippool_create(26, 1, 100, collision_cb, &flag);
+ g_assert(pool1);

- gateway = __connman_ippool_get_gateway(pool);
- broadcast = __connman_ippool_get_broadcast(pool);
- subnet_mask = __connman_ippool_get_subnet_mask(pool);
- start_ip = __connman_ippool_get_start_ip(pool);
- end_ip = __connman_ippool_get_end_ip(pool);
+ gateway = __connman_ippool_get_gateway(pool1);
+ broadcast = __connman_ippool_get_broadcast(pool1);
+ subnet_mask = __connman_ippool_get_subnet_mask(pool1);
+ start_ip = __connman_ippool_get_start_ip(pool1);
+ end_ip = __connman_ippool_get_end_ip(pool1);

g_assert(gateway);
g_assert(broadcast);
@@ -296,8 +296,6 @@ static void test_case_5(void)
"\tgateway %s broadcast %s mask %s", start_ip, end_ip,
gateway, broadcast, subnet_mask);

- __connman_ippool_unref(pool);
-
/*
* Now create the pool again, we should not get collision
* with existing allocated address.
@@ -305,14 +303,14 @@ static void test_case_5(void)

/* pool should return 192.168.2.1 now */
flag = 0;
- pool = __connman_ippool_create(23, 1, 100, collision_cb, &flag);
- g_assert(pool);
+ pool2 = __connman_ippool_create(23, 1, 100, collision_cb, &flag);
+ g_assert(pool2);

- gateway = __connman_ippool_get_gateway(pool);
- broadcast = __connman_ippool_get_broadcast(pool);
- subnet_mask = __connman_ippool_get_subnet_mask(pool);
- start_ip = __connman_ippool_get_start_ip(pool);
- end_ip = __connman_ippool_get_end_ip(pool);
+ gateway = __connman_ippool_get_gateway(pool2);
+ broadcast = __connman_ippool_get_broadcast(pool2);
+ subnet_mask = __connman_ippool_get_subnet_mask(pool2);
+ start_ip = __connman_ippool_get_start_ip(pool2);
+ end_ip = __connman_ippool_get_end_ip(pool2);

g_assert(gateway);
g_assert(broadcast);
@@ -332,14 +330,15 @@ static void test_case_5(void)

g_assert(flag == 0);

- __connman_ippool_unref(pool);
+ __connman_ippool_unref(pool1);
+ __connman_ippool_unref(pool2);

__connman_ippool_cleanup();
}

static void test_case_6(void)
{
- struct connman_ippool *pool;
+ struct connman_ippool *pool1, *pool2;
const char *gateway;
const char *broadcast;
const char *subnet_mask;
@@ -362,14 +361,14 @@ static void test_case_6(void)
g_assert(flag == 0);

/* pool should return 192.168.2.1 now */
- pool = __connman_ippool_create(26, 1, 100, collision_cb, &flag);
- g_assert(pool);
+ pool1 = __connman_ippool_create(26, 1, 100, collision_cb, &flag);
+ g_assert(pool1);

- gateway = __connman_ippool_get_gateway(pool);
- broadcast = __connman_ippool_get_broadcast(pool);
- subnet_mask = __connman_ippool_get_subnet_mask(pool);
- start_ip = __connman_ippool_get_start_ip(pool);
- end_ip = __connman_ippool_get_end_ip(pool);
+ gateway = __connman_ippool_get_gateway(pool1);
+ broadcast = __connman_ippool_get_broadcast(pool1);
+ subnet_mask = __connman_ippool_get_subnet_mask(pool1);
+ start_ip = __connman_ippool_get_start_ip(pool1);
+ end_ip = __connman_ippool_get_end_ip(pool1);

g_assert(gateway);
g_assert(broadcast);
@@ -387,8 +386,6 @@ static void test_case_6(void)
"\tgateway %s broadcast %s mask %s", start_ip, end_ip,
gateway, broadcast, subnet_mask);

- __connman_ippool_unref(pool);
-
/*
* Now create the pool again, we should not get collision
* with existing allocated address.
@@ -396,14 +393,14 @@ static void test_case_6(void)

/* pool should return 192.168.3.1 now */
flag = 0;
- pool = __connman_ippool_create(23, 1, 100, collision_cb, &flag);
- g_assert(pool);
+ pool2 = __connman_ippool_create(23, 1, 100, collision_cb, &flag);
+ g_assert(pool2);

- gateway = __connman_ippool_get_gateway(pool);
- broadcast = __connman_ippool_get_broadcast(pool);
- subnet_mask = __connman_ippool_get_subnet_mask(pool);
- start_ip = __connman_ippool_get_start_ip(pool);
- end_ip = __connman_ippool_get_end_ip(pool);
+ gateway = __connman_ippool_get_gateway(pool2);
+ broadcast = __connman_ippool_get_broadcast(pool2);
+ subnet_mask = __connman_ippool_get_subnet_mask(pool2);
+ start_ip = __connman_ippool_get_start_ip(pool2);
+ end_ip = __connman_ippool_get_end_ip(pool2);

g_assert(gateway);
g_assert(broadcast);
@@ -428,7 +425,8 @@ static void test_case_6(void)
__connman_ippool_newaddr(25, start_ip, 24);
g_assert(flag == 1);

- __connman_ippool_unref(pool);
+ __connman_ippool_unref(pool1);
+ __connman_ippool_unref(pool2);

__connman_ippool_cleanup();
}
--
2.1.4
Patrik Flykt
2015-10-28 09:13:20 UTC
Permalink
Post by Patrik Flykt
An IP pool exists after it has been created until it is unreferenced.
Make no assumptions that the ip pool address range will not be
re-assigned after it has ben unreferenced.
---
This behavior became obvious after changing the ip pool allocation
with the previous patches. Nevertheless, the unit test assumes things
that cannot be guaranteed and needs fixing.
Applied.

Patrik

Loading...