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

several code warnings related to deprecated funcs, truncation, and unused vars #71

Open
diablodale opened this issue Mar 2, 2021 · 10 comments

Comments

@diablodale
Copy link
Contributor

Build results in 10+ warnings of code style, deprecation, truncation, etc. These can likely be resolved easily by minor code changes, or hiding the warnings with pragmas.

Setup

  • Microsoft Windows [Version 10.0.19042.804]
  • Visual Studio 2019 Community v16.8.6
  • Current develop branch 126f5fc

Repro

  1. clean git clone, configure, build

Result

[proc] Executing command: "C:\Program Files\CMake\bin\cmake.exe" --build c:/njs/lux/depthai-core/build --config Debug --target all -- -j 15
[build] [3/43   2% :: 0.027] Generating depthai-resources resource loader
[build] [3/43   4% :: 0.846] Generating intermediate file for C:/njs/lux/depthai-core/build/resources/depthai-bootloader-0.0.11.cmd
[build] [3/43   6% :: 4.622] Generating intermediate file for C:/njs/lux/depthai-core/build/resources/depthai-device-fwp-5e4974e930f4ecbe1bf718463f6a5da664e68788.tar.xz
[build] [18/43   9% :: 7.446] Building CXX object CMakeFiles\depthai-core.dir\src\device\CallbackHandler.cpp.obj
[build] ..\src\device\CallbackHandler.cpp(40): warning C4101: 'ex': unreferenced local variable
[build] [19/43  11% :: 7.574] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\AssetManager.cpp.obj
[build] ..\src\pipeline\AssetManager.cpp(84): warning C4267: 'initializing': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] ..\src\pipeline\AssetManager.cpp(93): warning C4267: 'argument': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] [20/43  13% :: 7.579] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\node\XLinkIn.cpp.obj
[build] [21/43  16% :: 7.631] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\node\MyProducer.cpp.obj
[build] [22/43  18% :: 7.642] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\node\XLinkOut.cpp.obj
[build] [23/43  20% :: 7.646] Building C object CMakeFiles\depthai-core.dir\shared\depthai-bootloader-shared\src\SBR.c.obj
[build] ..\shared\depthai-bootloader-shared\src\SBR.c(169): warning C4996: 'strncpy': This function or variable may be unsafe. Consider using strncpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
[build] [24/43  23% :: 7.655] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\node\StereoDepth.cpp.obj
[build] [25/43  25% :: 8.391] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\node\ImageManip.cpp.obj
[build] [26/43  27% :: 8.439] Building CXX object CMakeFiles\depthai-core.dir\src\device\DataQueue.cpp.obj
[build] [27/43  30% :: 8.472] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\node\ColorCamera.cpp.obj
[build] ..\src\pipeline\node\ColorCamera.cpp(273): warning C4244: 'initializing': conversion from 'int' to 'float', possible loss of data
[build] ..\src\pipeline\node\ColorCamera.cpp(274): warning C4244: 'initializing': conversion from 'int' to 'float', possible loss of data
[build] [28/43  32% :: 8.491] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\node\MonoCamera.cpp.obj
[build] [29/43  34% :: 8.496] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\Node.cpp.obj
[build] [30/43  37% :: 8.517] Building CXX object CMakeFiles\depthai-core.dir\shared\depthai-shared\src\datatype\DatatypeEnum.cpp.obj
[build] [31/43  39% :: 8.639] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\node\NeuralNetwork.cpp.obj
[build] ..\src\pipeline\node\NeuralNetwork.cpp(69): warning C4267: '=': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] [32/43  41% :: 9.182] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\Pipeline.cpp.obj
[build] [33/43  44% :: 9.934] Building CXX object CMakeFiles\depthai-core.dir\src\utility\Initialization.cpp.obj
[build] [34/43  46% :: 9.995] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\datatype\Buffer.cpp.obj
[build] [35/43  48% :: 10.502] Building CXX object CMakeFiles\depthai-core.dir\src\device\DeviceBootloader.cpp.obj
[build] ..\src\device\DeviceBootloader.cpp(101): warning C4267: 'argument': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] ..\src\device\DeviceBootloader.cpp(102): warning C4267: 'argument': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] ..\src\device\DeviceBootloader.cpp(107): warning C4267: 'argument': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] ..\src\device\DeviceBootloader.cpp(108): warning C4267: 'argument': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] ..\src\device\DeviceBootloader.cpp(113): warning C4267: 'argument': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] ..\src\device\DeviceBootloader.cpp(114): warning C4267: 'argument': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] ..\src\device\DeviceBootloader.cpp(119): warning C4267: 'argument': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] ..\src\device\DeviceBootloader.cpp(120): warning C4267: 'argument': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] ..\src\device\DeviceBootloader.cpp(130): warning C4267: 'argument': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] ..\src\device\DeviceBootloader.cpp(190): warning C4101: 'ex': unreferenced local variable
[build] ..\src\device\DeviceBootloader.cpp(202): warning C4101: 'error': unreferenced local variable
[build] ..\src\device\DeviceBootloader.cpp(287): warning C4267: '=': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] ..\src\device\DeviceBootloader.cpp(288): warning C4267: '=': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] ..\src\device\DeviceBootloader.cpp(336): warning C4267: '=': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] ..\src\device\DeviceBootloader.cpp(337): warning C4267: '=': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] ..\src\device\DeviceBootloader.cpp(380): 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.
[build] [36/43  51% :: 10.574] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\node\SystemLogger.cpp.obj
[build] [37/43  53% :: 10.633] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\node\VideoEncoder.cpp.obj
[build] ..\src\pipeline\node\VideoEncoder.cpp(93): warning C4244: '=': conversion from 'int' to 'float', possible loss of data
[build] ..\src\pipeline\node\VideoEncoder.cpp(137): warning C4244: 'return': conversion from 'const float' to 'int', possible loss of data
[build] ..\src\pipeline\node\VideoEncoder.cpp(145): warning C4244: 'argument': conversion from 'float' to 'int', possible loss of data
[build] ..\src\pipeline\node\VideoEncoder.cpp(157): warning C4244: '=': conversion from 'float' to 'int32_t', possible loss of data
[build] ..\src\pipeline\node\VideoEncoder.cpp(167): warning C4244: 'argument': conversion from 'float' to 'int', possible loss of data
[build] ..\src\pipeline\node\VideoEncoder.cpp(170): warning C4244: 'argument': conversion from 'float' to 'int', possible loss of data
[build] ..\src\pipeline\node\VideoEncoder.cpp(173): warning C4244: 'argument': conversion from 'float' to 'int', possible loss of data
[build] ..\src\pipeline\node\VideoEncoder.cpp(176): warning C4244: 'argument': conversion from 'float' to 'int', possible loss of data
[build] [38/43  55% :: 10.748] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\datatype\ImgFrame.cpp.obj
[build] [39/43  58% :: 10.826] Building CXX object CMakeFiles\depthai-core.dir\src\xlink\XLinkStream.cpp.obj
[build] ..\src\xlink\XLinkStream.cpp(25): warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of data
[build] ..\src\xlink\XLinkStream.cpp(62): warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of data
[build] ..\src\xlink\XLinkStream.cpp(115): warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of data
[build] ..\src\xlink\XLinkStream.cpp(133): warning C4244: 'argument': conversion from '_Rep' to 'unsigned int', possible loss of data
[build]         with
[build]         [
[build]             _Rep=__int64
[build]         ]
[build] ..\src\xlink\XLinkStream.cpp(133): warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of data
[build] ..\src\xlink\XLinkStream.cpp(153): warning C4244: 'argument': conversion from '_Rep' to 'unsigned int', possible loss of data
[build]         with
[build]         [
[build]             _Rep=__int64
[build]         ]
[build] ..\src\xlink\XLinkStream.cpp(167): warning C4244: 'argument': conversion from '_Rep' to 'unsigned int', possible loss of data
[build]         with
[build]         [
[build]             _Rep=__int64
[build]         ]
[build] [40/43  60% :: 11.212] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\node\DetectionNetwork.cpp.obj
[build] [41/43  62% :: 11.257] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\datatype\CameraControl.cpp.obj
[build] [41/43  65% :: 11.306] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\datatype\SystemInformation.cpp.obj
[build] [41/43  67% :: 11.591] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\datatype\ImgDetections.cpp.obj
[build] [41/43  69% :: 11.645] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\datatype\ImageManipConfig.cpp.obj
[build] ..\src\pipeline\datatype\ImageManipConfig.cpp(90): warning C4305: 'initializing': truncation from 'double' to 'const float'
[build] [41/43  72% :: 11.729] Building C object CMakeFiles\depthai-core.dir\src\bspatch\bspatch.c.obj
[build] ..\src\bspatch\bspatch.c(134): warning C4244: 'function': conversion from 'int64_t' to 'unsigned int', possible loss of data
[build] ..\src\bspatch\bspatch.c(129): warning C4244: 'initializing': conversion from 'int64_t' to 'unsigned int', possible loss of data
[build] [41/43  74% :: 11.978] Building CXX object CMakeFiles\depthai-core.dir\src\openvino\BlobReader.cpp.obj
[build] [41/43  76% :: 12.049] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\datatype\NNData.cpp.obj
[build] C:\.hunter\_Base\062a19a\8d58f30\2ddc225\Install\include\fp16/fp16.h(377): warning C4244: 'argument': conversion from 'float' to 'uint32_t', possible loss of data
[build] ..\src\pipeline\datatype\NNData.cpp(64): warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss of data
[build] ..\src\pipeline\datatype\NNData.cpp(91): warning C4267: '=': conversion from 'size_t' to 'unsigned int', possible loss of data
[build] ..\src\pipeline\datatype\NNData.cpp(120): warning C4244: 'argument': conversion from '_Ty' to 'float', possible loss of data
[build]         with
[build]         [
[build]             _Ty=double
[build]         ]
[build] [41/43  79% :: 12.064] Building CXX object CMakeFiles\depthai-resources.dir\__cmrc_depthai-resources\lib.cpp.obj
[build] [41/43  81% :: 12.120] Building CXX object CMakeFiles\depthai-core.dir\src\xlink\XLinkConnection.cpp.obj
[build] ..\src\xlink\XLinkConnection.cpp(27): warning C4996: 'strncpy': This function or variable may be unsafe. Consider using strncpy_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
[build] ..\src\xlink\XLinkConnection.cpp(42): warning C4267: 'initializing': conversion from 'size_t' to 'int', possible loss of data
[build] ..\src\xlink\XLinkConnection.cpp(92): warning C4267: 'argument': conversion from 'size_t' to 'const unsigned int', possible loss of data
[build] ..\src\xlink\XLinkConnection.cpp(205): warning C4267: 'argument': conversion from 'size_t' to 'unsigned long', possible loss of data
[build] ..\src\xlink\XLinkConnection.cpp(330): warning C4996: 'strncat': This function or variable may be unsafe. Consider using strncat_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
[build] ..\src\xlink\XLinkConnection.cpp(336): warning C4996: 'strncat': This function or variable may be unsafe. Consider using strncat_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
[build] ..\src\xlink\XLinkConnection.cpp(338): warning C4996: 'strncat': This function or variable may be unsafe. Consider using strncat_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
[build] [41/43  83% :: 12.147] Building CXX object CMakeFiles\depthai-core.dir\src\openvino\OpenVINO.cpp.obj
[build] [41/43  86% :: 12.281] Building CXX object CMakeFiles\depthai-resources.dir\__cmrc_depthai-resources\intermediate\depthai-bootloader-0.0.11.cmd.cpp.obj
[build] [41/43  88% :: 12.738] Building CXX object CMakeFiles\depthai-core.dir\src\device\Device.cpp.obj
[build] ..\src\device\Device.cpp(422): warning C4101: 'ex': unreferenced local variable
[build] ..\src\device\Device.cpp(567): warning C4101: 'ex': unreferenced local variable
[build] [41/43  90% :: 13.352] Building CXX object CMakeFiles\depthai-core.dir\src\utility\Resources.cpp.obj
[build] [41/43  93% :: 13.360] Building CXX object CMakeFiles\depthai-core.dir\src\pipeline\datatype\StreamPacketParser.cpp.obj
[build] ..\src\pipeline\datatype\StreamPacketParser.cpp(168): warning C4267: 'initializing': conversion from 'size_t' to 'uint32_t', possible loss of data
[build] [41/43  95% :: 15.210] Building CXX object CMakeFiles\depthai-resources.dir\__cmrc_depthai-resources\intermediate\depthai-device-fwp-5e4974e930f4ecbe1bf718463f6a5da664e68788.tar.xz.cpp.obj
[build] [42/43  97% :: 15.248] Linking CXX static library depthai-resourcesd.lib
[build] [43/43 100% :: 15.514] Linking CXX static library depthai-cored.lib
[build] Build finished with exit code 0

