Search code examples
cstringmallocdynamic-memory-allocation

strcpy in C is illegally altering the copied string?


This is a really mysterious behavior I can't get around.

I'm trying to edit the string of one variable and then copy again to the source. I have the following code:

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

int main() {
    char word[100] = "1 hash";
    char* sp = malloc(100);
    sp = strchr(word, ' ');
    sp++;
    // the "bug" still happens with strcpy  instead of strncpy
    strncpy(word, sp, 100);

    printf("%s\n", word);
}

The output is:

hhsh

It should've been hash.

The weird thing is that "1 hash" is the only string I found that this bug happens. All the other strings I tried gave me the expected output. For example: "1 huhanh" -> huhanh or "3 a h c" -> a h c

Any help would be greatly appreciated.


Solution

  • You have undefined behavior. The malloc pointer is ignored.

    So, when you do the strcpy, sp and word are part of the same string.

    This is UB. Which means it may segfault. It may work. It may produce incorrect results.

    In this instance, we can't predict the order of the fetching of the bytes and the storing. With certain sequences, it will [appear to] work. With others, we may fetch a byte that was not an input but an output from the previous byte.

    Here is the corrected code:

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    int
    main()
    {
        char word[100] = "1 hash";
        char *copy = malloc(100);
    
        char *sp = strchr(word, ' ');
        sp++;
    
        strncpy(copy, sp, 100);
    
        printf("%s\n", copy);
    }
    

    UPDATE:

    Thanks for the answer. I indeed didn't realized they were all part of the same string. In the example code u provided, does the pointer sp still have to exist? – Pedro Vinícius

    Yes, because it is where you are copying from. Notice that you get the pointer from strchr. Then, you increment it. It is this updated value that becomes the source address.

    This is clean code.

    But, you could do:

    strncpy(copy, strchr(word, ' ') + 1, 100);
    

    But, I wouldn't do this. IMO, it's "too cute". The original sp code will execute just as fast. And, it's easier to read. And it let's you do some error checking:

    #include <stdio.h>
    #include <stdlib.h>
    #include <string.h>
    
    int
    main()
    {
        char word[100] = "1 hash";
        char *copy = malloc(100);
    
        char *sp = strchr(word, ' ');
    
        if (sp != NULL) {
            sp++;
            strncpy(copy, sp, 100);
        }
        else
            *copy = 0;
    
        printf("%s\n", copy);
    }