Using strcpy_s to copy string to char*

Question

I know that when you use strcpy_s you are supposed to supply the size of the destination string as the second parameter. But if the destination string is a char* then I'm not sure I'm doing it right. I have three examples:

char* dest = new char;

// Example 1
CString strTemp = "Bob";
strcpy_s(dest, strTemp.GetLength() + 1, strTemp);   

// Example 2
strcpy_s(dest, strlen("Jose")+1, "Jose");

// Example 3 
char* c = new char;
c = "Richard";
strcpy_s(dest, strlen(c) + 1,c);        

Is this all correct?

Answer 1

In all three examples you're passing the size of the source string. You're supposed to pass the size of the destination buffer so strcpy_s can verify that there won't be a buffer overflow.

Passing the source string's size defeats that check. strcpy_s already knows the size of the source string; it can do strlen(src) just as well as you can. What it can't do is auto-detect the size of the destination buffer. It needs to be told its size.

const int SIZE = 2048; // an arbitrary large number
char* dest = new char[SIZE];

std::errno_t result = strcpy_s(dest, SIZE, src);   

if (result != 0) {
    // error
}

All of your examples should instead look something like the above:

• dest points not at a single char, but a large array of chars.
• The second parameter is the destination buffer's size.
• Make sure to check the return code for errors.

Note: Avoid using C strings in C++ as much as possible. It is far, far better to use std::string. It'll take care of all of this mess for you. You won't have to deal with memory allocation or worry about buffer overflows.

Answer 2

Is this all correct?

None of the examples are correct.

char* dest = new char;

You've allocated a single char. The only string that it can represent is the empty string.

char* c = new char;
c = "Richard";

By assigning c to point elsewhere, you've lost the pointer value that was returned by new. As a consequence, you can no longer pass that value to delete. This is called a memory leak.

Furthermore, since C++11 this is ill-formed since string literals are no longer convertible into pointer to non-const char. Being ill-formed means that a compiler is not required to compile your program, and instead it is required to issue you a diagnostic message informing you about the ill-formedness.

strcpy_s(dest, strlen(c) + 1,c);

You're supposed to pass the size of the destination buffer; not the size of the source data. In this case the destination buffer is too small, but because you passed the wrong value, the error is not caught and the behaviour is undefined.

As I mentioned, pass the size of the destination buffer:

auto error = strcpy_s(dest, 1, "Richard");

This safely results in an error instead of undefined behaviour - or it might safely abort the program or do something else depending on implementation. You can control the constrain handler to have your desired behaviour.

Of course, you might want to allocate sufficient memory so that the copy works:

std::size_t destsz = 1024;
char* dest = new char[destsz];

You know the size that you allocated. Simply pass that to strcpy_s:

auto error = strcpy_s(dest, destsz, "Richard");

Don't forget to clean up:

delete[] dest;

---

P.S. The C++ standard library doesn't provide strcpy_s. It is a non-standard function. It is standard in C language only (but it is optional for an implementation to provide it).

P.S.S Don't use new to allocate strings. Use std::string in C++. With std::string you can copy like this:

std::string c = "Richard";
std::string dest = c;

It will be much harder to leak memory or make more serious bugs with this approach.

Answer 3

Note that you should allocate the right size for the destination, hence replace

char* dest = new char; which allocates one character by

char* dest = new char[strlen(strTemp)+1];.

And note that cString is not a type, hence replace CString strTemp = "Bob"; by char* strTemp = "Bob"; or const char* strTemp = "Bob"; (the last one is equivalent to auto strTemp = "Bob";).

Note that c style strings are not objects so strTemp.GetLength() is invalid. Replace it by strlen(strTemp) and include the header <cstring> which includes that functionstrlen().

Finally your code should be like the following one

#include<iostream>
#include<cstring>
using namespace std;
int main(){

    auto strTemp = "Bob";
    char* dest = new char[strlen(strTemp)+1];
    strcpy_s(dest, strlen(strTemp) + 1, strTemp);   
    cout<<dest;
}

Answer 4

If you allocate the memory before copying a string into it, using a string-copying function shows muddled thinking leading to blatant waste: Just use memcpy() (or std:copy_n()).

strcpy_s() needs pointer to destination, destination buffer size, and pointer to source.
If you guess the buffer size as the one needed as derived from the source, instead of knowing it, use strcpy() instead of that convoluted way to (hopefully) write the same less efficiently.

In conclusion, all your uses are wrong.

As an aside:
new char allocates a solitary char, not an array of appropriate size. Use new char [n] instead.
Do you really need to copy the string, or could you actually just pass a pointer?
The process-wide constraint-handler need not do what you want. Or actually anything useful at all.