Error freeing C** pointer malloced in function

Question

The program runs fine except for the last free, which results in the program freezing. When I comment out the last 'free' it runs fine.

The program gets all substrings from a string and returns it.

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

char** getPrefixes(char* invoer);

int main()
{
    char buffer[100];
    char *input;
    char **prefixes;
    int counter = 0;

    puts("Give string.");
    fgets(buffer, 99, stdin);
    fflush(stdin);

    if (buffer[strlen(buffer) - 1] == '\n')
        buffer[strlen(buffer) - 1] = '\0';

    input= (char*)malloc(strlen(buffer) + 1);

    if (input == NULL)
    {
        puts("Error allocating memory.");
        return;
    }

    strcpy(input, buffer);

    prefixes = (char**) getPrefixes(input);

    for (counter = strlen(input); counter > 0;  counter--)
    {
        puts(prefixes[counter]);
        free(prefixes[counter]);
    }

    free(input);

    free(prefixes);
}

char** getPrefixes(char* input)
{
    char** prefixes;
    int counter;

    prefixes = malloc(strlen(input) * sizeof(char*));

    if (prefixes == NULL)
    {
        puts("ELM.");
        return NULL;
    }


    for (counter= strlen(input); counter> 0; counter--)
    {
        prefixes[counter] = (char*)malloc(counter + 1);
        strcpy(prefixes[counter], input);
        input++;
    }

    return prefixes;
}

Thanks in advance!

Answer 1

The reason for your program freezing is simple: undefined behaviour + invalid return values: _Your main function returns void, not an int: add return 0 ASAP! If you type in echo $? in your console after executing your compiled binary, you should see a number other than 0. This is the program's exit code. anything other than 0 means trouble. if the main did not return an int, it's bad news.

Next:
The undefined behaviour occurs in a couple of places, for example right here:

prefixes = malloc(strlen(input) * sizeof(char*));
//allocate strlen(input) pointers, if input is 10 long => valid indexes == 0-9
for (counter= strlen(input); counter> 0; teller--)
{//teller doesn't exist, so I assume you meant "counter--"
    prefixes[teller] = (char*)malloc(counter + 1);//first call prefixes[10] ==> out of bounds
    strcpy(prefixes[counter], input);//risky, no zero-termination... use calloc + strncpy
    input++;
}

Then, when free-ing the memory, you're not freeing the pointer @ offset 0, so the free(prefixes) call is invalid:

for (counter = strlen(input); counter > 0;  counter--)
{//again 10 --> valid offsets are 9 -> 0
    puts(prefixes[counter]);
    free(prefixes[counter]);
}
free(prefixes);//wrong

Again, valid indexes are 0 and up, your condition in the loop (counter > 0) means that the loop breaks whenever counter is 0. You, at no point, are freeing the first pointer in the array, the one at index/offstet 0.

Write your loops like everyone would:

for (int i=0, size_t len = strlen(input); i<len; ++i)
{
    printf("%d\n", i);//prints 0-9... 10 lines, all valid indexes
}

Change your loops, and make sure you're only using the valid offsets and you _should be good to go. using strncpy, you can still get the same result as before:

for (int i=0;i<len;++i)
{
    //or malloc(i+2), char is guaranteed to be 1
    //I tend to use `calloc` to set all chars to 0 already, and ensure zero-termination
    prefixes[i] = malloc((i+2)*sizeof(*prefixes[i]));
    strncpy(prefixes[i], input, i+1);//max 1 - 10 chars are copied
}

If we apply this to your code, and re-write it like so:

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

char** getPrefixes(char* input);

int main( void )
{
    char *input;
    char **prefixes;
    int counter, i;

    input= calloc(50,1);
    if (input == NULL)
    {
        puts("Error allocating memory.");
        return;
    }
    strcpy(input, "teststring");

    prefixes = getPrefixes(input);
    counter = strlen(input);
    for (i=0; i<counter;++i)
    {
        puts(prefixes[i]);
        free(prefixes[i]);
    }

    free(input);
    free(prefixes);
    return 0;
}

char** getPrefixes(char* input)
{
    int i, counter = strlen(input);
    char** prefixes = malloc(counter * sizeof *prefixes);

    if (prefixes == NULL)
    {
        puts("ELM.");
        return NULL;
    }

    for (i=0; i<counter; ++i)
    {
        prefixes[i] = calloc(i + 2,sizeof *prefixes[i]);
        strncpy(prefixes[i], input, i+1);
    }
    return prefixes;
}

The output we get is:

t
te
tes
test
tests
testst
teststr
teststri
teststrin
teststring

As you can see for yourself

on this codepad

Answer 2

allocating memory for pointer to pointer:

char** cArray = (char**)malloc(N*sizeof(char*));

for(i=0;i<N;i++)
    cArray[i] = (char*)malloc(M*sizeof(char));

De-allocating memory - in reverse order:

for(i=0;i<N;i++)
    free(cArray[i]);
free(cArray)

I hope this gives you a little insight on what's wrong.

Answer 3

you are calling strcpy with prefixes[counter] as destination. However, you've only allocated 4/8 bytes per prefixes[counter] depending on the size of (char*)

When you call strcpy you're copying all of input all the way to the end requiring strlen(input)! space

Doing this will corrupt the heap which might explain why the program is freezing.