ERROR: strcat command

[khanhdt6]

i wrote a short code about string but is wrong. I don't know why? please help me. here my code

int main(){

char *s1;
char *s2;

s1=(char*) "hello ";
s2=(char*) "everyone!";
strcat (s1,s2);

cout<<s1<<endl;

return 0;
}

[MiiNiPaa]

1) "hello " is a string literal. It is prohibited to change it.
2) Do not use casts. Most often cast == bad program style.
3) You need to make sure that destination has enough free space to append second c-string

[kbw]

@MiiNiPaa +1

The correct thing is make a local buffer large enough for your use, initialise it with "hello" and append the content while respecting the limited size of the buffer.

int main()
{
    char s1[20] = "hello ";  // create a buffer, 20 chars long, initialised with "hello "
    const char* s2 = "everyone!";  // s2 points to the string "everyone!" held in read-only memory somewhere

    strncat(s1, s2, sizeof(s1)); // append s2 to s1, but don't overrun the buffer s1
}

[MiiNiPaa]

And it still can overflow: third parameter is maximum amount of characters to copy, not buffer capacity.

Correct usage:

int leftover = sizeof(s1) - strlen(s1) - 1/*Null terminator*/;
strncat(s1, s2, leftover);

[kbw]

... in which case you have to put the trailing null in yourself.

[MiiNiPaa]

Actually string is guaranteed to be null-terminated after call to strncat. I had to decrease leftover size by 1, or else terminating 0 will overflow on long strings.

[kbw]

Actually string is guaranteed to be null-terminated after call to strncat.

You're right about that. I got it confused with strncpy that doesn't do that.

I had to decrease leftover size by 1, or else terminating 0 will overflow on long strings.

You don't need that because of the guarantee above, which makes my original example correct.

[MiiNiPaa]

But overflow can still happen:

1 2 3 4 5 6 7 8 9 10

#include <iostream> #include <cstring> int main() { char foo[5] = "Hi!"; const char* bar = "Overly long string"; strncat(foo, bar, sizeof(foo)); //Expected Hi!O\0 std::cout << foo; }

Hi!Overl

It copies 5 characters in buffer already containing 3 + 1, making 5 char buffer to contain 9 characters.

In comparison:

1 2 3 4 5 6 7 8 9 10 11

#include <iostream> #include <cstring> int main() { char foo[5] = "Hi!"; const char* bar = "Overly long string"; int leftover = sizeof(foo) - strlen(foo) - 1/*Null terminator*/; strncat(foo, bar, leftover); std::cout << foo; }

Hi!O

[kbw]

Yes, you're right.