I'm currently trying to implement the setenv function in C that sets the value of an environment variable, and I'm concerned about memory leaks in my code flagged by Valgrind due to strdup. I would greatly appreciate your insights in helping me identify any memory leak issues in the following code:
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
extern char **environ;
int _setenv(const char *name, const char *value)
{
char **new_environ, *current_env, *env_name;
int count = 0, modified = 0, i = 0;
/* Count the number of environment strings */
while (environ[i])
{
count++;
i++;
}
/* Create a copy of the environment variable */
new_environ = malloc(sizeof(char *) * (count + 2));
if (!new_environ)
{
perror("Memory Allocation Error");
return (-1);
}
for (i = 0; i < count; i++)
{
current_env = strdup(environ[i]);
env_name = strtok(current_env, "=");
if (env_name != NULL && strcmp(env_name, name) == 0)
{
new_environ[count] = malloc(strlen(name) + strlen(value) + 2);
if (new_environ[count])
{
sprintf(new_environ[count], "%s=%s", name, value);
new_environ[count + 1] = NULL; /* Terminate the new array */
modified = 0;
free(current_env);
break;
}
}
else
{
new_environ[i] = current_env;
modified = 1;
free(current_env);
}
}
if (!modified)
{
new_environ[count] = malloc(strlen(name) + strlen(value) + 2);
if (new_environ[count])
{
sprintf(new_environ[count], "%s=%s", name, value);
new_environ[count + 1] = NULL; /* Terminate the new array */
}
}
else
{
new_environ[count] = NULL; /* Terminate the new array */
}
environ = new_environ;
return (0);
}
A better design would be:
Also I don't understand this part at all:
The
new_environ[i] = current_envis a soft copy only assigning a pointer to the pointer array. The actual data remains where it is. So if youfree(current_env)thennew_environ[i]ends up pointing at garbage since you just freed the data it pointed at.