nlohmann / json

JSON for Modern C++
https://json.nlohmann.me
MIT License
43.21k stars 6.74k forks source link

Warnings when compiling with VisualStudio 2015 #723

Closed ChipBurwell closed 7 years ago

ChipBurwell commented 7 years ago

I've update to version 2.1.1 (from previously using 2.0.10) and now I'm getting the warnings listed below. I don't think any of them are critical, but I'm wondering if there is a simple way to fix this without turning off warnings that I'd like to have on?

1>json.hpp(6620): warning C4996: '_snprintf': This function or variable may be unsafe. Consider using _snprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. 1> c:\program files (x86)\windows kits\10\include\10.0.10240.0\ucrt\stdio.h(1952): note: see declaration of '_snprintf' 1> json.hpp(6594): note: while compiling class template member function 'void nlohmann::detail::serializer<nlohmann::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::adl_serializer>>::dump_float(double)' 1> json.hpp(6203): note: see reference to function template instantiation 'void nlohmann::detail::serializer<nlohmann::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::adl_serializer>>::dump_float(double)' being compiled 1> json.hpp(8609): note: see reference to class template instantiation 'nlohmann::detail::serializer<nlohmann::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::adl_serializer>>' being compiled 1> json.hpp(8607): note: while compiling class template member function 'std::basic_string<char,std::char_traits,std::allocator> nlohmann::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::adl_serializer>::dump(const int,const char,const bool) const' 1> json.hpp(13523): note: see reference to function template instantiation 'std::basic_string<char,std::char_traits,std::allocator> nlohmann::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::adl_serializer>::dump(const int,const char,const bool) const' being compiled 1> c:\program files (x86)\microsoft visual studio 14.0\vc\include\type_traits(639): note: see reference to class template instantiation 'nlohmann::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::adl_serializer>' being compiled 1> c:\program files (x86)\microsoft visual studio 14.0\vc\include\type_traits(668): note: see reference to class template instantiation 'std::is_nothrow_constructible<_Ty,nlohmann::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::adl_serializer> &&>' being compiled 1> with 1> [ 1> _Ty=nlohmann::json 1> ] 1> json.hpp(13504): note: see reference to class template instantiation 'std::is_nothrow_move_constructible' being compiled

nlohmann commented 7 years ago

I can confirm the warnings, but I am not MSVC expert, so I cannot decide whether they are critical. If someone has an idea - PRs are welcome :)

crea7or commented 7 years ago

Regarding '_snprintf' and _CRT_SECURE_NO_WARNINGS: Microsoft have special, modified string functions where strings sizes are passed as additional parameters:

int snprintf( char buffer, size_t count, const char format [, argument] ...);
vs int _snprintf_s( char buffer, size_t sizeOfBuffer, size_t count, const char format [, argument] ... );

they are considered as more secure, so this is the reason for this warning. warning can be suppressed for msvc compilers via:

define _CRT_SECURE_NO_WARNINGS 1

or

pragma warning(disable:4996)

crea7or commented 7 years ago

However I can't see this warning(warning C4996) in the current release and dev branches right now (vs 2015 and vs 2017).

nlohmann commented 7 years ago

I won't make another 2.x.x release - are there similar warnings in the develop branch?

nlohmann commented 7 years ago

@crea7or Ignore my previous comment https://github.com/nlohmann/json/issues/723#issuecomment-333583839.

If the warning disappeared in the develop branch, I can close this issue.

ChipBurwell commented 7 years ago

I just did a get of the latest develop branch and I can confirm this warning is still happening. I don't think this issue should have been closed.

Chip

On 10/2/2017 9:22 AM, Niels Lohmann wrote:

@crea7or https://github.com/crea7or Ignore my previous comment #723 (comment) https://github.com/nlohmann/json/issues/723#issuecomment-333583839.

If the warning disappeared in the |develop| branch, I can close this issue.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nlohmann/json/issues/723#issuecomment-333586157, or mute the thread https://github.com/notifications/unsubscribe-auth/AEoQadF9R52CIL9e945OH2zaqfBdK-W2ks5soQ3KgaJpZM4PLEDK.

nlohmann commented 7 years ago

Can you please paste the complete warning? Which compiler version are you using?

crea7or commented 7 years ago

There is nothing related to sprintf on a line 6620 in dev or master branches. Other errors does not match the code in branches too.

ChipBurwell commented 7 years ago

These are the warnings I'm getting with code pulled this morning from the develop branch:

