Realloc problem, environment variables Linux C

Hey Community,
I have to write my own implementation of setenv, getenv and unsetenv on a linux system for school.

Now if i'm adding an environment variable i have to copy all variables from stack to heap.
That works fine. But if i'm adding more variables to the list realloc throws some strange error.
I'm pretty shure the error occurs when realloc has to actually move the memory block. In most cases the error doesnt occure when calling realloc the first time


Here's my whole implementation. If you dont want to read all the code, just look at env.c on the bottom in the function enter_env (CASE3)


Env.h

Code:

---

#ifndef ENV_H
#define ENV_H

#include <stdio.h>
#include <string.h>
#include <stdlib.h>

void init_env();
void print_menu();
void print_env();
void print_specific_env( char *name);
void delete_env(char*name);
void enter_env(char* name, char* value);

#endif

---

Env.c

Code:

---

#include "env.h"
extern char ** environ; //Environment pointer
char ** environ_intern; //Internal envitonment pointer
int is_on_stack = 1;    //Environment on stack or heap?

static void check_alloc(void*ptr)///checks if memory has been allocated correctly
{
    if (ptr) {return;}                          //check for bad alloc
    perror("Not enough memory.");              //throw error
    exit(EXIT_FAILURE);
}

static char* make_env(char*name,char*value)///creates a new environment variable and returns its pointer
{
    char *new_env = (char*) malloc ((strlen(name)+strlen(value) +1)*sizeof(char));//create variable
    check_alloc(new_env);    //check if correctly allocated
    strcpy(new_env,name);
    strcat(new_env, "=");
    strcat(new_env,value);
    return new_env;
}

static int get_environment_index(char** env, char* name)///returns index of environment variable ,returns -1 if not found
{
    char temp[256];
    strcpy(temp, name);
    strcat(temp, "=");
    int i;
    for (i=0; env[i] != NULL;i++)
    {
        if (strstr(env[i],temp)){return i; }//String found
    }
    return -1; //not found
}

static int get_environment_length(char** env)///returns the length of the environment list
{
    int i =0;
    while (env[i]!=NULL){i++;}
    return i;
}

void print_menu()///Prints the Environment Menu
{
    printf("-----------------------\n");
    printf ("Environment-List\n");
    printf("-----------------------\n");
    printf("\n\n0: End\n");
    printf("1: Show environment list\n");
    printf("2: Read single variable\n");
    printf("3: Add new variable\n");
    printf("4: Delete Variable \n");
    printf("\n\nYour choice:");
}

void init_env()///Set pointer to internal environment variable
{
    environ_intern = environ;
}

void print_env()///Print all environment variables
{
    int i;
    for (i=0; environ_intern[i] != NULL;i++){printf("%d,%s\n", i,environ_intern[i]);}
}

void print_specific_env(char *name)///Print a specific environment variable
{
    int i = get_environment_index( environ_intern, name); //get index
    if ( i!=-1){printf("%s\n", environ_intern[i]);}    //print if found
}

void delete_env(char*name)///Delete an environment variable -- we have to shift all variables above the index one item up
{
    int j = get_environment_index(environ_intern, name),i =j; //i an j get position of index
    if (i == -1){return;}    //environment variable doesnt exist
    do{environ_intern[j] = environ_intern[j+1];j++;}while( environ[j+1] != NULL);//shift up
  //  if(!is_on_stack){free(environ[j+1]);environ[j+1] = NULL;} //free memory if necessary
}

