Realloc triggered a breakpoint

Question

int n = 0;
char s[] = "I am a college student, I live in Vietnam";
char **a = NULL;
char **temp = NULL;

char* pc = strtok(s, ",");

while (pc)
{
    n++;
    temp = (char**)realloc(a, n * sizeof(char*));

    if (temp)
    {
        a = temp;
        free(temp);
        temp = NULL;
    }
    else
    {
        for (int k = 0; k < n; k++)
        {
            free(a);
            a = NULL;
        }
        free(*a);
        *a = NULL;
    }

    a[n - 1] = (char*)malloc((strlen(pc) + 1) * sizeof(wchar_t));
    strcpy(a[n - 1], pc);
    a[n - 1][strlen(pc)] = '\0';

    pc = strtok(NULL, ",");
}
for (int i = 0; a[i]; i++)
{
        free(a[i]);
        a[i] = NULL;
}
free(*a);
*a = NULL;

In while loop. I used realloc to allocate with each element is char*. It worked normally when I allocated first element. But in the second times when I allocated "I live in Vietnam".

    temp = (char**)realloc(a, n * sizeof(char*));

It has triggered a breakpoint. Realloc does copy all the data when it reallocate, so it must have copied "I am a college student". Why's this error happened?

And after that. In freeing memory part. I haven't tried but it must be something wrong here. I haven't figured it out. Thank you for helping me

score 0 · Accepted Answer · answered Apr 16 '18 at 01:59

You are using realloc wrong:

This is wrong

temp = (char**)realloc(a, n * sizeof(char*));

if (temp)
{
    a = temp;
    free(temp);  // <-- wrong, you cannot do that
    temp = NULL;
}
...

If realloc returns a different pointer, it will take care of copying the data from the old destination into the new one and it will free the old destination. You don't have to worry about freeing the memory from the old location.

Here however you are reallocation new memory, setting a to the reallocated memory and immediately freeing it again. Then you are accessing memory that has been freed. This yield undefined behaviour.

What you've to do is this:

temp = realloc(a, n * sizeof *a);

if(temp == NULL)
{
    // DO the error handling
    for(int k = 0; k < n; ++k)
        free(a[i]);
    free(a);
    break; // or return
}

a = temp;

a[n - 1] = malloc(strlen(pc) + 1);
...

Are you really sure that for the a[i] you want to allocate (strlen(pc) + 1) * sizeof(wchar_t) bytes? Are you really dealing with wchar_t strings here? In that case it would be better to declare a as wchar_t **a instead.

Also note: do not cast malloc

Realloc triggered a breakpoint

1 Answers1