Expected

No warnings of unused vars, deprecated functions, potentially unsafe truncation, etc.

@Luxonis-Brandon
Copy link
Contributor

Thank you for the thorough report!

@themarpe
Copy link
Collaborator

themarpe commented Mar 2, 2021

@diablodale if you are well versed on MSVC side of things, you can create a PR addressing some of those and I'll take a look at it.

Otherwise will check those out once I have more time on hands.

@diablodale
Copy link
Contributor Author

I can prob do some of these. I suspect many of them are related to 64-bit compile. GSL::narrow_cast<> is the modern way to do those truncations, e.g. https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es46-avoid-lossy-narrowing-truncating-arithmetic-conversions.

While the deprecated/unsafe functions like strncpy() are likely in the class of security/buffer protection and might need a holistic look. The latter you can do quickly while I might need significant time to grok the corpus of code.

I'll look either way. Gimme a week.

@themarpe
Copy link
Collaborator

themarpe commented Mar 2, 2021

I'd like to bring in GSL as well - don't know about public usage though. I guess it would be okay if used only in private headers/sources. What do you think?

I already took a quick look at the strncpy() and the likes - will tackle those myself.

Thanks a lot!

@diablodale
Copy link
Contributor Author

@themarpe , to which branch should I do work and submit PRs?
develop, gen2_develop, or ...?

@themarpe
Copy link
Collaborator

themarpe commented Mar 3, 2021

@diablodale to develop please.
I think I can add some defaults for this - will look into it

@diablodale
Copy link
Contributor Author

I have a PR ready, but ctest is failing on the unchanged develop branch #77
When I get clarity on how to correct ctest so unit testing can be done, I can submit the PR.

I request your input and/or review on several topics -- they will be highlighted in the PR. I can always update the PR. 😄 I might have several commits in the PR as we work through it. I will gladly squash them all into one for the final.

@Luxonis-Brandon
Copy link
Contributor

Thank you!

@diablodale
Copy link
Contributor Author

@themarpe where does your team stand on the idea to use gsl? https://github.com/microsoft/GSL

I would recommend to first use its gsl::narrow_cast() for the noisy truncates/loss of data. Probably doing it in waves of commits and checking where it is applied that the trunc/loss is actually desired...or if a floor(), ceil(), etc. is more appropriate.

@themarpe
Copy link
Collaborator

I'm in favor of it - also for the span instead of having to do std::vector& assuming that caller will have data managed by a vector.

GSL should be also available as is by Hunter although not sure if the latest.

We'd have to specify the version for it if we use it publically, as we did with nlohmann json the other day

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

No branches or pull requests

3 participants