How to get rid of "unsafe" warnings / errors in Visual Studio (strcpy, sprintf, strdup)

Question

I'm trying to get rid of some compiler warnings that say strcpy, sprintf, etc are unsafe. I get why they're unsafe, but I can't think of a good way to fix the code, in a C++ style.

Here's a excerpt of the code:

extList->names[i]=(char *)malloc(length*sizeof(char));
strcpy(extList->names[i],extName);                     // unsafe
// strncpy(extList->names[i],extName,length);          // also unsafe

Here's the message:

C4996: 'strcpy': This function or variable may be unsafe. Consider using strcpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.

I can't think of a safe way to copy the data over in C++ without knowing the length of the stuff to copy. I know there's strlen(), but that's also unsafe since it assumes (maybe incorrectly) that the data is null-terminated.

Also:

// used to concatenate:
sprintf(extStr,"%s%s",platExtStr,glExtStr);

C4996: 'sprintf': This function or variable may be unsafe. Consider using sprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.

Using std::string to concatenate is easy enough, but then I need to get the data into extStr somehow (and not using strcpy, lol). The string::c_str() function returns a pointer to un-modifiable data, so I can't just set extStr equal to it. (And I'm not even sure if the c_str() pointer needs delete called on it later? Does it allocate space using "new"?)

Any advice on this stuff? This is part of a 10,000 line file that's not mine... so I'm not exactly keen on re-writing the thing in the C++ way.

Answer 1

You don't really need pragmas to disable them.

For win32/msvc, in ProjectProperties -> Configuration Properties -> C/C++ -> Preprocessor -> Preprocessor Definitions, add following macros:

_CRT_SECURE_NO_DEPRECATE  
_CRT_NONSTDC_NO_DEPRECATE

Or you can pass thos in command line parameters (-D_CRT_SECURE_NO_DEPRECATE). You can probably #define them at the beginning of certain *.cpp files. Also, there are probably more of them (see crtdefs.h - looks like there are a lot of them...). Those kind of warnings normally tell you with which macros you can disable them - just read compiler output.

Answer 2

Here is another answer to this question.

#ifdef _MSC_VER
#pragma warning(push)
#pragma warning(disable : 4996)
#endif

        strcpy(destination, source);

#ifdef _MSC_VER
#pragma warning(pop)
#endif

Answer 3

If getting rid of warnings only is your objective... simply define this _CRT_SECURE_NO_WARNINGS and it will suppress all the deprecation warnings. But this will not fix the underlying problems with unsafe CRT functions.

If you are on visual studio version >= 2005 and want to fix these warnings in a proper way... easiest method is to #define _CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES 1 and #define _CRT_SECURE_CPP_OVERLOAD_STANDARD_NAMES_COUNT 1 in your project.

without any further code changes you can observe most of the warnings are fixed automatically. By defining this windows will automatically call the secure overloaded functions for most of the unsafe CRT functions. Buffer sizes for static arrays are calculated automatically.

Although Dynamically allocated buffers are not fixed by this way and we need to fix them manually. Please refer this link for further details.

Below is a way to correct your example programatically

strcpy_s(extList->names[i], length, extName); 

Answer 4

You do know how much to copy - you allocated space for it!

Surely you wouldn't willingly copy more than the space you allocated?

I would prefer to use a method that explicitly avoids buffer overruns by limiting the number of items copied. Back when I was a C programmer we used

dest = malloc(len);         // note: where did we get len?
if ( dest is null )  panic!  // note: malloc can fail
strncpy(dest, src, len);
dest[len-1] =0;

This is slightly messy, and has been pointed out is using strncpy() a method which really was originally designed for fixed-width fields rather than strings. However it does ach

There are methods such as strdup() and strlcpy() which may we help.

My recommendations:

1). Your target should not be to suppress warnings but to make the code robust.

2). When copying strings you need to ensure these things:

• Protect yourself from bad input, for example an unterminated or excessively long string.
• Protect yourself from malloc failures,
• Strongly prefer copies of counted numbers of characters to copying until we see a null
• If you claim to build a string, then make abolsutely sure you null terminate it

If strlcpy() is available in your environment then you could use it, otherwise why not write your own little utilityy function? Then if there are warnings in just that function you've localised then problem.

Answer 5

In your first example, you know the length already. Since you aren't allocating length+1 bytes I'll assume that length INCLUDES the null terminator. In that case, just std::copy the string: std::copy(extName, extName + length, expList->names[i]);

In your second example assuming the source strings are null terminated you could compute the destination string length and use std::copy again to concatenate manually, or you could use std::string and the std::copy from the results of c_str into your destination (Again assuming you allocated enough space for it).

c_str() does not allocate memory that would require external deletion.

Finally note that sizeof(char) will always be one and so is redundant in your malloc, although the number of bits in that character may not be 8 (See CHAR_BIT).

Answer 6

I think that you should replace all the function-calls if possible to call an implementation of your own. A good example here would be a function to replace strcpy and call the compiler-specific version of strcpy inside it. Your implementation can then easily be modified to suit any compiler of your choice, specifically if you will be adding or changing platforms/compilers.

Example:

char* StringCopy(char* Destination, const char* Source, size_t DestinationSize)
{
#ifdef _MSC_VER
  return strcpy_s(Destination, Source, DestinationSize);
#else
  if(!(strlen(Source) >= DestinationSize))
     return strcpy(Destination, Source);
  else
     return 0x0;
#endif
}

Answer 7

If portability is not a concern, you can use 'strcpy_s'.

Answer 8

If this code is only to be compiled for windows platform then it is better to use the secured version of these functions. However, if this code is going to compiled across multiple platforms(linux, Aix etc) then either you can disable the warning in your windows project configuration file (for example, .vcxproj) by using _CRT_SECURE_NO_WARNINGS or, you can use a code snippet like this one in places where those functions have been called in the .cpp file.

#if _OS_ == _OS__WINDOWS
//secure function call
#else
//already written code
#endif

Answer 9

as it is suggested in the message, use _CRT_SECURE_NO_WARNINGS to disable this warnings.

in ProjectProperties -> Configuration Properties -> C/C++ -> Preprocessor -> Preprocessor Definitions, add following macros:

_CRT_SECURE_NO_WARNINGS