CODEWITHSUNDEEP
carraysstringmemory-managementfree
darksky
darksky

Reputation: 21019

Freeing Object Error

I am mallocing an array of c strings. After releasing it, I get the following error:

Assembler(87536) malloc: *** error for object 0x108500840: pointer being freed was not allocated *** set a breakpoint in malloc_error_break to debug

Why is that? I am pretty sure I am doing the malloc correctly. I'm pretty experienced with memory management, but I am not sure why this is giving me an error. The array is should hold three strings, each of which is 2 characters long.

Here is how I am mallocing the array:

char **reg_store;
reg_store = malloc(3 * (sizeof(char*)));
if (reg_store == NULL) {
     fprintf(Out, "Out of memory\n");
     exit(1);
}

for (int i = 0; i < 3; i++) {
    reg_store[i] = malloc(2 * sizeof(char));
    if (reg_store[i] == NULL) {
          fprintf(Out, "Out of memory\n");
          exit(1);
    }
}

Here is how I am freeing it:

for (int i = 0; i < 3; i++) {
    free(reg_store[i]);
}
free(reg_store);

Here is what I have in between:

// Keeps a reference to which register has been parsed for storage

int count = 0;
char *reg = NULL;
char *inst_ptr // POINTS TO SOME STRING. EXAMPLE: $t2, $t1, $a0

while (1) {

    // Parses the string in inst_ptr with dollar, comma and space as a delimiter.
    reg = parse_token(inst_ptr, " $,\n", &inst_ptr, NULL);

    if (reg == NULL || *reg == '#') {
        break;
    }

    reg_store[count] = reg;
    count++;
    free(reg);
}

I am printing out reg after I call parse_token and it does print out correctly. I am also printing out reg_store[count] and it does also print out correctly.

Upvotes: 0

Views: 80

Answers (4)

Salvatore Previti
Salvatore Previti

Reputation: 9050

The error was already pointed out so no need to write it again. I can however point out that i don't like the way you are handling errors.

void freeRegStore(char** reg_store)
{
    int i;
    if (reg_store != NULL)
    {
        for (i = 0; i < 3; i++)
            free(reg_store[i]);

        free(reg_store);
    }
}

char** allocRegStore()
{
    int i;
    char **reg_store;
    reg_store = calloc(3 * (sizeof(char*)), 1);
    if (reg_store != NULL)
    {
        for (i = 0; i < 3; i++)
        {
            reg_store[i] = malloc(2 * sizeof(char));
            if (reg_store[i] == NULL)
            {
                freeRegStore(reg_store);
                return NULL;
            }
        }
    }
    return reg_store;
}

In this method, the function allocRegStore will return NULL if there was not enough memory without leaving pieces around. Then you can handle this case in main and not in the allocation function itself. I disagree with the use of printf and exit inside functions.

int main()
{
    char** reg_store = allocRegStore();

    if (reg_store == NULL)
    {
        puts("Out of memory");
        return 1;
    }

    ... do your stuff

    freeRegStore();
    return 0;
}

I can also say that the memory used by this program will never go out of memory :) i would not worry about that.

Upvotes: 0

Daniel
Daniel

Reputation: 31579

Your problem is here:

reg_store[count] = reg;
free(reg);

and later

free(reg_store[i]);

reg is already freed and you free it another time (not talking about the problems with using it later). to fix this replace

reg_store[count] = reg;

with

strcpy(reg_store[count], reg);

or as suggested in the comments, since you know its two charaters, its better to memcpy it:

memcpy(reg_store[count], reg, 2);

Upvotes: 1

mu is too short
mu is too short

Reputation: 434635

Your problem is in the "in between" code, in particular, right here:

reg_store[count] = reg;
count++;
free(reg);

You allocated reg_store[count] with malloc during your set up, then you overwrite the allocated value with reg and then free reg. The result is a memory leak from the original pointers that were in reg_store and a double-free on each element of reg_store when you try to clean everything up.

You need to copy reg into the memory already allocated in reg_store[count] (watching the size of course) or don't allocate any space for the elements of reg_store before the "in between" code at all.

Upvotes: 1

Eamonn O&#39;Brien-Strain
Eamonn O&#39;Brien-Strain

Reputation: 3412

I would suggest adding some printfs (or use the debugger) to see the values of all the malloced pointers just after they have been malloced. Then do the same just before they are freed, to make sure they are the same. Perhaps there is some other rogue code elsewhere in the program that is stomping over memory.

Upvotes: 1

Related Questions