Improved freeing process of the env_values array.

Extracted the process into a function and called it where it was missing.

Author: matejak

src/SCE/sce_engine.c

@@ -346,6 +346,15 @@ static void _pipe_try_read_into_string(int fd, struct oscap_string *string, bool
 	}
 }
 
+
+static void free_env_values(char **env_values, size_t index_of_first_env_value_not_compiled_in, size_t real_env_values_count) {
+	for (i = index_of_first_env_value_not_compiled_in; i < real_env_values_count; i++) {
+		free(env_values[i]);
+	}
+	free(env_values);
+}
+
+
 xccdf_test_result_type_t sce_engine_eval_rule(struct xccdf_policy *policy, const char *rule_id, const char *id, const char *href,
 		struct xccdf_value_binding_iterator *value_binding_it,
 		struct xccdf_check_import_iterator *check_import_it,
@@ -390,6 +399,7 @@ xccdf_test_result_type_t sce_engine_eval_rule(struct xccdf_policy *policy, const
 	// bound values in KEY=VALUE form, ready to be passed as environment variables
 	char ** env_values = malloc(10 * sizeof(char * ));
 	size_t env_value_count = 10;
+	const size_t index_of_first_env_value_not_compiled_in = 10;
 
 	env_values[0] = "PATH=/bin:/sbin:/usr/bin:/usr/sbin";
 
@@ -488,12 +498,7 @@ xccdf_test_result_type_t sce_engine_eval_rule(struct xccdf_policy *policy, const
 	if (pipe(stdout_pipefd) == -1 || pipe(stderr_pipefd) == -1)
 	{
 		perror("pipe");
-		// the first 9 values (0 to 8) are compiled in
-		for (size_t i = 9; i < env_value_count; ++i)
-		{
-			free(env_values[i]);
-		}
-		free(env_values);
+		free_env_values(env_values, index_of_first_env_value_not_compiled_in, env_value_count);
 		return XCCDF_RESULT_ERROR;
 	}
 
@@ -507,9 +512,11 @@ xccdf_test_result_type_t sce_engine_eval_rule(struct xccdf_policy *policy, const
 
 		if (fork_result == 0)
 		{
-		    // we won't read from the pipes, so close the reading fd
-		    close(stdout_pipefd[0]);
-		    close(stderr_pipefd[0]);
+			free_env_values(env_values, index_of_first_env_value_not_compiled_in, env_value_count);
+
+			// we won't read from the pipes, so close the reading fd
+			close(stdout_pipefd[0]);
+			close(stderr_pipefd[0]);
 
 			// forward stdout and stderr to our custom opened pipes
 			dup2(stdout_pipefd[1], fileno(stdout));
@@ -550,13 +557,15 @@ xccdf_test_result_type_t sce_engine_eval_rule(struct xccdf_policy *policy, const
 			if (flag_stdout == -1) {
 				oscap_seterr(OSCAP_EFAMILY_SCE, "Failed to obtain status of stdout pipe: %s",
 						strerror(errno));
+				free_env_values(env_values, index_of_first_env_value_not_compiled_in, env_value_count);
 				return XCCDF_RESULT_ERROR;
 			}
 			int retval = fcntl(stdout_pipefd[0], F_SETFL, flag_stdout | O_NONBLOCK);
 			if (retval == -1) {
 				oscap_seterr(OSCAP_EFAMILY_SCE,
 						"Failed to set nonblocking flag on stdout pipe: %s",
 						strerror(errno));
+				free_env_values(env_values, index_of_first_env_value_not_compiled_in, env_value_count);
 				return XCCDF_RESULT_ERROR;
 			}
 
@@ -565,13 +574,15 @@ xccdf_test_result_type_t sce_engine_eval_rule(struct xccdf_policy *policy, const
 				oscap_seterr(OSCAP_EFAMILY_SCE,
 						"Failed to obtain status of stderr pipe: %s",
 						strerror(errno));
+				free_env_values(env_values, index_of_first_env_value_not_compiled_in, env_value_count);
 				return XCCDF_RESULT_ERROR;
 			}
 			retval = fcntl(stderr_pipefd[0], F_SETFL, flag_stderr | O_NONBLOCK);
 			if (retval == -1) {
 				oscap_seterr(OSCAP_EFAMILY_SCE,
 						"Failed to set nonblocking flag on stderr pipe: %s",
 						strerror(errno));
+				free_env_values(env_values, index_of_first_env_value_not_compiled_in, env_value_count);
 				return XCCDF_RESULT_ERROR;
 			}
 
@@ -630,12 +641,7 @@ xccdf_test_result_type_t sce_engine_eval_rule(struct xccdf_policy *policy, const
 				sce_session_add_check_result(session, check_result);
 			}
 
-			// the first 10 values (0 to 9) are compiled in
-			for (size_t i = 10; i < env_value_count; ++i)
-			{
-				free(env_values[i]);
-			}
-			free(env_values);
+			free_env_values(env_values, index_of_first_env_value_not_compiled_in, env_value_count);
 
 			// lets interpret the check imports passed to us
 			xccdf_check_import_iterator_reset(check_import_it);
@@ -663,12 +669,7 @@ xccdf_test_result_type_t sce_engine_eval_rule(struct xccdf_policy *policy, const
 	}
 	else
 	{
-		// the first 9 values (0 to 8) are compiled in
-		for (size_t i = 9; i < env_value_count; ++i)
-		{
-			free(env_values[i]);
-		}
-		free(env_values);
+		free_env_values(env_values, index_of_first_env_value_not_compiled_in, env_value_count);
 
 		close(stdout_pipefd[0]);
 		close(stdout_pipefd[1]);