Skip to content

Commit 75cc34e

Browse files
flichtenheldcron2
authored andcommitted
mbuf: Add unit tests
While fixing the conversion warning I was somewhat confused how this works, so added UTs to verify I understood it. v2: - disable assert test for MS VS - add define for memory-intensive UTs and only enable it by default for CMake builds, so we do not break a lot of builds out there due to memory allocation failures Change-Id: Icab68a5fd1b6288955f0073179f1ddde1468d951 Signed-off-by: Frank Lichtenheld <[email protected]> Acked-by: Gert Doering <[email protected]> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1432 Message-Id: <[email protected]> URL: https://www.mail-archive.com/[email protected]/msg35050.html Signed-off-by: Gert Doering <[email protected]>
1 parent 2b8149a commit 75cc34e

File tree

7 files changed

+209
-5
lines changed

7 files changed

+209
-5
lines changed

CMakeLists.txt

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,7 @@ if (NOT WIN32)
633633
endif ()
634634
endif ()
635635

636+
option(UT_ALLOW_BIG_ALLOC "Allow unit-tests to use > 1 GB of memory" ON)
636637

637638
if (BUILD_TESTING)
638639
find_package(cmocka CONFIG)
@@ -649,6 +650,7 @@ if (BUILD_TESTING)
649650
"test_buffer"
650651
"test_crypto"
651652
"test_dhcp"
653+
"test_mbuf"
652654
"test_misc"
653655
"test_ncp"
654656
"test_options_parse"
@@ -739,6 +741,9 @@ if (BUILD_TESTING)
739741
# for compat with IDEs like Clion that ignore the tests properties
740742
# for the environment variable srcdir when running tests as fallback
741743
target_compile_definitions(${test_name} PRIVATE "UNIT_TEST_SOURCEDIR=\"${_UT_SOURCE_DIR}\"")
744+
if (UT_ALLOW_BIG_ALLOC)
745+
target_compile_definitions(${test_name} PRIVATE UNIT_TEST_ALLOW_BIG_ALLOC)
746+
endif ()
742747

743748
if (NOT ${test_name} STREQUAL "test_buffer")
744749
target_sources(${test_name} PRIVATE
@@ -800,6 +805,12 @@ if (BUILD_TESTING)
800805
src/openvpn/xkey_provider.c
801806
)
802807

