Merge pull request #86 from jhunkeler/memfixes

ASAN: Fix leaks

13 files changed, 58 insertions, 10 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d21a553..76b7186 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -7,6 +7,13 @@ set(win_cflags /Wall)
 set(CMAKE_C_STANDARD 99)
 find_package(LibXml2)
 find_package(CURL)
+
+option(ASAN OFF)
+if (ASAN)
+	add_compile_options(-fsanitize=address)
+	add_link_options(-fsanitize=address)
+endif()
+
 link_libraries(CURL::libcurl)
 link_libraries(LibXml2::LibXml2)
 include_directories(${LIBXML2_INCLUDE_DIR})
diff --git a/src/lib/core/junitxml.c b/src/lib/core/junitxml.c
index f747224..628f75f 100644
--- a/src/lib/core/junitxml.c
+++ b/src/lib/core/junitxml.c
@@ -8,6 +8,9 @@ static void testcase_result_state_free(struct JUNIT_Testcase **testcase) {
     if (tc->tc_result_state_type == JUNIT_RESULT_STATE_FAILURE) {
         guard_free(tc->result_state.failure->message);
         guard_free(tc->result_state.failure);
+    } else if (tc->tc_result_state_type == JUNIT_RESULT_STATE_ERROR) {
+        guard_free(tc->result_state.error->message);
+        guard_free(tc->result_state.error);
     } else if (tc->tc_result_state_type == JUNIT_RESULT_STATE_SKIPPED) {
         guard_free(tc->result_state.skipped->message);
         guard_free(tc->result_state.skipped);
@@ -31,6 +34,7 @@ void junitxml_testsuite_free(struct JUNIT_Testsuite **testsuite) {
     for (size_t i = 0; i < suite->_tc_alloc; i++) {
         testcase_free(&suite->testcase[i]);
     }
+    guard_free(suite->testcase);
     guard_free(suite);
 }
 
diff --git a/src/lib/core/strlist.c b/src/lib/core/strlist.c
index ec7b3f4..0436f54 100644
--- a/src/lib/core/strlist.c
+++ b/src/lib/core/strlist.c
@@ -218,6 +218,7 @@ void strlist_remove(struct StrList *pStrList, size_t index) {
         return;
     }
     if (pStrList->data[index] != NULL) {
+        guard_free(pStrList->data[index]);
         for (size_t i = index; i < count; i++) {
             pStrList->data[i] = pStrList->data[i + 1];
         }
diff --git a/src/lib/core/wheel.c b/src/lib/core/wheel.c
index d5d5ff0..32091cd 100644
--- a/src/lib/core/wheel.c
+++ b/src/lib/core/wheel.c
@@ -121,5 +121,6 @@ void wheel_free(struct Wheel **wheel) {
     guard_free(w->python_tag);
     guard_free(w->abi_tag);
     guard_free(w->python_tag);
+    guard_free(w->platform_tag);
     guard_free(w);
 }
diff --git a/src/lib/delivery/delivery.c b/src/lib/delivery/delivery.c
index aa3e51a..41be64c 100644
--- a/src/lib/delivery/delivery.c
+++ b/src/lib/delivery/delivery.c
@@ -40,6 +40,7 @@ void delivery_free(struct Delivery *ctx) {
     guard_free(ctx->info.build_name);
     guard_free(ctx->info.build_number);
     guard_free(ctx->info.release_name);
+    guard_free(ctx->info.time_info);
     guard_free(ctx->conda.installer_baseurl);
     guard_free(ctx->conda.installer_name);
     guard_free(ctx->conda.installer_version);
@@ -50,8 +51,10 @@ void delivery_free(struct Delivery *ctx) {
     guard_free(ctx->conda.tool_build_version);
     guard_strlist_free(&ctx->conda.conda_packages);
     guard_strlist_free(&ctx->conda.conda_packages_defer);
+    guard_strlist_free(&ctx->conda.conda_packages_purge);
     guard_strlist_free(&ctx->conda.pip_packages);
     guard_strlist_free(&ctx->conda.pip_packages_defer);
+    guard_strlist_free(&ctx->conda.pip_packages_purge);
     guard_strlist_free(&ctx->conda.wheels_packages);
 
     for (size_t i = 0; i < sizeof(ctx->tests) / sizeof(ctx->tests[0]); i++) {
@@ -62,6 +65,7 @@ void delivery_free(struct Delivery *ctx) {
         guard_free(ctx->tests[i].repository_info_tag);
         guard_strlist_free(&ctx->tests[i].repository_remove_tags);
         guard_free(ctx->tests[i].script);
+        guard_free(ctx->tests[i].script_setup);
         guard_free(ctx->tests[i].build_recipe);
         // test-specific runtime variables
         guard_runtime_free(ctx->tests[i].runtime.environ);
diff --git a/src/lib/delivery/delivery_init.c b/src/lib/delivery/delivery_init.c
index 2fced03..fe60075 100644
--- a/src/lib/delivery/delivery_init.c
+++ b/src/lib/delivery/delivery_init.c
@@ -296,10 +296,12 @@ int bootstrap_build_info(struct Delivery *ctx) {
     ctx->info.build_name = strdup(local.info.build_name);
     ctx->info.build_number = strdup(local.info.build_number);
     ctx->info.release_name = strdup(local.info.release_name);
-    ctx->info.time_info = malloc(sizeof(*ctx->info.time_info));
     if (!ctx->info.time_info) {
-        SYSERROR("Unable to allocate %zu bytes for tm struct: %s", sizeof(*local.info.time_info), strerror(errno));
-        return -1;
+        ctx->info.time_info = malloc(sizeof(*ctx->info.time_info));
+        if (!ctx->info.time_info) {
+            SYSERROR("Unable to allocate %zu bytes for tm struct: %s", sizeof(*local.info.time_info), strerror(errno));
+            return -1;
+        }
     }
     memcpy(ctx->info.time_info, local.info.time_info, sizeof(*local.info.time_info));
     ctx->info.time_now = local.info.time_now;
diff --git a/src/lib/delivery/delivery_populate.c b/src/lib/delivery/delivery_populate.c
index e9aea89..63df9fd 100644
--- a/src/lib/delivery/delivery_populate.c
+++ b/src/lib/delivery/delivery_populate.c
@@ -30,11 +30,21 @@ int populate_info(struct Delivery *ctx) {
     if (!ctx->info.time_str_epoch) {
         // Record timestamp used for release
         time(&ctx->info.time_now);
-        ctx->info.time_info = localtime(&ctx->info.time_now);
+        if (!ctx->info.time_info) {
+            ctx->info.time_info = malloc(sizeof(*ctx->info.time_info));
+            if (!ctx->info.time_info) {
+                msg(STASIS_MSG_ERROR, "%s: Unable to allocate memory for time_info\n", strerror(errno));
+                return -1;
+            }
+            if (!localtime_r(&ctx->info.time_now, ctx->info.time_info)) {
+                msg(STASIS_MSG_ERROR, "%s: localtime_r failed\n", strerror(errno));
+                return -1;
+            }
+        }
 
         ctx->info.time_str_epoch = calloc(STASIS_TIME_STR_MAX, sizeof(*ctx->info.time_str_epoch));
         if (!ctx->info.time_str_epoch) {
-            msg(STASIS_MSG_ERROR, "Unable to allocate memory for Unix epoch string\n");
+            msg(STASIS_MSG_ERROR, "%s: Unable to allocate memory for Unix epoch string\n", strerror(errno));
             return -1;
         }
         snprintf(ctx->info.time_str_epoch, STASIS_TIME_STR_MAX - 1, "%li", ctx->info.time_now);
diff --git a/tests/test_artifactory.c b/tests/test_artifactory.c
index 2c732fa..4f41543 100644
--- a/tests/test_artifactory.c
+++ b/tests/test_artifactory.c
@@ -73,7 +73,7 @@ int main(int argc, char *argv[]) {
     memset(&gnoauth, 0, sizeof(gnoauth));
     memset(&ctx, 0, sizeof(ctx));
 
-    const char *basedir = realpath(".", NULL);
+    char *basedir = realpath(".", NULL);
     const char *ws = "workspace";
     mkdir(ws, 0755);
     if (pushd(ws)) {
@@ -107,6 +107,7 @@ int main(int argc, char *argv[]) {
     ctx._stasis_ini_fp.cfg = ini_open(cfg_path);
     if (!ctx._stasis_ini_fp.cfg) {
         SYSERROR("%s: configuration is invalid", cfg_path);
+        guard_free(basedir);
         return STASIS_TEST_SUITE_SKIP;
     }
     delivery_init_platform(&ctx);
@@ -118,6 +119,7 @@ int main(int argc, char *argv[]) {
         SYSERROR("%s", "Not configured to test Artifactory. Skipping.");
         return STASIS_TEST_SUITE_SKIP;
     }
+    guard_free(basedir);
 
     STASIS_TEST_FUNC *tests[] = {
         test_jfrog_cli_rt_ping,
diff --git a/tests/test_copy.c b/tests/test_copy.c
index 14310e4..b37b1d3 100644
--- a/tests/test_copy.c
+++ b/tests/test_copy.c
@@ -18,7 +18,7 @@ void test_copy() {
     for (size_t i = 0; i < sizeof(tc) / sizeof(*tc); i++) {
         struct stat st_a, st_b;
         struct testcase *test = &tc[i];
-        char *mock_data = malloc(test->expect_size * sizeof(*mock_data));
+        char *mock_data = malloc(test->expect_size + 1 * sizeof(*mock_data));
         memset(mock_data, 'A', test->expect_size);
         mock_data[test->expect_size] = '\0';
         stasis_testing_write_ascii(test->in_file, mock_data);
diff --git a/tests/test_environment.c b/tests/test_environment.c
index 3418800..134d061 100644
--- a/tests/test_environment.c
+++ b/tests/test_environment.c
@@ -31,7 +31,7 @@ void test_runtime() {
     STASIS_ASSERT((idx = runtime_contains(env, "CUSTOM_KEY")) >= 0, "CUSTOM_KEY should exist in object");
     STASIS_ASSERT(strcmp(strlist_item(env, idx), "CUSTOM_KEY=Very custom") == 0, "Incorrect index returned by runtime_contains");
 
-    const char *custom_value = runtime_get(env, "CUSTOM_KEY");
+    char *custom_value = runtime_get(env, "CUSTOM_KEY");
     STASIS_ASSERT_FATAL(custom_value != NULL, "CUSTOM_KEY should not be NULL");
     STASIS_ASSERT(strcmp(custom_value, "Very custom") == 0, "CUSTOM_KEY has incorrect data");
     STASIS_ASSERT(strstr_array(environ, "CUSTOM_KEY") == NULL, "CUSTOM_KEY should not exist in the global environment");
@@ -44,6 +44,7 @@ void test_runtime() {
     STASIS_ASSERT(setenv("CUSTOM_KEY", "modified", 1) == 0, "modifying global CUSTOM_KEY failed");
     global_custom_value = getenv("CUSTOM_KEY");
     STASIS_ASSERT(strcmp(global_custom_value, custom_value) != 0, "local and global CUSTOM_KEY variable are supposed to be different");
+    guard_free(custom_value);
 
     runtime_replace(&env, environ);
     runtime_apply(env);
@@ -53,13 +54,19 @@ void test_runtime() {
     STASIS_ASSERT_FATAL(custom_value != NULL, "CUSTOM_KEY must exist in local environment after calling runtime_replace()");
     STASIS_ASSERT_FATAL(global_custom_value != NULL, "CUSTOM_KEY must exist in global environment after calling runtime_replace()");
     STASIS_ASSERT(strcmp(global_custom_value, custom_value) == 0, "local and global CUSTOM_KEY variable are supposed to be identical");
+    guard_free(custom_value);
 
     char output_truth[BUFSIZ] = {0};
-    sprintf(output_truth, "Your PATH is '%s'.", runtime_get(env, "PATH"));
-    const char *output_expanded = runtime_expand_var(env, "Your PATH is '${PATH}'.");
+    char *your_path = runtime_get(env, "PATH");
+    sprintf(output_truth, "Your PATH is '%s'.", your_path);
+    guard_free(your_path);
+
+    char *output_expanded = runtime_expand_var(env, "Your PATH is '${PATH}'.");
     STASIS_ASSERT(output_expanded != NULL, "expansion of PATH should not fail");
     STASIS_ASSERT(strcmp(output_expanded, output_truth) == 0, "the expansion, and the expected result should be identical");
+    guard_free(output_expanded);
 
+    guard_runtime_free(env);
     // TODO: runtime_export()
     // requires dumping stdout to a file and comparing it with the current environment array
 }
diff --git a/tests/test_recipe.c b/tests/test_recipe.c
index 531b3b4..fc7cc78 100644
--- a/tests/test_recipe.c
+++ b/tests/test_recipe.c
@@ -85,6 +85,7 @@ void test_recipe_clone() {
             // Verify the result path does not exist
             STASIS_ASSERT(result_path && access(result_path, F_OK) != 0, "result path should not exist");
         }
+        guard_free(result_path);
     }
 
 }
diff --git a/tests/test_system.c b/tests/test_system.c
index 0f6bfb7..2271e13 100644
--- a/tests/test_system.c
+++ b/tests/test_system.c
@@ -18,6 +18,7 @@ void test_shell_output_null_args() {
     result = shell_output(NULL, &status);
     STASIS_ASSERT(strcmp(result, "") == 0, "no output expected");
     STASIS_ASSERT(status != 0, "expected a non-zero exit code due to null argument string");
+    guard_free(result);
 }
 
 void test_shell_output_non_zero_exit() {
@@ -26,6 +27,7 @@ void test_shell_output_non_zero_exit() {
     result = shell_output("HELLO1234 WORLD", &status);
     STASIS_ASSERT(strcmp(result, "") == 0, "no output expected");
     STASIS_ASSERT(status != 0, "expected a non-zero exit code due to intentional error");
+    guard_free(result);
 }
 
 void test_shell_output() {
@@ -34,6 +36,7 @@ void test_shell_output() {
     result = shell_output("echo HELLO WORLD", &status);
     STASIS_ASSERT(strcmp(result, "HELLO WORLD\n") == 0, "output message was incorrect");
     STASIS_ASSERT(status == 0, "expected zero exit code");
+    guard_free(result);
 }
 
 void test_shell_safe() {
diff --git a/tests/test_utils.c b/tests/test_utils.c
index 9090b05..8f0a667 100644
--- a/tests/test_utils.c
+++ b/tests/test_utils.c
@@ -46,6 +46,7 @@ void test_redact_sensitive() {
         char output[100] = {0};
         redact_sensitive(to_redact, sizeof(to_redact) / sizeof(*to_redact), input, output, sizeof(output) - 1);
         STASIS_ASSERT(strcmp(output, expected[i]) == 0, "incorrect redaction");
+        guard_free(input);
     }
 }
 
@@ -118,6 +119,7 @@ void test_path_store() {
     STASIS_ASSERT(dest != NULL, "dest should not be NULL");
     STASIS_ASSERT(dest && (access(dest, F_OK) == 0), "destination path was not created");
     rmtree(dest);
+    guard_free(dest);
 }
 
 void test_isempty_dir() {
@@ -226,6 +228,7 @@ void test_git_clone_and_describe() {
     char *taginfo_bad = git_describe("abc1234_not_here_or_there");
     STASIS_ASSERT(taginfo_bad == NULL, "a repository that shouldn't exist... exists and has a tag.");
     chdir(cwd);
+    guard_free(cwd);
 }
 
 void test_touch() {
@@ -414,9 +417,11 @@ void test_pushd_popd() {
     // we should be inside the test directory, not the starting directory
     STASIS_ASSERT(strcmp(cwd_workspace, cwd) != 0, "");
     STASIS_ASSERT(popd() == 0, "return from directory failed");
+    guard_free(cwd);
 
     char *cwd_after_popd = getcwd(NULL, PATH_MAX);
     STASIS_ASSERT(strcmp(cwd_workspace, cwd_after_popd) == 0, "should be the path where we started");
+    guard_free(cwd_after_popd);
 }
 
 void test_pushd_popd_suggested_workflow() {
@@ -436,6 +441,7 @@ void test_pushd_popd_suggested_workflow() {
         // cwd should be our starting directory
         char *cwd = getcwd(NULL, PATH_MAX);
         STASIS_ASSERT(strcmp(cwd_workspace, cwd) == 0, NULL);
+        guard_free(cwd);
     } else {
         STASIS_ASSERT(false, "mkdir function failed");
     }