void enter_env(char* name, char* value)/// Adds an environment variable
{
    int i = get_environment_index(environ_intern, name);    //check if exists

    if (i!=-1) // CASE 1 Already existing -- Simply create a new variable and replace
    {
        environ_intern[i] = make_env(name,value);//create the new variable and overwrite the old
        return;
    }
    int env_size = get_environment_length(environ_intern);//Get number of environment variables
    if (is_on_stack) // CASE 2 First time Copy everything to Heap
    {
        environ_intern = (char**) malloc ((env_size+1)*sizeof(char*)); //allocate new environment list
        check_alloc(environ_intern);                                    //check if memory has been allocated
        for (i=0; i < env_size;i++)//copy environment variables from stack to heap (DEEP COPY)
        {
            char *new_env = (char*) malloc (sizeof(environ[i])*sizeof(char)+1);
            strcpy(new_env,environ[i]);
            environ_intern[i] = new_env;
        }
        environ_intern[i] = make_env(name,value);                      //create the new variable
        environ_intern[i+1]= NULL;                                      //end with zero
        environ = environ_intern;                                      //refering environment variable to new adress
        is_on_stack = 0;                                                //now everything is on heap
        return;
    }
    // CASE 3 Already on HEAP, reallocate space
    environ_intern = realloc (environ_intern,(env_size+1)*sizeof(char*));          //get length of environment
    check_alloc(environ_intern); //check if memory has been allocated correctly
    environ_intern[env_size] = make_env(name,value);                          //create the new variable
    environ_intern[env_size+1]= NULL; //terminate end with zero
    environ = environ_intern ;//In case realloc moved memory around the environ pointer would become dangling
}

---

Main.c

Code:

---

#include <stdio.h>
#include "env.h"
#define BUFSIZE 256
#define FLUSH_INP while ( getchar() != '\n'); ///Emptys the Input Buffer

int main (int argc, char*argv[])
{
    init_env();
    while (1)
    {
        char param1[BUFSIZE],param2[BUFSIZE];
        print_menu();

        switch (getchar())
        {
        case '0': ///EXIT
            exit(EXIT_SUCCESS);
        case '1': ///PRINT ALL
            print_env();
            break;
        case '2': ///PRINZ SPEVIFIC
            printf("Name of variable:");
            scanf("%s", param1);
            print_specific_env(param1);
            break;
        case '3':///SET_ENV
            printf("enter name:");
            scanf("%s", param1);
            FLUSH_INP;
            printf("enter value:");
            scanf("%s", param2);
            enter_env(param1,param2);
            break;
        case '4':///DELETE ENV
            printf("Name of variable:");
            scanf("%s", param1);
            delete_env(param1);
            break;
        default :
            printf("unknown command\n");
        }
        FLUSH_INP
    }
}

---

Error Message

Code:

---

** glibc detected *** ./directory: realloc(): invalid next size: 0x0000000001aca010 ***
======= Backtrace: =========
/lib/libc.so.6(+0x775b6)[0x7f1b05bbc5b6]
/lib/libc.so.6(+0x7dd96)[0x7f1b05bc2d96]
/lib/libc.so.6(realloc+0xf0)[0x7f1b05bc30b0]
./directory[0x400ed4]
./directory[0x40107e]
/lib/libc.so.6(__libc_start_main+0xfd)[0x7f1b05b63c4d]
./directory[0x400909]
======= Memory map: ========
00400000-00402000 r-xp 00000000 08:01 524295                            /home/gunther/Desktop/bes/directory
00601000-00602000 r--p 00001000 08:01 524295                            /home/gunther/Desktop/bes/directory
00602000-00603000 rw-p 00002000 08:01 524295                            /home/gunther/Desktop/bes/directory
01aca000-01aeb000 rw-p 00000000 00:00 0                                  [heap]
7f1b00000000-7f1b00021000 rw-p 00000000 00:00 0
7f1b00021000-7f1b04000000 ---p 00000000 00:00 0
7f1b0592e000-7f1b05944000 r-xp 00000000 08:01 24043880                  /lib/libgcc_s.so.1
7f1b05944000-7f1b05b43000 ---p 00016000 08:01 24043880                  /lib/libgcc_s.so.1
7f1b05b43000-7f1b05b44000 r--p 00015000 08:01 24043880                  /lib/libgcc_s.so.1
7f1b05b44000-7f1b05b45000 rw-p 00016000 08:01 24043880                  /lib/libgcc_s.so.1
7f1b05b45000-7f1b05cbf000 r-xp 00000000 08:01 24043728                  /lib/libc-2.11.1.so
7f1b05cbf000-7f1b05ebe000 ---p 0017a000 08:01 24043728                  /lib/libc-2.11.1.so
7f1b05ebe000-7f1b05ec2000 r--p 00179000 08:01 24043728                  /lib/libc-2.11.1.so
7f1b05ec2000-7f1b05ec3000 rw-p 0017d000 08:01 24043728                  /lib/libc-2.11.1.so
7f1b05ec3000-7f1b05ec8000 rw-p 00000000 00:00 0
7f1b05ec8000-7f1b05ee8000 r-xp 00000000 08:01 24043631                  /lib/ld-2.11.1.so
7f1b060c2000-7f1b060c5000 rw-p 00000000 00:00 0
7f1b060e3000-7f1b060e7000 rw-p 00000000 00:00 0
7f1b060e7000-7f1b060e8000 r--p 0001f000 08:01 24043631                  /lib/ld-2.11.1.so
7f1b060e8000-7f1b060e9000 rw-p 00020000 08:01 24043631                  /lib/ld-2.11.1.so
7f1b060e9000-7f1b060ea000 rw-p 00000000 00:00 0
7fffa1830000-7fffa1845000 rw-p 00000000 00:00 0                          [stack]
7fffa1955000-7fffa1956000 r-xp 00000000 00:00 0                          [vdso]
ffffffffff600000-ffffffffff601000 r-xp 00000000 00:00 0                  [vsyscall]

