Open hodoulp opened 4 years ago
Thanks for the report.
I suggest we drop IEXDEBUGTHROW. I added that more than a decade ago when debuggers lacked a consistent ability to break on exception throwing. Nowadays, it's trivially easy. That solves the use of getenv in Iex. For the command line tools it would make sense to copy the pattern in OCIO that you've linked.
operator = in ImfName.h is vaguely unsafe, and using the _s variant is a panacea. The warning does highlight a potential problem. Probably we should review the assignment operator, and sanity check things, by at least ensuring that there is a terminating zero. At that point, acceding to annoying nag and calling strncpy_s on Windows is probably more desirable than using a _CRT_SECURE_NO_WARNINGS hack, since _CRT_SECURE_NO_WARNINGS leaks out to projects using OpenEXR.
All of the conversion warnings need individual attention, because in many cases a static_cast is the right answer, but in order to avoid introducing new bugs versus images with enormous dimensions, we should think each one through.
Our bandwidth right now is consumed by the migration to ASWF, so PRs from the community addressing these issues, even little by little, would be most welcome.
calling strncpy_s on Windows is probably more desirable than using a _CRT_SECURE_NO_WARNINGS
Agree.
Anyway there is nothing blocking an occasional Windows contributor. That's much more an annoyance because of the number of warnings.
Another option we discussed is to disable the warning via a #pragma in the header.
@hodoulp, if you'd consider submitting a PR, we'll review.
Revisiting this, we're preparing a 3.0 release in the next few weeks, if anyone would like to contribute a PR that addresses these warnings, now would be a good time.
Note: we should inventory warnings under msvc again, and update this issue as appropriate
There are still a number of warnings, here are a few representative messages, each of which recurs many times.
C:\Projects\openexr\src\lib\OpenEXR\ImfName.h(83,5): 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. [C:\Projec
ts\build\src\bin\exrstdattr\exrstdattr.vcxproj]
C:\Projects\openexr\src\bin\exrmultipart\exrmultipart.cpp(576,19): warning C4834: discarding return value of function
with 'nodiscard' attribute [C:\Projects\build\src\bin\exrmultipart\exrmultipart.vcxproj]
C:\Projects\openexr\src\bin\exrmultipart\exrmultipart.cpp(381,59): warning C4267: 'argument': conversion from 'size_t'
to 'int', possible loss of data [C:\Projects\build\src\bin\exrmultipart\exrmultipart.vcxproj]
C:\Projects\openexr\src\bin\exrmultipart\exrmultipart.cpp(381,59): warning C4267: 'argument': conversion from 'size_t'
to 'int', possible loss of data [C:\Projects\build\src\bin\exrmultipart\exrmultipart.vcxproj]
C:\Projects\openexr\src\bin\exrenvmap\blurImage.cpp(320,27): warning C4244: 'argument': conversion from 'int' to 'T',
possible loss of data [C:\Projects\build\src\bin\exrenvmap\exrenvmap.vcxproj]
C:\Projects\openexr\src\lib\OpenEXRUtil\ImfCheckFile.cpp(518,38): warning C4244: 'argument': conversion from 'uint64_t
' to 'long', possible loss of data [C:\Projects\build\src\lib\OpenEXRUtil\OpenEXRUtil.vcxproj]
When compiling
master
&RB-2.4
withVisual Studio 17
, there are lot of warnings. Most of them only require a small change in the source code. But one require several iterations to be correctly fixed in OCIO.Please refer to OCIO Getenv() implementation for details. To fully validate the Windows implementation with a unit test, you should use
putenv
but alsoSetEnvironmentVariable
.As lot of warnings could be fixed without having a Windows machine I copy & paste part of compilation results to only have warning messages.