C: Realloc invalid pointer?

Question

This is my task from school: Write a function insertString that inserts the string s2 into s1 at index n.s1 has been allocated using malloc and should be resized (the function is void again).

The program gave me a NULL on PC and when I switched to the phone the compilor said my realloc has a invalid pointer. But I really don't know what I did wrong.

Here is the code:

void insertString(char *str1, char *str2, int n){
    int lengStr2=strlen(str2);
    printf("%d %d ", lengStr2,n);
    printf("\nstr1= %s, str2: %s, n: %d ",str1,str2,n);
    str1=(char*)realloc(str1,lengStr2+n+1);
    if (str1==NULL){
        printf("Error\n");
        free(str1);
        return -1;
    }
        printf("\nstr1= %s, str2: %s, n: %d ",str1,str2,n);
    memcpy(str1+n,str2,lengStr2+1);
        printf("\nstr1= %s, str2: %s, n: %d ",str1,str2,n);
}
void testInsertString( char *str2, int n, char *expect){
    char*str1=(char*)malloc(3*sizeof(char));
    str1="hoi";
    printf("\nstr1= %s, str2: %s, n: %d ",str1,str2,n);
    insertString(str1,str2,n);
    printf("--> result:%s --> ",str1);
    (strcmp(str1,expect)==0)?printf("Success"): printf("Failure");
    free(str1);
    printf("\nIs Free\n");
}

Here the output:

str1= hoi, str2: Hallo, n: 1 5 1
str1= hoi, str2: Hallo, n: 1 Error
--> result:hoi --> Failure
Is Free

Process returned 0 (0x0)

Please if you know what I did wrong can you show me the correct version? Even so I have the problem that I can't right a program right just by reading a text, I need to see how it should be written. So I need to see the right result to learn from the mistake ^^" (that's why some stuff in school is for me really hard). Thanks in advance ^^

`str1` is a string literal. It cannot be realloced. After `str1=malloc(...)`, you need to copy data into `str1` with `sprintf` or `strcat` or the like. Reassigning `str1="hoi"` throws away the address of the memory you allocated. It is a memory leak. — William Pursell, Jan 13 '21 at 21:29
consider adding `-Wwrite-strings` to compiler flags; that would cause it to warn about `str1="hoi"`. — M.M, Jan 13 '21 at 21:37
OT: regarding: `int lengStr2=strlen(str2);` the function: `strlen()` returns a `size_t` (an unsigned value) not an `int` — user3629249, Jan 14 '21 at 01:58
in function: `void insertString(char *str1, char *str2, int n){` this function is declared (via the `void`) to not return a value, However, that function contains: `return -1;` When compiling, always enable the warnings, then fix those warnings. ( for `gcc`, at a minimum use: `-Wall -Wextra -Wconversion -pedantic -std=gnu11` ) Note: other compilers use different options to produce the same results — user3629249, Jan 14 '21 at 02:03
regarding: `str1=(char*)realloc(str1,lengStr2+n+1); if (str1==NULL){ printf("Error\n"); free(str1); return -1;` 1) when calling: `realloc()` always assign to a 'temp' variable. Otherwise, when the function fails, the original pointer is lost (resulting in a memory leak) 2) error messages should be output to `stderr`, not `stdout` Suggest using: `perror( "realloc failed" );` 3) the returned type is `void*` which can be assigned to any pointer. Casting just clutters the code (and is error prone) — user3629249, Jan 14 '21 at 02:09
regarding: `str1="hoi";` a string cannot be directly assigned, except at initialization. Suggest: `strcpy( str1, "hoi" );` However note that `str1` was only allocated 3 bytes and a string always has a trailing NUL byte so actually needs 4 bytes — user3629249, Jan 14 '21 at 02:15
regarding: `str1=(char*)realloc(str1,lengStr2+n+1);` This allocates enough bytes for `str2` + 1 for the NUL byte (and depending on `n`) the leading part of `str1`. The assignment says to `insert`, not `replace` so also need enough room for the rest of the original `str1`, — user3629249, Jan 14 '21 at 02:37
what happens when `n` is larger than the original length of `str1`? — user3629249, Jan 14 '21 at 02:37

score 2 · Answer 1 · answered Jan 13 '21 at 21:32

char *str1 = malloc(3*sizeof(char));  /* Allocate memory */
str1="hoi";       /* Discard the only reference to the allocated memory */

The above two lines are similar in spirit to:

int x = 5; x = 7;

You need to copy the string "hoi" into the newly allocated memory, and you need to allocate at least 4 bytes to hold the string "hoi". For example:

char *hoi = "hoi";
char *str1 = malloc(strlen(hoi) + 1);
if( str1 == NULL ){
    perror("malloc");
    exit(EXIT_FAILURE);
}
sprintf(str1, "%s", hoi);

dspr · Answer 2 · 2021-01-14T09:26:45.230

Here is a fixed version :

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

// Changing the signature to return the reallocated ptr would be safer
char * insertString(char *str1, char *str2, int n){
    int lengStr2=strlen(str2);

    //because str1 could differ from the received ptr after reallocation
    str1=(char*)realloc(str1,lengStr2+n+1); 
    if (str1==NULL){
        printf("Error\n");
        return NULL;
    }
    strcpy(str1+n,str2);
    return str1; // So we return the new ptr to the caller
}
void testInsertString( char *str2, int n, char *expect){
    char*str1=(char*)malloc(6*sizeof(char)); // The null termination must be counted
    strcpy(str1, "Hello");                   // You should use strCpy here instead of =
    str1 = insertString(str1,str2,n);        // Refreshes the ptr
    printf("\n--> result:%s --> ",str1);
    (strcmp(str1,expect)==0)?printf("Success"): printf("Failure");
    free(str1);
}

int main() {
    testInsertString(" World", 5, "Hello World"); // --> result:Hello World --> Success
    return 0;
}

You may want to compare the second comment in this snippet with the subsequent `if` statement. Hint: https://stackoverflow.com/questions/44789295/correct-use-of-realloc — Bob__, Jan 13 '21 at 22:48

C: Realloc invalid pointer?

2 Answers2