25

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.

6
  • 4
    You should probably mention what OS, programming environment and compiler you are using.
    – Paul R
    Jul 24, 2010 at 21:35
  • 1
    And if it's win32/visual studio - the compiler warnings even tell you how to turn them off.
    – nos
    Jul 24, 2010 at 21:47
  • 1
    10,000 lines of code in one file? I'd suggest refactoring, then worry about cleaning up warnings.
    – Sam Miller
    Jul 24, 2010 at 22:18
  • 6
    @Sam Miller: "I'd suggest refactoring" While refactoring is good in theory, there may be time limits on the project. Also when you modify a code, there will be always a chance that you'll introduce a bug. Programmer's goal is to do minimal amount of work, fix maximum number of problems and get max payment. Refactoring doesn't qualify. I worked on a few large horribly written projects. After I rewrote 25% of each, I decided that it isn't worth it unless It increases my payment. Refactoring may introduce new bugs, doesn't fix existing problems, and only makes everything "look" nicer.
    – SigTerm
    Jul 25, 2010 at 11:39
  • 1
    @J. Chomel I'd argue my edit fits in to the OP's original intent because VS2010 was the newest version at the time, and his "goal" was to get past this warning (now an error), is there value in keeping this question specific to VS 2010, when it's a simple edit to make it useful for VS 2010 and VS2013+? If not, do you think that the difference between VS2010 and VS2013+ is worth a new question?
    – jrh
    Oct 12, 2016 at 12:53

9 Answers 9

40

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.

2
  • This is exactly what you need. The first one will normally get rid of all the *_s function suggestions.
    – rubenvb
    Jul 25, 2010 at 10:43
  • 2
    _CRT_NO_MICROSOFT_JEDI_MIND_CONTROL would be a handy setting.
    – Bob Stein
    Apr 3, 2019 at 14:18
10

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
3
  • +1 This works for me. The answer of V M Rakesh didn't work. Don't know why.
    – Diversity
    Oct 21, 2014 at 17:04
  • It probably won't work if you're using precompiled headers as the header may already have been included without the macros set. Sep 15, 2015 at 10:28
  • You can always put it in, as one of the first lines, in stdafx.h before including any other file. One word of warning though: Check all the C4996 warning lines that the complier gives you before you disable all of it. This warning is not given for nothing.
    – PapaAtHome
    Feb 13, 2016 at 13:41
8

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); 
5

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.

2
  • 3
    strncpy is not intended for use with C strings. strlcpy or memcpy can be used, depending on the circumstances. Jul 24, 2010 at 21:38
  • Yeah, strncpy is also unsafe. Is strlcpy platform-independent? I know Microsoft has sprintf_s that won't work with other compilers.
    – Andre
    Jul 24, 2010 at 22:30
4

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).

1
  • 1
    Using std::copy also gives me a warning about it being unsafe because it requires the user to check the size :(
    – Andre
    Jul 24, 2010 at 23:19
2

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
}
6
  • Sorry about the !(strlen(Source) >= DestinationSize). I couldn't get the "<" working.
    – Simon
    Jul 24, 2010 at 22:07
  • 4
    -1. Standard implementation doesn't have unknown bugs, but your new implementation most likely will have them. You'll be reinventing the wheel.
    – SigTerm
    Jul 24, 2010 at 22:10
  • I don't recommend rewriting them from scratch. Unfortunately there are different implementations of several standard implementations depending on which platform/compiler you use. I suggest using the existing functions, although making sure that you get what you want from the functions is nothing negative. A good example is vsnprintf which does not null-terminate a trunkated string on MSVC, although GCC does.
    – Simon
    Jul 24, 2010 at 22:21
  • I see now that in my text I does not clearly express that you should use existing functions as much as possible although you should wrap it in a simplified API. My example still shows what I really meant though.
    – Simon
    Jul 24, 2010 at 22:24
  • 2
    +1, excellent idea. Ought to crash quicker though in non-MSVC compilers. Jul 25, 2010 at 0:10
2

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

0

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

-1

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

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.