Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warnings when compiling with VisualStudio 2015 #723

Closed
ChipBurwell opened this issue Sep 3, 2017 · 19 comments
Closed

Warnings when compiling with VisualStudio 2015 #723

ChipBurwell opened this issue Sep 3, 2017 · 19 comments

Comments

@ChipBurwell
Copy link

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_jsonstd::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_jsonstd::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_jsonstd::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_jsonstd::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_jsonstd::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_jsonstd::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_jsonstd::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_constructiblenlohmann::json' being compiled

@nlohmann nlohmann added the platform: visual studio related to MSVC label Sep 3, 2017
@nlohmann
Copy link
Owner

nlohmann commented Sep 3, 2017

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

@nlohmann nlohmann added confirmed state: help needed the issue needs help to proceed labels Sep 3, 2017
@crea7or
Copy link

crea7or commented Sep 30, 2017

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
Copy link

crea7or commented Oct 1, 2017

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

@nlohmann
Copy link
Owner

nlohmann commented Oct 2, 2017

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

@nlohmann
Copy link
Owner

nlohmann commented Oct 2, 2017

@crea7or Ignore my previous comment #723 (comment).

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

@nlohmann nlohmann closed this as completed Oct 2, 2017
@nlohmann nlohmann removed the state: help needed the issue needs help to proceed label Oct 2, 2017
@ChipBurwell
Copy link
Author

ChipBurwell commented Oct 2, 2017 via email

@nlohmann nlohmann reopened this Oct 2, 2017
@nlohmann
Copy link
Owner

nlohmann commented Oct 2, 2017

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

@crea7or
Copy link

crea7or commented Oct 2, 2017

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
Copy link
Author

ChipBurwell commented Oct 2, 2017 via email

@gregmarr
Copy link
Contributor

gregmarr commented Oct 2, 2017

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
Copy link
Author

ChipBurwell commented Oct 2, 2017 via email

@gregmarr
Copy link
Contributor

gregmarr commented Oct 2, 2017

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
Copy link
Contributor

gregmarr commented Oct 2, 2017

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
Copy link
Owner

nlohmann commented Oct 2, 2017

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

@crea7or
Copy link

crea7or commented Oct 2, 2017

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

@ChipBurwell
Copy link
Author

ChipBurwell commented Oct 2, 2017 via email

@gregmarr
Copy link
Contributor

gregmarr commented Oct 2, 2017

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

@nlohmann
Copy link
Owner

nlohmann commented Oct 3, 2017

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

@nlohmann nlohmann closed this as completed Oct 3, 2017
@gregmarr
Copy link
Contributor

gregmarr commented Oct 3, 2017

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants