What is wrong with this concatenation function?

Question

char* StrCat(char* dest, char* source)
{
    char* retVal = dest;

    while(*dest)
        dest++;

    while(*dest++ = *source++) ;

    return retVal;
}

int main()
{
    char* a = "One";
    char* b = "Two";

    char* x = StrCat(a, b);

    printf("%s\n", x);

    return 0;
}

The program crashes the first time when copying from source to destination, second while loop. (Access violation error)

First tell me what is right with this concatenation function (that you think) ? — haccks, Sep 23 '14 at 14:29
It looks like he's learning C. It's a good thing to try re-implementing strcat manually without any help of libc. — superjedi, Sep 23 '14 at 14:38
One thing about C-style strings/charArrays is that you cannot just magically extend them, eg. by seeking to the end and adding more chars from another string. It's UB. — Martin James, Sep 23 '14 at 15:44

superjedi · Accepted Answer · 2014-09-23T14:43:52.297

2

Content of variables a and b (character strings "One" and "Two") are stored in read-only segments, content is protected, you cannot overwrite its data.

Plus, StrCat() function has not much logic.

You'd need to use a global char array, or a char array that you pass to the function that has a limited size and make sure you don't go beyond its buffer size during the copy, otherwise you will end up with a stack overflow.

Another solution would be to allocate memory dynamically using malloc, this memory should be freed when you don't need it anymore.

Rethink how you would write such a function to correct it, but first fix this memory issue.

edited Sep 23 '14 at 14:43

answered Sep 23 '14 at 14:30

superjedi

524
3
7

2

a and b aren't stored in read-only segments. What they point to is. – Tom Tanner Sep 23 '14 at 14:39
Totally true - Thanks for the comment Tom. – superjedi Sep 23 '14 at 14:43
It won't be stack overflow, it will be UB. – Martin James Sep 23 '14 at 15:45

What is wrong with this concatenation function?

1 Answers1