C4996 error and warning with Visual Studio

Using latest commit, C4996 errors and warnings spew on every string manipulation function calls:

std::strcpy (Warning)
std::strcat (Warning)
std::copy (Error - compile fail) on non-iterator/pointer destination

Error C4996 'std::copy::_Unchecked_iterators::_Deprecate': Call to 'std::copy' with parameters that may be unsafe - this call relies on the caller to check that the passed values are correct. To disable this warning, use -D_SCL_SECURE_NO_WARNINGS. See documentation on how to use Visual C++ 'Checked Iterators'

changing std::copy(m_start, m_end, buf.data()); to std::copy(m_start, m_end, buf.begin()); fix this problem.

warning C4996: 'strcat': This function or variable may be unsafe. Consider using strcat_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS.

I think turning off the warning like proposed on #453 before might be enough but probably not a clean solution.

Thanks for reporting! I had some headaches taking over the code with strcat/strlen, but it seemed more elegant compared to a manual search for the \0 byte. However, this manual search can be done in a for loop in this setting, so it shall be safe to replace the array<char, N> by a std::vector<char> or std::string with respective reservation of N bytes.

Since dealing with string here, In my opinion the std::string with reservation of N bytes approach is a better choice. There will be no need to call strcat/strcpy or do it manually since we got + operator and = operator. The strlen you mentioned isn't affected and still can be used, but switching to .length() is recommended.

There is only one usage of strlen on array data I found in latest commit

const auto data_end = m_buf.begin() + strlen(m_buf.data());

const bool value_is_int_like =
      std::none_of(m_buf.begin(), data_end, [](const char c)
      {
           return (c == '.' or c == 'e' or c == 'E');
      });

which then can be switched to .cbegin(), .cend() instead.

The rationale for using std::array<char, N> instead of a std::string in this context was to avoid heap allocations, which is the performance bottleneck.

Well both std::string and std::vector does heap allocation. So for performance critical, I think ignoring the warning worth it, since we control the input data ".", "-0.0" and "0.0".

Another solution for the warnings

#if defined(_MSC_VER)
#define JSON_STRCPY strcpy_s
#define JSON_STRCAT strcat_s
#define JSON_STROUT(out, buflen) out, buflen
#else
#define JSON_STRCPY std::strcpy
#define JSON_STRCAT std::strcat
#define JSON_STROUT(out, buflen) out
#endif

Then call like this JSON_STRCAT(JSON_STROUT(m_buf.data(), m_buf.size()), ".0");

and finally

#undef JSON_STRCAT
#undef JSON_STRCPY
#undef JSON_STROUT

What do you think?

🔨 refactored code to avoid using strcpy/strlen/strcat (#463)

I did some refactoring to avoid the unsafe calls, see 83a9c60. I combined a call to strlen and a std::none_of to a single for loop and unrolled stdcpy/ strcat to a sequence of assignments. What do you think?

@prsyahmi

@prsyahmi Could you please try with the most recent version?

The code looks good and it compiles fine here, no more warning and error 👍 I guess this can be closed now. Thanks!

Navigation Menu

Repository navigation

C4996 error and warning with Visual Studio #463

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

C4996 error and warning with Visual Studio #463

Description

Activity

nlohmann commented on Feb 20, 2017

prsyahmi commented on Feb 20, 2017

TurpentineDistillery commented on Feb 20, 2017

prsyahmi commented on Feb 20, 2017

prsyahmi commented on Feb 20, 2017

nlohmann commented on Feb 20, 2017

nlohmann commented on Feb 20, 2017

prsyahmi commented on Feb 20, 2017

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions