@@ -685,6 +685,7 @@
target_sources(test_buffer PRIVATE
tests/unit_tests/openvpn/mock_get_random.c
+ src/openvpn/env_set.c
)
target_sources(test_crypto PRIVATE
@@ -112,6 +112,10 @@
}
if (do_free)
{
+ if (current->free_function)
+ {
+ current->free_function(current);
+ }
secure_memzero(current->string, strlen(current->string));
free(current->string);
free(current);
@@ -124,15 +128,37 @@
}
static void
-add_env_item(char *str, const bool do_alloc, struct env_item **list, struct gc_arena *gc)
+env_gc_freefunction_wrapper(void *arg)
+{
+ struct env_item *ei = arg;
+ ei->free_function(ei);
+ free(ei->string);
+ free(ei);
+}
+
+static void
+add_env_item(char *str, void (*free_function)(struct env_item *),
+ struct env_item **list, struct gc_arena *gc)
{
struct env_item *item;
ASSERT(str);
ASSERT(list);
- ALLOC_OBJ_GC(item, struct env_item, gc);
- item->string = do_alloc ? string_alloc(str, gc) : str;
+ if (gc && free_function)
+ {
+ /* alloc items manually without gc to allow the gc_wrap_free
+ * function to free them all in the right order */
+ ALLOC_OBJ(item, struct env_item);
+ item->string = string_alloc(str, NULL);
+ gc_addspecial(item, env_gc_freefunction_wrapper, gc);
+ }
+ else
+ {
+ ALLOC_OBJ_GC(item, struct env_item, gc);
+ item->string = string_alloc(str, gc);
+ }
+ item->free_function = free_function;
item->next = *list;
*list = item;
}
@@ -146,10 +172,11 @@
}
static void
-env_set_add_nolock(struct env_set *es, const char *str)
+env_set_add_nolock(struct env_set *es, const char *str,
+ void (*free_function)(struct env_item *))
{
remove_env_item(str, es->gc == NULL, &es->list);
- add_env_item((char *)str, true, &es->list, es->gc);
+ add_env_item((char *)str, free_function, &es->list, es->gc);
}
struct env_set *
@@ -171,6 +198,10 @@
while (e)
{
struct env_item *next = e->next;
+ if (e->free_function)
+ {
+ e->free_function(e);
+ }
free(e->string);
free(e);
e = next;
@@ -194,7 +225,16 @@
{
ASSERT(es);
ASSERT(str);
- env_set_add_nolock(es, str);
+ env_set_add_nolock(es, str, NULL);
+}
+
+void
+env_set_add_specialfree(struct env_set *es, const char *str,
+ void (*free_function)(struct env_item *))
+{
+ ASSERT(es);
+ ASSERT(str);
+ env_set_add_nolock(es, str, free_function);
}
const char *
@@ -246,7 +286,7 @@
e = src->list;
while (e)
{
- env_set_add_nolock(es, e->string);
+ env_set_add_nolock(es, e->string, e->free_function);
e = e->next;
}
}
@@ -36,6 +36,7 @@
struct env_item {
char *string;
+ void (*free_function)(struct env_item *);
struct env_item *next;
};
@@ -87,6 +88,11 @@
void env_set_add(struct env_set *es, const char *str);
+void
+env_set_add_specialfree(struct env_set *es, const char *str,
+ void (*free_function)(struct env_item *));
+
+
const char *env_set_get(const struct env_set *es, const char *name);
void env_set_print(int msglevel, const struct env_set *es);
@@ -52,6 +52,7 @@
buffer_testdriver_LDFLAGS = @TEST_LDFLAGS@ -L$(top_srcdir)/src/openvpn -Wl,--wrap=parse_line
buffer_testdriver_SOURCES = test_buffer.c mock_msg.c mock_msg.h \
mock_get_random.c \
+ $(top_srcdir)/src/openvpn/env_set.c \
$(top_srcdir)/src/openvpn/win32-util.c \
$(top_srcdir)/src/openvpn/platform.c
@@ -26,13 +26,22 @@
#endif
#include "syshead.h"
+#include <stdbool.h>
#include <setjmp.h>
#include <cmocka.h>
#include "buffer.h"
+#include "env_set.h"
#include "buffer.c"
+/* mock this function as it is required by env.c */
+int
+script_security(void)
+{
+ return 0;
+}
+
static void
test_buffer_strprefix(void **state)
{
@@ -353,6 +362,47 @@
assert_string_equal(buf, "There is a .'nice.' \"1234\" [.] year old .tree!");
}
+static bool unit_test_free_called = false;
+
+static void
+unit_test_special_free(struct env_item *ei)
+{
+ assert_string_equal(ei->string, "unit=test");
+ unit_test_free_called = true;
+}
+
+static void
+test_env_special_free(void **state)
+{
+ /* Test called via env_set_destroy */
+ struct env_set *es = env_set_create(NULL);
+ env_set_add_specialfree(es, "unit=test", &unit_test_special_free);
+ env_set_destroy(es);
+ assert_true(unit_test_free_called);
+ unit_test_free_called = false;
+
+ /* Test called via env_remove */
+ es = env_set_create(NULL);
+ env_set_add_specialfree(es, "unit=test", &unit_test_special_free);
+ env_set_del(es, "unit");
+ assert_true(unit_test_free_called);
+ unit_test_free_called = false;
+ env_set_destroy(es);
+
+ /* Test for env with GC */
+ struct gc_arena gc = gc_new();
+ es = env_set_create(&gc);
+ env_set_add_specialfree(es, "unit=test", &unit_test_special_free);
+ env_set_destroy(es);
+
+ /* The free should be done as part of gc free and not as part of
+ * the env set destroy */
+ assert_false(unit_test_free_called);
+
+ gc_free(&gc);
+ assert_true(unit_test_free_called);
+}
+
int
main(void)
{
@@ -385,6 +435,7 @@
cmocka_unit_test(test_buffer_free_gc_two),
cmocka_unit_test(test_buffer_gc_realloc),
cmocka_unit_test(test_character_class),
+ cmocka_unit_test(test_env_special_free)
};
return cmocka_run_group_tests_name("buffer", tests, NULL, NULL);
Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/486?usp=email to review the following change. Change subject: Allow having an extra function that is called when an env is freed ...................................................................... Allow having an extra function that is called when an env is freed Change-Id: I6899c1af65fbd670d434cdb17f81c82b900bfbd4 Signed-off-by: Arne Schwabe <arne@rfc2549.org> --- M CMakeLists.txt M src/openvpn/env_set.c M src/openvpn/env_set.h M tests/unit_tests/openvpn/Makefile.am M tests/unit_tests/openvpn/test_buffer.c 5 files changed, 106 insertions(+), 7 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/86/486/1