C: malloc error-pointer being freed was not allocated

Question

I am trying to return an array of string from a function and then free the memory it used. The code is below:

int main(int argc, const char * argv[])
{
    for (int m = 0; m < 10000; m++) {
        char **data = dataTest();

        int i = 0;
        while(data[i]) {
            printf("%p ",data[i]);
            free(data[i]);
            i++;
        }
        printf(" address= %p.\n",data);
        free(data);
    }

    return 0;
}

Here is the function:

char **dataTest()
{
    char *row[] = {"this", "is", "a", "data", "string", NULL};
    char **str = row;
    char **dataReturn = (char **) malloc(sizeof(char *) * 6);

    int i = 0;
    while (*str) {
        dataReturn[i] = malloc(sizeof(char) * strlen(*str));
        strcpy(dataReturn[i++], *str);
        str++;
    }

    return dataReturn;
}

It runs well in the beginning, but soon the error occurs. Below is the result. The address goes wrong somehow and the malloc error happens. Anyone has met the same problem before?

0x100300030 0x100300040 0x100300050 0x100300060 0x100300070  address= 0x100300000.
0x100300030 0x100300040 0x100300050 0x100300060 0x100300070  address= 0x100300000.
0x100400030 0x100300030 0x100300040 0x100300050 0x100300060  address= 0x100400000.
testC(562,0x7fff73e71310) malloc: *** error for object 0x3000000000000:     
pointer being freed was not allocated
*** set a breakpoint in malloc_error_break to debug
0x100300060 0x100300070 0x100300030 0x100300040 0x100300050 0x3000000000000                 
Program ended with exit code: 9

Please see why not to cast the return value of malloc() and family in C. — Sourav Ghosh
– Sourav Ghosh, Commented Jul 13, 2015 at 14:25
I think you just forgot to add one to the result of strlen for the null terminator. And BTW, sizeof(char) is always unnecessary, because it is 1 by definition. — Fred Larson
– Fred Larson, Commented Jul 13, 2015 at 14:25
printf(" address= %p.\n",data); should be printf(" address= %p.\n",(void *)data);. That's one of few cases where you actually should cast a pointer. As others have said: char ** malloc(sizeof(char *) * 6); should be: malloc( sizeof *dataReturn * 6);. It's easier to read and understand anyway — Elias Van Ootegem
– Elias Van Ootegem, Commented Jul 13, 2015 at 14:33
For completeness on why printf("%p", (void *) data) needs the cast, quote from the standard: (C11, 7.21.6.1p8 Formatted input/output functions) "p The argument shall be a pointer to void.". Put simply: not passing a void * to printf here is UB — Elias Van Ootegem
– Elias Van Ootegem, Commented Jul 13, 2015 at 14:38

Jabberwocky · Accepted Answer · 2015-07-13 14:43:08Z

8

You need to add this to just before return dataReturn; in your dataTest function:

dataReturn[i] = NULL ;

otherwise your while (data[i]) {} will continue further than wanted.

And instead of:

dataReturn[i] = malloc( sizeof(char) * (strlen(*str)) );

write:

dataReturn[i] = malloc(strlen(*str) + 1);

in order to allocate space for the terminating zero.

BTW sizeof (char) is always 1.

edited Jul 13, 2015 at 14:43

answered Jul 13, 2015 at 14:34

Jabberwocky

51.3k18 gold badges71 silver badges127 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Ingo Leonhardt Over a year ago

I think you mean while(data[i]) (in the free() loop) instead of while (*str)

dbush Over a year ago

@ButterLover Feel free to accept this answer if it was useful to you.

Collectives™ on Stack Overflow

C: malloc error-pointer being freed was not allocated

1 Answer 1

2 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related