C function using strcat with strings

Question

I'm trying to learn C. So I've challenged myself to create a function called two() which will effectively "double" a string.

two("foo") // => "foofoo"

But I'm having trouble using strcat() in conjunction with pointers. Here's what I have:

char *two(char *foo);

int main() {
    printf("The value of two(\"foo\") is %s", two("foo"));
}

char *two(char *foo) {
    return strcat(foo, foo);
}

It compiles but errors on run. Why?

I have a feeling the error resides in using strcat with pointer strings.

Duplicate of [Why is this string reversal C code causing a segmentation fault?](http://stackoverflow.com/questions/1614723/why-is-this-string-reversal-c-code-causing-a-segmentation-fault) and large number of other questions suffering from the same error. — AnT stands with Russia, Sep 16 '13 at 01:23

score 7 · Answer 1 · answered Sep 16 '13 at 01:17

There are multiple rules of strcat you are breaking:

The first parameter shouldn't be a string literal, because it's non-modifiable.
The first parameter must be large enough for the concatenated string.
The first and second parameter shouldn't overlap.

score 4 · Answer 2 · answered Sep 16 '13 at 01:17

The docs say strcat

Appends a copy of the source string to the destination string. The terminating null character in destination is overwritten by the first character of source, and a null-character is included at the end of the new string formed by the concatenation of both in destination.\

You're passing a constant string "foo" to strcat as the destination buffer. Trying to overwrite constant string is bad. Even if it weren't constant, you're writing 7 characters over 4 (including the terminating nulls). This is also bad.

You probably want this:

char *two(char *result, char *str)
{
    return strcat(strcpy(result, str), str);
}

And when you call it,

int main() {
    char buf[40];
    printf("The value of two(\"foo\") is %s", two(buf, "foo"));
}

+1 This is clearly an excellent answer if you wish to pass the destination buffer into the function. — dcaswell, Sep 16 '13 at 01:25

score 2 · Accepted Answer · edited Sep 04 '16 at 09:27

2

You didn't allocate space to hold two copies of your input string.

strcpy copies strings.

strcat appends one string onto another.

malloc allocats bytes on the heap that should be free() later.

char *two(char *foo);

int main() {
    char * ptr =  two("foo");
    printf("The value of two(\"foo\") is %s", ptr);
    free(ptr);
}

char *two(char *foo) {
    char * ptr = malloc(strlen(foo) * 2 + 1);
    strcpy(ptr, foo);
    strcat(ptr, foo);
    return ptr;
}

edited Sep 04 '16 at 09:27

Ahsanul Haque

10,676
4
41
57

answered Sep 16 '13 at 01:14

dcaswell

3,137
2
26
25

Exactly right, I indicated it needed to be freed and I upvoted the answer below that passes in a buffer. – dcaswell Sep 17 '13 at 22:17
1

Yes, you indicated that it needs to be freed, but you didn't reflect that in your example. (Note: nobody ever gets to see _who_ upvoted anything.) – This isn't my real name Sep 17 '13 at 22:19

score 0 · Answer 4 · answered Sep 16 '13 at 01:12

It seems you are misunderstanding strcat. Have a look at this link for a brief explanation of what it does. In particular the man page says that

The strings may not overlap, and the dest string must have enough space for the result

You are passing it the same address for both parameters, which means that the strings overlap. Also, I can't see how you are allocating the string so I can't tell if there is enough space in the buffer / array for the resulting string.

C function using strcat with strings

4 Answers4