Getting (core dumped) when using realloc

Question

void replace(char *str) {
    unsigned int len = 0;
    unsigned int no_of_spaces = 0;
    while (*str) {
        if ((char)*str == SPACE)
            no_of_spaces++;
        str++;
        len++;
    }

    unsigned int new_len = len + 2 * no_of_spaces;
    str = (char*) realloc(str, new_len * sizeof(char));
    str[new_len] = '\0';
}

I use function like replace("random string");.

Here I am trying to increase the size of the string so that spaces can be replaced with another string. For this I need to count the no of spaces and also get the length of the original string. I have been able to do that.

For resizing I am using realloc but when I run it, it gives Aborted (core dumped)?

Answer 1

Was your original string allocated using malloc? or realloc? Perhaps you are trying to increase the size of a static string (a string literal):

char sStatic[256];   // cannot realloc

char *sNonStatic = NULL;   // can / must realloc

replace("random string") // cannot realloc within replace

EDIT: after reading your comment, you should take a copy of your incoming string, increase the size, then output a copy/ new string. You cannot increase the size of a constant string (a string literal).

Answer 2

The only pointers that can be passed to realloc are null pointers and those that have been returned by calloc, malloc or realloc previously!

This is important because you mentioned that you've called your function like replace("random string")... Is "random string" a null pointer, or returned by one of those *alloc functions? No. Perhaps you meant to use strdup or something (e.g. char *foo = strdup("random string"); replace(foo); free(foo);)? strdup is a POSIX function (e.g. not C-standard like the *alloc functions) but it should return something returned by the *alloc functions.

---

Following this code:

unsigned int new_len = len + 2 * no_of_spaces;
str = (char*) realloc(str, new_len * sizeof(char)); /* NOTE there's a potential memory leak
                                                     * when realloc returns NULL here, though
                                                     * that's the least of your problems */

... you must check str to ensure realloc succeeded, and only then the only valid indexes for str are between 0 and new_len - 1. This is either a null pointer dereference or a buffer overflow:

str[new_len] = '\0';

Perhaps you meant the following:

size_t new_len = len + 2 * no_of_spaces;
void *temp = realloc(str, new_len + 1); /* <--- NOTE +1 HERE! */
if (temp == NULL) {
    /* XXX: Bomb out due to allocation failure */
}
str = temp;

... and now the valid indexes are between 0 and new_len + 1 - 1, so this is valid:

str[new_len] = '\0';

Answer 3

This line is incorrect, since valid indices range from 0 .. new_len - 1:

str[new_len] = '\0';

It should probably be:

str[new_len - 1] = '\0';

You also have a couple of other potential problems:

• realloc can return NULL - you should check for this

• in the case that realloc fails you lose your original str pointer and get a memory leak - you should use a temp pointer for the result, test this for NULL, and then only if realloc has succeeded should you set str equal to temp:

---

char * temp = realloc(str, new_len);
if (temp == NULL)
{
    // handle error here...
}
else
{
    str = temp; // success...
    str[new_len - 1] = '\0';
}

---

• not a bug as such, but you have a lot of unnecessary casts which are potentially dangerous as they can masks bugs that would otherwise generate compiler errors or warnings. You can safely remove all the casts in your code above.

Answer 4

You also move the pointer

while (*str) {
    if ((char)*str == SPACE)
        no_of_spaces++;
    str++;
    len++;
}

and when you are at the end you try to reallocate it. But you are already away from where the array is. Use a temp variable here. And as the others said. The string is hopefully created with malloc and not as an array.

And str[new_len] = '\0'; is out of bounds.