Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1141?usp=email
to review the following change.
Change subject: Review CMocka assertion usage
......................................................................
Review CMocka assertion usage
Replace some assert_true calls with more specific
assertions. This should improve reporting in case
of problems and also just makes the code nicer.
Change-Id: Ia2f374476c87855bba6c0f9d3e2f28a5fe62a152
Signed-off-by: Frank Lichtenheld <fr...@lichtenheld.com>
---
M tests/unit_tests/openvpn/test_auth_token.c
M tests/unit_tests/openvpn/test_packet_id.c
M tests/unit_tests/openvpn/test_provider.c
M tests/unit_tests/openvpn/test_tls_crypt.c
4 files changed, 27 insertions(+), 29 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/41/1141/1
diff --git a/tests/unit_tests/openvpn/test_auth_token.c
b/tests/unit_tests/openvpn/test_auth_token.c
index 0c5467e..e993409 100644
--- a/tests/unit_tests/openvpn/test_auth_token.c
+++ b/tests/unit_tests/openvpn/test_auth_token.c
@@ -286,9 +286,9 @@
strcpy(ctx->up.password, ctx->multi.auth_token);
assert_int_equal(verify_auth_token(&ctx->up, &ctx->multi, ctx->session),
AUTH_TOKEN_HMAC_OK);
- assert_int_not_equal(0, memcmp(ctx->multi.auth_token_initial +
strlen(SESSION_ID_PREFIX),
- token_sessiona + strlen(SESSION_ID_PREFIX),
- AUTH_TOKEN_SESSION_ID_BASE64_LEN));
+ assert_memory_not_equal(ctx->multi.auth_token_initial +
strlen(SESSION_ID_PREFIX),
+ token_sessiona + strlen(SESSION_ID_PREFIX),
+ AUTH_TOKEN_SESSION_ID_BASE64_LEN);
/* The first token is valid but should trigger the invalid response since
* the session id is not the same */
diff --git a/tests/unit_tests/openvpn/test_packet_id.c
b/tests/unit_tests/openvpn/test_packet_id.c
index d623c3d..85179cc 100644
--- a/tests/unit_tests/openvpn/test_packet_id.c
+++ b/tests/unit_tests/openvpn/test_packet_id.c
@@ -82,9 +82,9 @@
now = 5010;
assert_true(packet_id_write(&data->pis, &data->test_buf, false, false));
- assert_true(data->pis.id == 1);
- assert_true(data->test_buf_data.buf_id == htonl(1));
- assert_true(data->test_buf_data.buf_time == 0);
+ assert_int_equal(data->pis.id, 1);
+ assert_int_equal(data->test_buf_data.buf_id, htonl(1));
+ assert_int_equal(data->test_buf_data.buf_time, 0);
}
static void
@@ -96,8 +96,8 @@
assert_true(packet_id_write(&data->pis, &data->test_buf, true, false));
assert_int_equal(data->pis.id, 1);
assert_int_equal(data->pis.time, now);
- assert_true(data->test_buf_data.buf_id == htonl(1));
- assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
+ assert_int_equal(data->test_buf_data.buf_id, htonl(1));
+ assert_int_equal(data->test_buf_data.buf_time, htonl((uint32_t)now));
}
static void
@@ -108,9 +108,9 @@
data->test_buf.offset = sizeof(packet_id_type);
now = 5010;
assert_true(packet_id_write(&data->pis, &data->test_buf, false, true));
- assert_true(data->pis.id == 1);
- assert_true(data->test_buf_data.buf_id == htonl(1));
- assert_true(data->test_buf_data.buf_time == 0);
+ assert_int_equal(data->pis.id, 1);
+ assert_int_equal(data->test_buf_data.buf_id, htonl(1));
+ assert_int_equal(data->test_buf_data.buf_time, 0);
}
static void
@@ -123,8 +123,8 @@
assert_true(packet_id_write(&data->pis, &data->test_buf, true, true));
assert_int_equal(data->pis.id, 1);
assert_int_equal(data->pis.time, now);
- assert_true(data->test_buf_data.buf_id == htonl(1));
- assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
+ assert_int_equal(data->test_buf_data.buf_id, htonl(1));
+ assert_int_equal(data->test_buf_data.buf_time, htonl((uint32_t)now));
}
static void
@@ -156,8 +156,8 @@
assert_int_equal(data->pis.id, 1);
assert_int_equal(data->pis.time, now);
- assert_true(data->test_buf_data.buf_id == htonl(1));
- assert_true(data->test_buf_data.buf_time == htonl((uint32_t)now));
+ assert_int_equal(data->test_buf_data.buf_id, htonl(1));
+ assert_int_equal(data->test_buf_data.buf_time, htonl((uint32_t)now));
}
static void
diff --git a/tests/unit_tests/openvpn/test_provider.c
b/tests/unit_tests/openvpn/test_provider.c
index 463b394..48adb96 100644
--- a/tests/unit_tests/openvpn/test_provider.c
+++ b/tests/unit_tests/openvpn/test_provider.c
@@ -287,9 +287,9 @@
for (size_t i = 0; i < _countof(pubkeys); i++)
{
pubkey = load_pubkey(pubkeys[i]);
- assert_true(pubkey != NULL);
+ assert_non_null(pubkey);
EVP_PKEY *privkey = xkey_load_management_key(NULL, pubkey);
- assert_true(privkey != NULL);
+ assert_non_null(privkey);
management->settings.flags = MF_EXTERNAL_KEY | MF_EXTERNAL_KEY_PSSPAD;
@@ -384,11 +384,11 @@
for (size_t i = 0; i < _countof(pubkeys); i++)
{
pubkey = load_pubkey(pubkeys[i]);
- assert_true(pubkey != NULL);
+ assert_non_null(pubkey);
EVP_PKEY *privkey =
xkey_load_generic_key(NULL, (void *)dummy, pubkey, xkey_sign,
xkey_free);
- assert_true(privkey != NULL);
+ assert_non_null(privkey);
xkey_sign_called = 0;
xkey_free_called = 0;
diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c
b/tests/unit_tests/openvpn/test_tls_crypt.c
index 596f0e0..6ae26fb 100644
--- a/tests/unit_tests/openvpn/test_tls_crypt.c
+++ b/tests/unit_tests/openvpn/test_tls_crypt.c
@@ -487,9 +487,8 @@
assert_true(tls_crypt_v2_unwrap_client_key(&unwrapped_client_key2,
&unwrap_metadata,
wrapped_client_key,
&ctx->server_keys.decrypt));
- assert_true(0
- == memcmp(ctx->client_key2.keys, unwrapped_client_key2.keys,
- sizeof(ctx->client_key2.keys)));
+ assert_memory_equal(ctx->client_key2.keys, unwrapped_client_key2.keys,
+ sizeof(ctx->client_key2.keys));
}
/**
@@ -511,9 +510,8 @@
assert_true(tls_crypt_v2_unwrap_client_key(&unwrapped_client_key2,
&unwrap_metadata, ctx->wkc,
&ctx->server_keys.decrypt));
- assert_true(0
- == memcmp(ctx->client_key2.keys, unwrapped_client_key2.keys,
- sizeof(ctx->client_key2.keys)));
+ assert_memory_equal(ctx->client_key2.keys, unwrapped_client_key2.keys,
+ sizeof(ctx->client_key2.keys));
assert_true(buf_equal(&ctx->metadata, &unwrap_metadata));
struct tls_wrap_ctx wrap_ctx = {
@@ -563,8 +561,8 @@
ctx->wkc,
&ctx->server_keys.decrypt));
const struct key2 zero = { 0 };
- assert_true(0 == memcmp(&unwrapped_client_key2, &zero, sizeof(zero)));
- assert_true(0 == BLEN(&ctx->unwrapped_metadata));
+ assert_memory_equal(&unwrapped_client_key2, &zero, sizeof(zero));
+ assert_int_equal(0, BLEN(&ctx->unwrapped_metadata));
}
/**
@@ -587,8 +585,8 @@
ctx->wkc,
&ctx->server_keys.decrypt));
const struct key2 zero = { 0 };
- assert_true(0 == memcmp(&unwrapped_client_key2, &zero, sizeof(zero)));
- assert_true(0 == BLEN(&ctx->unwrapped_metadata));
+ assert_memory_equal(&unwrapped_client_key2, &zero, sizeof(zero));
+ assert_int_equal(0, BLEN(&ctx->unwrapped_metadata));
}
static void
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1141?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Ia2f374476c87855bba6c0f9d3e2f28a5fe62a152
Gerrit-Change-Number: 1141
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <fr...@lichtenheld.com>
Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
