0

I know that this question exists in other places like:

pointer being freed was not allocated in C error: pointer being freed was not allocated

but, I'm still very confused. Errors seem to be associated with things like "modifying the original pointer returned by malloc" and "failing to malloc before freeing". I just don't see how these reasons apply to my program.

I'm writing a dynamically allocated array of strings:

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

#define NLETTERS 25

typedef struct {
    char** array;
    size_t used;
    size_t size;
} array_t;

array_t* initArray(size_t initialSize) {
    array_t* a = malloc(sizeof(array_t));
    a->array = malloc(initialSize*sizeof(char*));
    a->used = 0;
    a->size = initialSize;

    int i;
    for(i = 0; i < initialSize; i++) {
        a->array[i] = malloc(sizeof(char) * NLETTERS);
    }

    return a;
}

void insertArray(array_t *a, char* element) {
    if (a->used == a->size) {
        a->size *= 2;

        a->array = realloc(a->array, a->size * sizeof(char*));

        int i;
        for(i = (int)a->used; i < a->size; i++) {
            a->array[i] = malloc(sizeof(char) * NLETTERS);
        }
    }
    a->array[a->used++] = element;
}

void freeArray(array_t *a) {
    int i;
    for(i = 0; i < a->size; i++) {
        free(a->array[i]);
    }

    free(a->array);
    free(a);
    a->array = NULL;
    a->used = a->size = 0;
}

void print_array(array_t *a) {
    int i;
    for(i = 0; i < a->size; i++) {
        printf("%s\n", a->array[i]);
    }
}

int main(int argc, const char * argv[]) {
    array_t *a;
    a = initArray(2);
    insertArray(a, "hello");
    insertArray(a, "how are you");
    print_array(a);
    insertArray(a, "yup");
    insertArray(a, "you know it");
    print_array(a);

    freeArray(a);

    return 0;
}

When I attempt to "free", I get the error: "pointer being freed was not allocated" right at

free(a->array[0]) 

in the first iteration of the for-loop in freeArray();

Help would be greatly appreciated.

Community
  • 1
  • 1
Chris
  • 47
  • 6
  • Are you suggesting that my realloc is incorrect? What should it change to? I'm not really understanding what the fix is under that link. – Chris Nov 29 '16 at 04:34
  • Note that in `insertArray()`, you have the idiom `old_ptr = realloc(old_ptr, new_size);`. This is bad. If (when!) `realloc()` fails, you leak memory because the `old_ptr` is over-written with NULL, so you can no longer free the old memory, even though it is still allocated. Use `new_ptr = realloc(old_ptr, new_size); if (new_ptr == NULL) { …report error, etc… } old_ptr = new_ptr;` (and usually also `old_size = new_size;` too). – Jonathan Leffler Nov 29 '16 at 04:56

3 Answers3

2

In your code, by saying

  a->array[a->used++] = element;

you're overwriting the allocated memory by malloc(), so, later while passing it to free() causes the issue.

Related, quoting C11, chapter §7.22.3.3, The free function, (emphasis mine)

The free function causes the space pointed to by ptr to be deallocated, that is, made available for further allocation. If ptr is a null pointer, no action occurs. Otherwise, if the argument does not match a pointer earlier returned by a memory management function, or if the space has been deallocated by a call to free or realloc, the behavior is undefined.

Also, at a later point, this causes memory leak, as the memory allocated by malloc() is not getting free()-d, actually.

Solution: You should use strcpy() to copy the content into the allocated memory.

Sourav Ghosh
  • 133,132
  • 16
  • 183
  • 261
0

The source of the problem is the line:

a->array[a->used++] = element;

It has two problems:

  1. It leaks memory. The memory returned by malloc() is lost.

  2. It points to read only memory used for string literals, which causes problems when you call free on it.

Replace that line with

strcpy(a->array[a->used++], element);
R Sahu
  • 204,454
  • 14
  • 159
  • 270
0

I see two issues :-

  1. a->array[a->used++] = element;
    You have allocated memory but inplace of using that, you are again pointing it to some other location resulting in memory leak. So change it to :-

    strcpy(a->array[a->used++], element);

  2. Change FreeArray like this (you are freeing memory and than using it, which is resulting in seg fault).

    void freeArray(array_t *a) {
        int i;
        for(i = 0; i < a->size; i++) {
            free(a->array[i]);
        }
    
        free(a->array);
        a->array = NULL;
        a->used = a->size = 0;
        free(a);
    }
    
instance
  • 1,366
  • 15
  • 23