11

I have a large project about static source code analysis, and everything compiles successfully, except for one thing. I have provided the error message in the title. The point that confuses me is that it gives an error message saying unsafe. I thought it should be just warning, not an error. By the way, I'm using Visual Studio 2012. Here is the part of the code where I get the error, in ctime. If someone can help me overcome this error, I would be glad.

void CppCheckExecutor::reportProgress(const std::string &filename, const char stage[], const std::size_t value)
{
     (void)filename;

     if (!time1)
         return;

     // Report progress messages every 10 seconds
     const std::time_t time2 = std::time(NULL);
     if (time2 >= (time1 + 10)) {
         time1 = time2;

         // current time in the format "Www Mmm dd hh:mm:ss yyyy"
         const std::string str(std::ctime(&time2));

         // format a progress message
         std::ostringstream ostr;
         ostr << "progress: "
              << stage
              << ' ' << value << '%';
         if (_settings->_verbose)
             ostr << " time=" << str.substr(11, 8);

         // Report progress message
         reportOut(ostr.str());
     }
}

5 Answers5

13

If you're sure no safety issues in your code, you can disable this by #pragma warning(disable : 4996).

SomeWittyUsername
  • 18,025
  • 3
  • 42
  • 85
11

If you look at the description of ctime you will note:

This function returns a pointer to static data and is not thread-safe. In addition, it modifies the static tm object which may be shared with gmtime and localtime. POSIX marks this function obsolete and recommends strftime instead.

The behavior may be undefined for the values of time_t that result in the string longer than 25 characters (e.g. year 10000)

... that's a lot of things to worry about.

On the other hand, if you look at strftime:

size_t strftime( char* str, size_t count, const char* format, tm* time );

Return value

number of bytes written into the character array pointed to by str not including the terminating '\0' on success. If count was reached before the entire string could be stored, ​0​ is returned and the contents are undefined.

All the parameters are explicit, so that you fully control the possible data races, and there is no risk of overflowing the buffer provided as well.

This is the C-way though, and C++ introduces the <chrono> in which a specific function std::put_time can also be used to output time to a stream:

#include <iostream>
#include <iomanip>
#include <ctime>
#include <chrono>

int main() {
    std::time_t const now_c = std::time();
    std::cout << "One day ago, the time was "
              << std::put_time(std::localtime(&now_c), "%F %T") << '\n';
}

which is even better since you no longer have to worry about the possible buffer overflow.

Matthieu M.
  • 287,565
  • 48
  • 449
  • 722
  • Thanks for chrono and put_time! –  Nov 25 '12 at 14:55
  • 2
    Note that `std::localtime` also returns a pointer to static data and may not be thread-safe, so this only fixes the possible buffer overflow. – interjay Jun 27 '13 at 15:08
  • @interjay: right, the C++ example only addresses the printing part; unfortunately I know of no conversion function from `time_t` to `struct tm` that is both Standard and thread-safe. – Matthieu M. Jun 27 '13 at 15:11
  • 5
    for VS 2017 constructor is a bit changed: std::time_t const now_c = std::time(NULL); – ingconti Aug 04 '17 at 11:30
  • this is error , and introduce new unsafe way,.,,,,, – Nicholas Jela Mar 06 '23 at 06:12
  • @NicholasJela: Unfortunately, I don't yet read minds, so you'll have to be a _tad_ more explicit about what you think is wrong with this answer for me to have any chance to improve it. – Matthieu M. Mar 06 '23 at 08:15
  • put_time is also unsafe, since the op need a way to avoid unsafe, so it might have better solution then this. However, I just find one, – Nicholas Jela Mar 07 '23 at 02:37
  • @NicholasJela: I don't see any warning about [`put_time`](https://en.cppreference.com/w/cpp/io/manip/put_time) here. What's unsafe about it? – Matthieu M. Mar 07 '23 at 08:17
5

Yes, it should be just warning, not an error. To get a simple warning instead of an error, disable SDL check in VS project (in Configuration Properties -> C/C++ -> General tab).

Javanna
  • 59
  • 1
  • 1
  • According to http://en.cppreference.com/w/c/chrono/ctime this function is obsolete. POSIX and the C standard recommend `strftime` instead. – lmiguelmh Dec 29 '15 at 14:42
4

std::ctime is not thread safe for two reasons:

  • It can modify a global object of type std::tm that is shared by multiple functions.
  • It modifies a global char array and returns a pointer to that array.

There is a potential for collisions if you have other threads that call std::gmtime, std::localtime, or std::ctime.

The best thing to do is to convert that call to std::ctime to a call to std::strftime. This is consistent with POSIX, which deems ctime to be obsolete and recommends usage of strftime in its stead.

David Hammen
  • 32,454
  • 9
  • 60
  • 108
1

vs 2017:

#include "stdafx.h"


#include <iostream>
#include <iomanip>
#include <ctime>
#include <chrono>

int main() {
    std::time_t const now_c = std::time(NULL);
    auto s = std::put_time(std::localtime(&now_c), "%F %T");
    std::cout << s << std::endl;
}

but you will receive anyway:

....cpp(31): warning C4996: 'localtime': This function or variable may be unsafe. Consider using localtime_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.

to prevent you can use:

errno_t err;
struct tm time_info;
time_t time_create = time(NULL);
localtime_s(&time_info, &time_create);
char timebuf[26];
err = asctime_s(timebuf, 26, &time_info);

plain C taken partially from MSDN... old way..

ingconti
  • 10,876
  • 3
  • 61
  • 48