srt
srt copied to clipboard
Any plans to eliminate the many compile warnings, in e.g. Windows build?
I use a custom VS2019 project with mostly default settings and the following warnings appear a lot:
C4101 - local variable never used C4244 - conversion from type1 to type2 possible loss of data C4267 - 'var' conversion from 'size_t' to 'type' possible loss of data
I also get the equivalent on other platforms, but maybe this is because I am not using the cmake system (I've not tried it)? I thought maybe the cmake system was downgrading some of these warnings but I couldn't find any reference to the above VS warnings in the repo, so assume not? Or maybe the overall warning level is lower or something?
Anyway, the trouble with lots of warnings is they hide possible non-trivial warnings, such as this one I get in a Debug x64 config when building core.cpp:
common.h (361): warning 4717: 'EventVariant::EventVariant<CPacket const*>': recursive on all control paths, function will cause runtime stack overflow.
As an update to this, I addressed the warnings I was getting via this PR, which you can take if you want.
https://github.com/Haivision/srt/pull/1657
This disables the warnings via pragma. Addressing them would be better, and most of the could be addressed via casting but maybe that isn't the best solution and the types of some variables needs changing to be fixed properly. So in the meantime I did this.
Thank you, @oviano Resolving warnings needs to be done. We'll either take into work this issue directly or use PR #1657 as a ~~permanent~~ temporal solution.
TODO
Fix MSVC compiler warnings (316 when built without encryption):
- [x] C4101 - local variable never used (2). PR #1669
- [ ] C4244 - conversion from type1 to type2 possible loss of data (62)
- [ ] C4267 - 'var' conversion from 'size_t' to 'type' possible loss of data (191). PR #1699 and #2675
- [x] C4275 - DLL-interface class 'class_1' used as base for DLL-interface class 'class_2' (19). PR #1675.
- [x] C4129- 'character' : unrecognized character escape sequence (1). PR #1669
- [ ] C4996 - Your code uses a function, class member, variable, or typedef that's marked deprecated (38):
sscanf
->sscanf_s
,etc. - [x] C4305 - truncation from 'type1' to 'type2' (1). PR #1669
- [ ] MinGW warnings reported in Travis CI
- [ ] Group-related warning reported by Travis CI (Linux build no. 2)
Looks like a few warnings remain to be fixed.
\srtcore\common.h(1396,18): warning C4996: 'sscanf': This function or variable may be unsafe. Consider using sscanf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
\srtcore\logging.h(169,9): warning 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.
\srtcore\logging.h(175,13): warning C4996: 'strcat': This function or variable may be unsafe. Consider using strcat_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
\srtcore\logging.h(176,13): warning C4996: 'strcat': This function or variable may be unsafe. Consider using strcat_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
\haicrypt\cryspr-openssl.c(51,51): warning C4267: 'function': conversion from 'size_t' to 'const int', possible loss of data
\haicrypt\cryspr-openssl.c(56,51): warning C4267: 'function': conversion from 'size_t' to 'const int', possible loss of data
\haicrypt\cryspr-openssl.c(166,54): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data
\haicrypt\cryspr-openssl.c(166,68): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data
\haicrypt\cryspr-openssl.c(166,80): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data
These are to be fixed by defaulting to OpenSSL EVP API (-DUSE_ENCLIB=openssl-evp
).
\haicrypt\cryspr-openssl.c(51,13): warning C4996: 'AES_set_encrypt_key': Since OpenSSL 3.0
\haicrypt\cryspr-openssl.c(56,13): warning C4996: 'AES_set_decrypt_key': Since OpenSSL 3.0
\haicrypt\cryspr-openssl.c(127,75): warning C4996: 'AES_encrypt': Since OpenSSL 3.0
But those are to be fixed then:
\srt-max\haicrypt\cryspr-openssl-evp.c(52,48): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data
\srt-max\haicrypt\cryspr-openssl-evp.c(177,61): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data
\srt-max\haicrypt\cryspr-openssl-evp.c(146,45): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data
\srt-max\haicrypt\cryspr-openssl-evp.c(230,61): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data
\srt-max\haicrypt\cryspr-openssl-evp.c(344,45): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data
\srt-max\haicrypt\cryspr-openssl-evp.c(344,63): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data
\srt-max\haicrypt\cryspr-openssl-evp.c(344,78): warning C4267: 'function': conversion from 'size_t' to 'int', possible loss of data
Unfortunately I build SRT by dropping the relevant source files directly into another project containing my own code, and I’m only set up to build using mbedTLS.
Happy to have a look based on the above logs though, when I am back from holiday next week.
@oviano Thank you for the heads up and for the contribution you've done already! The list of warnings is not that scary now 🙂
Now I just need to persuade you guys to turn on “warnings as errors” permanently 🙂
I don't need to be persuaded. As soon as it can be done - I'm in! 🙂
So I've addressed most of these here:
https://github.com/Haivision/srt/pull/2679
The only ones I've not addressed are the OpenSSL 3.0 deprecation ones as without building OpenSSL I can't really test any potential changes.
Also, let me know if you prefer a different solution to any of fixes for _CRT_SECURE_NO_WARNINGS.
@oviano
The only ones I've not addressed are the OpenSSL 3.0 deprecation ones as without building OpenSSL I can't really test any potential changes.
Those will go away if you build with -DUSE_ENCLIB=openssl-evp
, which would likely become the default on in the future releases.