issue with using std::copy

Question

I am getting warning when using the std copy function.

I have a byte array that I declare.

byte *tstArray = new tstArray[length];

Then I have a couple other byte arrays that are declared and initialized with some hex values that i would like to use depending on some initial user input.

I have a series of if statements that I use to basically parse out the original input, and based on some string, I choose which byte array to use and in doing so copy the results to the original tstArray.

For example:

if(substr1 == "15")
{
   std::cout<<"Using byte array rated 15"<<std::endl;
   std::copy(ratedArray15,ratedArray15+length,tstArray);
} 

The warning i get is warning C4996: 'std::copy': Function call with parameters that may be unsafe - this call relies on the caller to check that the passed values are correct.

A possible solution is to to disable this warning is by useing -D_SCL_SECURE_NO_WARNINGS, I think. Well, that is what I am researching.

But, I am not sure if this means that my code is really unsafe and I actually needed to do some checking?

Answer 1

Basically, what this warning tells you is that you have to be absolutely sure that tstArray points to an array that is large enough to hold "length" elements, as std::copy does not check that

Answer 2

C4996 means you're using a function that was marked as __declspec(deprecated). Probably using D_SCL_SECURE_NO_WARNINGS will just #ifdef out the deprecation. You could go read the header file to know for sure.

But the question is why is it deprecated? MSDN doesn't seem to say anything about it on the std::copy() page, but I may be looking at the wrong one. Typically this was done for all "unsafe string manipulation functions" during the great security push of XPSP2. Since you aren't passing the length of your destination buffer to std::copy, if you try to write too much data to it it will happily write past the end of the buffer.

To say whether or not your usage is unsafe would require us to review your entire code. Usually there is a safer version they recommend when they deprecate a function in this manner. You could just copy the strings in some other way. This article seems to go in depth. They seem to imply you should be using a std::checked_array_iterator instead of a regular OutputIterator.

Something like:

stdext::checked_array_iterator<char *> chkd_test_array(tstArray, length);
std::copy(ratedArray15, ratedArray15+length, chkd_test_array);

(If I understand your code right.)

Answer 3

Well, I assume Microsoft's unilateral deprecation of the stdlib also includes passing char* to std::copy. (They've messed with a whole range of functions actually.)

I suppose parts of it has some merit (fopen() touches global ERRNO, so it's not thread-safe) but other decisions do not seem very rational. (I'd say they took a too big swathe at the whole thing. There should be levels, such as non-threadsafe, non-checkable, etc)

I'd recommend reading the MS-doc on each function if you want to know the issues about each case though, it's pretty well documented why each function has that warning, and the cause is usually different in each case.

Answer 4

At least it seems that VC++ 2010 RC does not emit that warning at the default warning level.