808+
target_sources(test_mbuf PRIVATE
809+
tests/unit_tests/openvpn/mock_get_random.c
810+
src/openvpn/buffer.c
811+
src/openvpn/mbuf.c
812+
)
813+
803814
target_sources(test_misc PRIVATE
804815
tests/unit_tests/openvpn/mock_get_random.c
805816
src/openvpn/options_util.c

src/openvpn/mbuf.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@
4242
struct mbuf_set *
4343
mbuf_init(unsigned int size)
4444
{
45+
ASSERT(size <= MBUF_SIZE_MAX);
46+
4547
struct mbuf_set *ret;
4648
ALLOC_OBJ_CLEAR(ret, struct mbuf_set);
4749
ret->capacity = adjust_power_of_2(size);
@@ -121,6 +123,7 @@ mbuf_extract_item(struct mbuf_set *ms, struct mbuf_item *item)
121123
bool ret = false;
122124
if (ms)
123125
{
126+
ASSERT(item);
124127
while (ms->len)
125128
{
126129
*item = ms->array[ms->head];

src/openvpn/mbuf.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@
3636
struct multi_instance;
3737

3838
#define MBUF_INDEX(head, offset, size) (((head) + (offset)) & ((size) - 1))
39+
/* limited by adjust_power_of_2 and array_mult_safe */
40+
#define MBUF_SIZE_MAX (ALLOC_SIZE_MAX / sizeof(struct mbuf_item))
3941

4042
struct mbuf_buffer
4143
{

src/openvpn/options.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
#include "options_util.h"
6363
#include "tun_afunix.h"
6464
#include "domain_helper.h"
65+
#include "mbuf.h"
6566

6667
#include <ctype.h>
6768

@@ -7552,7 +7553,7 @@ add_option(struct options *options, char *p[], bool is_inline, const char *file,
75527553
else if (streq(p[0], "bcast-buffers") && p[1] && !p[2])
75537554
{
75547555
VERIFY_PERMISSION(OPT_P_GENERAL);
7555-
atoi_constrained(p[1], &options->n_bcast_buf, p[0], 1, INT_MAX, msglevel);
7556+
atoi_constrained(p[1], &options->n_bcast_buf, p[0], 1, MBUF_SIZE_MAX, msglevel);
75567557
}
75577558
else if (streq(p[0], "tcp-queue-limit") && p[1] && !p[2])
75587559
{

tests/unit_tests/openvpn/Makefile.am

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,22 @@ AM_TESTSUITE_SUMMARY_HEADER = ' for $(PACKAGE_STRING) Unit-Tests'
66

77
AM_TESTS_ENVIRONMENT = export LSAN_OPTIONS=suppressions=$(srcdir)/input/leak_suppr.txt;
88

9-
test_binaries = argv_testdriver buffer_testdriver crypto_testdriver dhcp_testdriver packet_id_testdriver auth_token_testdriver \
10-
ncp_testdriver misc_testdriver options_parse_testdriver pkt_testdriver ssl_testdriver \
11-
user_pass_testdriver push_update_msg_testdriver provider_testdriver socket_testdriver
9+
test_binaries = argv_testdriver \
10+
auth_token_testdriver \
11+
buffer_testdriver \
12+
crypto_testdriver \
13+
dhcp_testdriver \
14+
ncp_testdriver \
15+
mbuf_testdriver \
16+
misc_testdriver \
17+
options_parse_testdriver \
18+
packet_id_testdriver \
19+
pkt_testdriver \
20+
provider_testdriver \
21+
push_update_msg_testdriver \
22+
socket_testdriver \
23+
ssl_testdriver \
24+
user_pass_testdriver
1225

1326
if HAVE_LD_WRAP_SUPPORT
1427
if !WIN32
@@ -327,6 +340,20 @@ ncp_testdriver_SOURCES = test_ncp.c \
327340
$(top_srcdir)/src/compat/compat-strsep.c \
328341
$(top_srcdir)/src/openvpn/ssl_util.c
329342

343+
mbuf_testdriver_CFLAGS = \
344+
-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
345+
-DSOURCEDIR=\"$(top_srcdir)\" @TEST_CFLAGS@
346+
347+
mbuf_testdriver_LDFLAGS = @TEST_LDFLAGS@
348+
349+
mbuf_testdriver_SOURCES = test_mbuf.c \
350+
mock_msg.c test_common.h \
351+
mock_get_random.c \
352+
$(top_srcdir)/src/openvpn/buffer.c \
353+
$(top_srcdir)/src/openvpn/win32-util.c \
354+
$(top_srcdir)/src/openvpn/platform.c \
355+
$(top_srcdir)/src/openvpn/mbuf.c
356+
330357
misc_testdriver_CFLAGS = \
331358
-I$(top_srcdir)/include -I$(top_srcdir)/src/compat -I$(top_srcdir)/src/openvpn \
332359
-DSOURCEDIR=\"$(top_srcdir)\" @TEST_CFLAGS@

tests/unit_tests/openvpn/test_buffer.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ test_snprintf(void **state)
386386

387387
/* Instead of trying to trick the compiler here, disable the warnings
388388
* for this unit test. We know that the results will be truncated
389-
* and we want to test that. Not we need the clang as clang-cl (msvc) does
389+
* and we want to test that. Note we need the clang as clang-cl (msvc) does
390390
* not define __GNUC__ like it does under UNIX(-like) platforms */
391391
#if defined(__GNUC__) || defined(__clang__)
392392
/* some clang version do not understand -Wformat-truncation, so ignore the
Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
/*
2+
* OpenVPN -- An application to securely tunnel IP networks
3+
* over a single UDP port, with support for SSL/TLS-based
4+
* session authentication and key exchange,
5+
* packet encryption, packet authentication, and
6+
* packet compression.
7+
*
8+
* Copyright (C) 2025 OpenVPN Inc. <[email protected]>
9+
*
10+
* This program is free software; you can redistribute it and/or modify
11+
* it under the terms of the GNU General Public License version 2
12+
* as published by the Free Software Foundation.
13+
*
14+
* This program is distributed in the hope that it will be useful,
15+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
16+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
17+
* GNU General Public License for more details.
18+
*
19+
* You should have received a copy of the GNU General Public License along
20+
* with this program; if not, see <https://www.gnu.org/licenses/>.
21+
*/
22+
23+
#ifdef HAVE_CONFIG_H
24+
#include "config.h"
25+
#endif
26+
27+
#include "syshead.h"
28+
29+
#include <setjmp.h>
30+
#include <cmocka.h>
31+
32+
#include "buffer.h"
33+
#include "multi.h"
34+
#include "mbuf.h"
35+
#include "test_common.h"
36+
37+
static void
38+
test_mbuf_init(void **state)
39+
{
40+
struct mbuf_set *ms = mbuf_init(256);
41+
assert_int_equal(ms->capacity, 256);
42+
assert_false(mbuf_defined(ms));
43+
assert_non_null(ms->array);
44+
mbuf_free(ms);
45+
46+
ms = mbuf_init(257);
47+
assert_int_equal(ms->capacity, 512);
48+
mbuf_free(ms);
49+
50+
#ifdef UNIT_TEST_ALLOW_BIG_ALLOC /* allocates up to 2GB of memory */
51+
ms = mbuf_init(MBUF_SIZE_MAX);
52+
assert_int_equal(ms->capacity, MBUF_SIZE_MAX);
53+
mbuf_free(ms);
54+
55+
/* NOTE: expect_assert_failure does not seem to work with MSVC */
56+
#ifndef _MSC_VER
57+
expect_assert_failure(mbuf_init(MBUF_SIZE_MAX + 1));
58+
#endif
59+
#endif
60+
}
61+
62+
static void
63+
test_mbuf_add_remove(void **state)
64+
{
65+
struct mbuf_set *ms = mbuf_init(4);
66+
assert_int_equal(ms->capacity, 4);
67+
assert_false(mbuf_defined(ms));
68+
assert_non_null(ms->array);
69+
70+
/* instances */
71+
struct multi_instance mi = { 0 };
72+
struct multi_instance mi2 = { 0 };
73+
/* buffers */
74+
struct buffer buf = alloc_buf(16);
75+
struct mbuf_buffer *mbuf_buf = mbuf_alloc_buf(&buf);
76+
assert_int_equal(mbuf_buf->refcount, 1);
77+
struct mbuf_buffer *mbuf_buf2 = mbuf_alloc_buf(&buf);
78+
assert_int_equal(mbuf_buf2->refcount, 1);
79+
free_buf(&buf);
80+
/* items */
81+
struct mbuf_item mb_item = { .buffer = mbuf_buf, .instance = &mi };
82+
struct mbuf_item mb_item2 = { .buffer = mbuf_buf2, .instance = &mi2 };
83+
84+
mbuf_add_item(ms, &mb_item);
85+
assert_int_equal(mbuf_buf->refcount, 2);
86+
assert_int_equal(mbuf_buf2->refcount, 1);
87+
assert_int_equal(mbuf_len(ms), 1);
88+
assert_int_equal(mbuf_maximum_queued(ms), 1);
89+
assert_int_equal(ms->head, 0);
90+
assert_ptr_equal(mbuf_peek(ms), &mi);
91+
92+
mbuf_add_item(ms, &mb_item2);
93+
assert_int_equal(mbuf_buf->refcount, 2);
94+
assert_int_equal(mbuf_buf2->refcount, 2);
95+
assert_int_equal(mbuf_len(ms), 2);
96+
assert_int_equal(mbuf_maximum_queued(ms), 2);
97+
assert_int_equal(ms->head, 0);
98+
assert_ptr_equal(mbuf_peek(ms), &mi);
99+
100+
mbuf_add_item(ms, &mb_item2);
101+
assert_int_equal(mbuf_buf->refcount, 2);
102+
assert_int_equal(mbuf_buf2->refcount, 3);
103+
assert_int_equal(mbuf_len(ms), 3);
104+
assert_int_equal(mbuf_maximum_queued(ms), 3);
105+
assert_int_equal(ms->head, 0);
106+
assert_ptr_equal(mbuf_peek(ms), &mi);
107+
108+
mbuf_add_item(ms, &mb_item2);
109+
mbuf_add_item(ms, &mb_item2); /* overflow, first item gets removed */
110+
assert_int_equal(mbuf_buf->refcount, 1);
111+
assert_int_equal(mbuf_buf2->refcount, 5);
112+
assert_int_equal(mbuf_len(ms), 4);
113+
assert_int_equal(mbuf_maximum_queued(ms), 4);
114+
assert_int_equal(ms->head, 1);
115+
assert_ptr_equal(mbuf_peek(ms), &mi2);
116+
117+
mbuf_add_item(ms, &mb_item);
118+
assert_int_equal(mbuf_buf->refcount, 2);
119+
assert_int_equal(mbuf_buf2->refcount, 4);
120+
assert_int_equal(mbuf_len(ms), 4);
121+
assert_int_equal(mbuf_maximum_queued(ms), 4);
122+
assert_int_equal(ms->head, 2);
123+
assert_ptr_equal(mbuf_peek(ms), &mi2);
124+
125+
struct mbuf_item out_item;
126+
assert_true(mbuf_extract_item(ms, &out_item));
127+
assert_ptr_equal(out_item.instance, mb_item2.instance);
128+
assert_int_equal(mbuf_buf->refcount, 2);
129+
assert_int_equal(mbuf_buf2->refcount, 4);
130+
assert_int_equal(mbuf_len(ms), 3);
131+
assert_int_equal(mbuf_maximum_queued(ms), 4);
132+
assert_int_equal(ms->head, 3);
133+
assert_ptr_equal(mbuf_peek(ms), &mi2);
134+
mbuf_free_buf(out_item.buffer);
135+
136+
mbuf_dereference_instance(ms, &mi2);
137+
assert_int_equal(mbuf_buf->refcount, 2);
138+
assert_int_equal(mbuf_buf2->refcount, 1);
139+
assert_int_equal(mbuf_len(ms), 3);
140+
assert_int_equal(mbuf_maximum_queued(ms), 4);
141+
assert_int_equal(ms->head, 3);
142+
assert_ptr_equal(mbuf_peek(ms), &mi);
143+
144+
mbuf_free(ms);
145+
assert_int_equal(mbuf_buf->refcount, 1);
146+
mbuf_free_buf(mbuf_buf);
147+
assert_int_equal(mbuf_buf2->refcount, 1);
148+
mbuf_free_buf(mbuf_buf2);
149+
}
150+
151+
int
152+
main(void)
153+
{
154+
const struct CMUnitTest tests[] = {
155+
cmocka_unit_test(test_mbuf_init),
156+
cmocka_unit_test(test_mbuf_add_remove),
157+
};
158+
159+
return cmocka_run_group_tests_name("mbuf", tests, NULL, NULL);
160+
}

0 commit comments

Comments
 (0)