Why malloc can't work with strcpy?

Question

char * removeChar(char * str, char c){
    int len = strlen(str);
    int i = 0;
    int j = 0;
    char * copy = malloc(sizeof(char) * (len + 1));
    while(i < len){
        if(str[i] != c){
            copy[j] = str[i];
            j++;
            i++;
        }else{
            i++;
        }
   }

    if(strcmp(copy, str) != 0){
        strcpy(str,copy);

    }else{
        printf("Error");
    }
    return copy;
}




int main(int argc, char * argv[]){
    char str[] = "Input string";
    char * input;
    input = removeChar(str,'g');
    printf("%s\n", input);
    free(input);
    return 0;
}

I don't know why every time I try to run it ,it always says uninitialized variable and sticks in the strcpy line and printf line.

Basically this function is to take a string, and a character and removes the that character from the string (because I am learning malloc so that's why I wrote the function like this).

Answer 1

After the while loop do:

copy[j] = '\0';

to NULL-terminate your string; that way it can work with methods coming from <string.h>, which assume that the string is nul-terminated.

---

PS: One warning you should see is about not returning copy in your function in any case, because now if the condition of the if statement is wrong, your function won't return something valid, so add this:

return copy;

at the end of your function (which is now corrected with your edit).

Other than that, the only warning you should still get are for the unused arguments of main(), nothing else:

prog.c: In function 'main':
prog.c:32:14: warning: unused parameter 'argc' [-Wunused-parameter]
 int main(int argc, char * argv[]){
              ^~~~
prog.c:32:27: warning: unused parameter 'argv' [-Wunused-parameter]
 int main(int argc, char * argv[]){
                           ^~~~

Answer 2

While you copy over bytes from str to copy, you don't add a terminating null byte at the end. As a result, strcmp reads past the copied characters into unitialized memory, possibly past the end of the allocated memory block. This invokes undefined behavior.

After your while loop, add a terminating null byte to copy.

Also, you never return a value if the if block at the end is false. You need to return something for that, probably the copied string.

char * removeChar(char * str, char c){
    int len = strlen(str);
    int i = 0;
    int j = 0;
    char * copy = malloc(sizeof(char) * (len + 1));
    while(i < len){
        if(str[i] != c){
            copy[j] = str[i];
            j++;
            i++;
        }else{
            i++;
        }
    }
    //  add terminating null byte
    copy[j] = '\0';

    if(strcmp(copy, str) != 0){
       strcpy(str,copy);
    }
    // always return copy
    return copy;
}

Answer 3

You never initialised input and the some compilers don't notice, that the the value is never used before the line

input = removeChar(str, 'g');

in your code. So they emit the diagnostic just to be sure.

strcpy(str, copy)

gets stuck in your code, as copy never got a closing 0 byte and so depends on the nondeterministic content of your memory at the moment of the allocation of the memory backing copy, how long strcpy will run and if you get eventually a SIGSEGV (or similar).

strcpy will loop until it finds a 0 byte in your memory.

Answer 4

For starters to remove a character from a string there is no need to create dynamically a character array and then copy this array into the original string.

Either you should write a function that indeed removes the specified character from a string or a function that creates a new string based on the source string excluding the specified character.

It is just a bad design that only confuses users. That is the function is too complicated and uses redundant functions like malloc, strlen, strcmp and strcpy. And in fact it has a side effect that is not obvious. Moreover there is used incorrect type int for the length of a string instead of the type size_t.

As for your function implementation then you forgot to append the terminating zero '\0' to the string built in the dynamically allocated array.

If you indeed want to remove a character from a string then the function can look as it is shown in the demonstrative program.

#include <stdio.h>

char * remove_char(char *s, char c)
{
    char *p = s;

    while (*p && *p != c) ++p;

    for ( char *q = p; *p++; )
    {
        if (*p != c) *q++ = *p;
    }

    return s;
}

int main( void )
{
    char str[] = "Input string";

    puts(str);
    puts(remove_char(str, 'g'));

    return 0;
}

The program output is

Input string
Input strin

If you are learning the function malloc and want to use it you in any case should try to implement a correct design.

To use malloc you could write a function that creates a new string based on the source string excluding the specified character. For example

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

char * remove_copy_char(const char *s, char c)
{
    size_t n = 0;

    for (const char *p = s; *p; ++p)
    {
        if (*p != c) ++n;
    }

    char *result = malloc(n + 1);

    if (result)
    {
        char *q = result;

        for (; *s; ++s)
        {
            if (*s != c) *q++ = *s;
        }

        *q = '\0';
    }

    return result;
}

int main( void )
{
    char *str = "Input string";

    puts(str);

    char *p = remove_copy_char(str, 'g');

    if ( p ) puts(p );

    free(p);

    return 0;
}

The program output will be the same as above.

Input string
Input strin

Pay attention to the function declaration

char * remove_copy_char(const char *s, char c);
                        ^^^^^^

In this case the source string can be a string literal.

char *str = "Input string";