5>E:\TruGolf\e6_2\head\Fexy/Utils/json.hpp(6597): warning C4996: '_snprintf': This function or variable may be unsafe. Consider using _snprintf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. 5>  C:\Program Files (x86)\Windows Kits\10\Include\10.0.10240.0\ucrt\stdio.h(1952): note: see declaration of '_snprintf' 5>  E:\TruGolf\e6_2\head\Fexy/Utils/json.hpp(6571): note: while compiling class template member function 'void nlohmann::detail::serializer<nlohmann::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::adl_serializer>>::dump_float(double)' 5>  E:\TruGolf\e6_2\head\Fexy/Utils/json.hpp(6180): note: see reference to function template instantiation 'void nlohmann::detail::serializer<nlohmann::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::adl_serializer>>::dump_float(double)' being compiled 5>  E:\TruGolf\e6_2\head\Fexy/Utils/json.hpp(8587): note: see reference to class template instantiation 'nlohmann::detail::serializer<nlohmann::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::adl_serializer>>' being compiled 5>  E:\TruGolf\e6_2\head\Fexy/Utils/json.hpp(8585): note: while compiling class template member function 'std::basic_string<char,std::char_traits,std::allocator> nlohmann::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::adl_serializer>::dump(const int,const char,const bool) const' 5>  E:\TruGolf\e6_2\head\Fexy/Utils/json.hpp(13512): note: see reference to function template instantiation 'std::basic_string<char,std::char_traits,std::allocator> nlohmann::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::adl_serializer>::dump(const int,const char,const bool) const' being compiled 5>  C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\type_traits(639): note: see reference to class template instantiation 'nlohmann::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::adl_serializer>' being compiled 5>  C:\Program Files (x86)\Microsoft Visual Studio 14.0\VC\include\type_traits(668): note: see reference to class template instantiation 'std::is_nothrow_constructible<_Ty,nlohmann::basic_json<std::map,std::vector,std::string,bool,int64_t,uint64_t,double,std::allocator,nlohmann::adl_serializer> &&>' being compiled 5>          with 5>          [ 5>              _Ty=nlohmann::json 5>          ]

I'm using Visual Studio VC2015 which uses the VS 140 compilers I believe.

I would suggest fixing this by replacing line 6597 which currently is:

         std::ptrdiff_t len = snprintf(number_buffer.data(), number_buffer.size(), "%.*g", d, x);

with

ifdef _MSC_VER

        std::ptrdiff_t len = _snprintf_s( number_buffer.data(), number_buffer.size(), _TRUNCATE, "%.*g", d, x );

else

        std::ptrdiff_t len = snprintf( number_buffer.data(), number_buffer.size(), "%.*g", d, x );

endif

Chip

On 10/2/2017 9:52 AM, Niels Lohmann wrote:

Can you please paste the complete warning? Which compiler version are you using?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nlohmann/json/issues/723#issuecomment-333594640, or mute the thread https://github.com/notifications/unsubscribe-auth/AEoQadR4wK8raL5SibG-qpc3qFFHBSAJks5soRTXgaJpZM4PLEDK.

gregmarr commented 7 years ago

The current code is exactly the same as the proposed code. MSVC should not be warning here. It would be ugly to "fix" this by introducing this non-standard code that does exactly the same thing as the standard code.

ChipBurwell commented 7 years ago

If you don't want to use _snprintf_s and you feel snprintf is not a security hazard and that MSVC should not be generating the warning would you like the following fix better?

ifdef _MSC_VER

pragma warning(push)

pragma warning(disable : 4996)

endif

        // the actual conversion         std::ptrdiff_t len = snprintf( number_buffer.data(), number_buffer.size(), "%.*g", d, x );

ifdef _MSC_VER

pragma warning(pop)

endif

And lastly, I'm not too sure about this, but would changing snprint to  use std::to_string instead be a good solution???

Chip

gregmarr commented 7 years ago

No, I wouldn't like that either. I would like Microsoft to fix their incorrect warning.

snprintf is not a security hazard when it is implemented according to the standard.

The behavior of _snprintf_s(data, len, _TRUNCATE, ... is EXACTLY the same as snprintf(data, len, ....

It did not used to be that way, but Microsoft fixed their snprintf function in VS 2015 to no longer be a security hazard.

Changing to std::to_string is not correct either, as it would lose precision.

Note that I'm just a user, @nlohmann will have to make that decision.

gregmarr commented 7 years ago

Aha!

5>E:\TruGolf\e6_2\head\Fexy/Utils/json.hpp(6597): warning C4996:
'_snprintf': This function or variable may be unsafe. Consider using
_snprintf_s instead. To disable deprecation, use

This is _snprintf, not snprintf.

You probably have this somewhere in your code

#define snprintf _snprintf
nlohmann commented 7 years ago

I agree with @gregmarr. Changing the library code to silence MSVC makes no sense here.

crea7or commented 7 years ago

Visual Studio 2015 with compiler v140 and SDK 10240(as yours) have no such warning. Do you use VST3 SDK?

ChipBurwell commented 7 years ago

Well, it took a bit of digging but I can confirm that Adobe Scaleform header file AS2_Action.h which we include in our project (because we are using Adobe Scaleform) does in fact include the line

define snprintf _snprintf

Congrats to Greg for leading me to that.

Unfortunately, because I can't imagine for one minute that Adobe will do anything about this, I guess that leaves me with the choice of altering json.hpp or ignoring the warning. Neither of which I like. But I suppose that's my problem and not for any of you. (Ah the joys of making various packages work together.)

Thanks and feel free to close the bug.

Chip

gregmarr commented 7 years ago

You should be able to either include json.hpp before AS2_Action.h or put #undef snprintf between the two headers.

nlohmann commented 7 years ago

Thanks for checking back! And I am impressed by @gregmarr to have found this subtle difference!

gregmarr commented 7 years ago

It's something that I've done in the past, and was reminded of it when I saw a mention of it in some forum somewhere while looking for documentation on the snprintf change in VS 2015.