Aborted

---

Thanks for any hints.
Greets yodakohl

Code:

---

static int get_environment_index(char** env, char* name)///returns index of environment variable ,returns -1 if not found
{
    char temp = (char) malloc (strlen(name)+1)*sizeof(char);
    strcpy(temp, name);
    strcat(temp, "=");

---

Well, for starters, you aren't allocating enough space here. If 'name' is four, and you allocate five spaces, to hold 'name', and then you strcat "=" to the end of it, you have just overrun the space you've allocated.


Quzah.

Quote:

---

Originally Posted by quzah

Code:

---

static int get_environment_index(char** env, char* name)///returns index of environment variable ,returns -1 if not found
{
    char temp = (char) malloc (strlen(name)+1)*sizeof(char);
    strcpy(temp, name);
    strcat(temp, "=");

---

Well, for starters, you aren't allocating enough space here. If 'name' is four, and you allocate five spaces, to hold 'name', and then you strcat "=" to the end of it, you have just overrun the space you've allocated.


Quzah.

---

I'm sorry, i have posted my developement version. I have edited this back to my old version

One thing you should do when using realloc is to use a temporary pointer, so if the realloc fails you do not lose the memory already allocated and can clean up.

Code:

---

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

int main(void)
{
    int count = 0;
    char **strings = NULL;
    char buff[BUFSIZ];
    printf("Enter a string: ");
    while (fgets(buff, sizeof(buff), stdin))
    {
        char **temp = realloc(strings, (count + 1) * sizeof(*temp));
        if (!temp)
        {
            int i = 0;
            while (i < count)
                free(strings[i++]);
            free(strings);
            strings = NULL;
            break;
        }

        strings = temp;
        strings[count] = malloc(strlen(buff) + 1);
        strcpy(strings[count++], buff);
        printf("Enter a string: ");
    }

    printf ("\n");

    while (--count >= 0)
    {
        printf("String %d: %s", count, strings[count]);
        free(strings[count]);
    }
    free(strings);

    return 0;
}

---

Are the warnings turned all the way up on the compiler? Try gcc -Wall main.c env.c.
Why do you put so many statements on a single line (e.g. if and two subordinate statements)? Makes it very hard to read.

Quote:

---

Originally Posted by yodakohl

static int get_environment_index(char** env, char* name)///returns index of environment variable ,returns -1 if not found
{
char temp = (char) malloc (strlen(name)+1)*sizeof(char);

---

This line has a number of problems:

1. Don't cast malloc
2. You want temp to be a char *, not a char.
3. You need extra parentheses around the malloc parameters, or drop sizeof(char) since it's 1.
4. You need an extra extra byte, one for the mandatory null, one for the '=' you cat on the end: (strlen(name)+2).

You also need to do a +2 instead of a +1 on the following places:

1. malloc in make_env (again, for an '=' and a null)
2. environ_intern = malloc in enter_env, since env_size is the number of current env variables, plus one for the new var plus one for the null terminator

Then, you need to change the following (around line 111 in env.c) since sizeof(environ[i]) is sizeof(char *), no matter how much data is in there. You want strlen:

Code:

---

char *new_env = (char*) malloc (strlen(environ[i])*sizeof(char)+1);

---