strcat_s after strcpy_s gives Error

Question

I am trying to concatenate two strings in C++ using VS 2013. Below is the code:

char *stringOne = "Hello";
char *stringTwo = " There!"; 
char *hello = new char[strlen(stringOne) + strlen(stringTwo) + 1];
strcpy_s(hello, strlen(hello), stringOne);
//hello[strlen(stringOne)+1] = '\0';
strcat_s(hello, strlen(hello), stringTwo);//<-----Does not return from this call

If the strcat_s statement is commented,it runs fine and *hello contains "Hello".

But with it, VS says the application has triggered a breakpoint after showing:

Expression: (L"String is not null terminated" && 0)

It doesn't work anyway I've tried. The closest existing question I found is here. As prescribed, manually setting the last character as null doesn't help either.

Any ideas anyone?

`strlen(hello)` is UB because `hello` contains unitialized memory. besides you dont need to call strlen you know exactly what the size is: `strlen(stringOne) + strlen(stringTwo) + 1` — Borgleader, Apr 04 '18 at 11:23
first, I suggest that you change `stringOne` and `stringTwo` from `char *` to `const char*`, if you want to avoid pointer-related bugs — SomethingSomething, Apr 04 '18 at 11:23
This type of string management is somewhat complicated and tiring. Consider using `std::string` which essentially does this and more and skip having to re-implement the annoying little details repeatedly. — nwp, Apr 04 '18 at 11:30
secondly, in `strcpy()`, you use `strlen(hello)` before any string is assigned to `hello`. Use `strlen(stringOne)` instead — SomethingSomething, Apr 04 '18 at 11:30
It's also somewhat amusing that it is an attempt to use the "safe" interface of the `_s` functions that triggers the error. Plain old `strcpy(hello, stringOne);` would have worked fine. — Bo Persson, Apr 04 '18 at 11:37
Thanx for all the response! The issue was same as you pointed out, calling strlen(hello) without initializing it. — Prabal Kajla, Apr 04 '18 at 11:38
@BoPersson or they could have followed C++ idiom and just used `std::string` which doesn't have any of these issues and is much more portable. — Mgetz, Apr 04 '18 at 11:49
@Mgetz - Sure, that is a much better idea. I'm just giggling over the fact that when you use `strpcy` the MS compiler claims that it is deprecated (not true) and that `strcpy_s` works much better. Turns out that it didn't - it is *harder* to use. — Bo Persson, Apr 04 '18 at 11:56
@BoPersson I disagree, if this had been approached in a C manner I think it would have been very clear because return codes would have been checked. — Mgetz, Apr 04 '18 at 12:12

score 1 · Accepted Answer · answered Apr 04 '18 at 11:26

1

strlen(hello) is the wrong string length, respectively it's complete garbage at that time since hello isn't even initialized yet.

The expression strlen(stringOne) + strlen(stringTwo) + 1 which you had used to allocate the target buffer would had been the appropriate length to pass.

Also better get used to checking the return values of the _s functions, because then you would have known that already the first function call had failed.

answered Apr 04 '18 at 11:26

Ext3h

5,713
17
43

There's no guarantee of getting a meaningful error code as a return value after invoking undefined behavior like here. – interjay Apr 04 '18 at 11:31
@Ext3h : That did it, changing strlen(hello) to the size expression. Thanx a ton! – Prabal Kajla Apr 04 '18 at 11:36
I am still curious though, why did strcpy_s alone gave expected results. – Prabal Kajla Apr 04 '18 at 11:40
1

@PrabalKajla you shouldn't expect any given result when you invoke undefined behavior. from [here](http://en.cppreference.com/w/cpp/string/byte/strlen) - _The behavior is undefined if there is no null character in the character array pointed to by str_. – default Apr 04 '18 at 13:15

strcat_s after strcpy_s gives Error

1 Answers1