Search code examples
c++pointerscopyc-stringsgarbage

Why does my char* copier return different things?


Writing a simple string copier and testing it in the main() fucntion. What's odd is that sometimes the program returns

"HelloHello"

like it should, but maybe every third time I run it, the program prints out:

"Hello!Hello!▌▌▌▌▌▌▌▌▌▌▒"UòB╚"

Why is the tail of garbage data only sometimes being added to the end of my second string?

#include <iostream>
using namespace std;

int strlength(const char* c)
{
    int size = 0;
    while (*c) {
        ++c;
        ++size;
    }
    return size;
}

char* mystrdup(const char* c)
{
    int size = strlength(c);
    char* result = new char;
    copy(c, c + size, result);
    return result;      
}

void print_array(const char* c)
{
    int size = strlength(c);
    while (*c) {
        cout << *c;
        ++c;
    }
}

int main()
{
    char test[] = "Hello!";
    char* res = mystrdup(test);
    print_array(test);
    print_array(res);
}


Solution

  • The program has undefined behavior because you are allocating not enough memory for the result string.

    char* mystrdup(const char* c)
    {
        int size = strlength(c);
        char* result = new char;
        ^^^^^^^^^^^^^^^^^^^^^^^           
        copy(c, c + size, result);
        return result;      
    }
    

    Moreover you are not copying the terminating zero to the result string.

    At least the two functions strlength and mystrdup can look the following way

    size_t strlength( const char *s )
    {
        size_t size = 0;
    
        while ( s[size] ) ++size;
    
        return size;
    }
    
    char * mystrdup( const char *s )
    {
        size_t size = strlength( s ) + 1;
    
        char *result = new char[size];
    
        copy( s, s + size, result );
    
        return result;      
    }
    

    Of course instead of the standard algorithm std::copy you could use the standard C function strcpy declared in the header <cstring>.

    strcpy( result, s );
    

    And do not forget to delete the allocated array.

    char* res = mystrdup(test);
    //…
    delete [] res;
    

    Pay attention to that the function print_array does not use the variable size. There is no need to output a C-string character by character.

    The function could be defined like

    std::ostream & print_array( const char *s, std::ostream &os = std::cout )
    {
        return os << s;
    }
    

    And at last the identifier c is usually used with single objects of the type char. If you deal with a string then it is better to use the identifier s.