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

Fixed build break when enabled SRT_ASSERT. Enabled _DEBUG in debug mode #2423

Merged
merged 3 commits into from Aug 2, 2022

Conversation

ethouris
Copy link
Collaborator

@ethouris ethouris commented Aug 1, 2022

Changes:

  1. INT_MAX replaced with std::numeric_limits<int>::max().
  2. Added <limits> header in buffer_rcv.cpp
  3. Added -D_DEBUG when ENABLE_DEBUG is on (debug only).
  4. Added cast in a condition to clear out the warning

@maxsharabayko maxsharabayko added this to the Next Release milestone Aug 1, 2022
@maxsharabayko maxsharabayko added Type: Maintenance Work required to maintain or clean up the code [core] Area: Changes in SRT library core labels Aug 1, 2022
@maxsharabayko
Copy link
Collaborator

As discussed offline, MSVC itself defines _DEBUG for a Debug build configuration and NDEBUG for a Release build configuration.
The build configuration is set during the build, not at the CMake's project configuration stage.

(configure)
cmake ./
(build)
cmake . --build --config debug

Running cmake ./ -DCMAKE_BUILD_TYPE=Debug defines _NDEBUG for all build configurations of MSVC. The _NDEBUG is thus defined in the release build as well.

image

Setting -DCMAKE_BUILD_TYPE for MSVC is probably not the right thing to do. Still, someone can do it, and redefining MSVC standard macros is not a good approach.

Copy link
Collaborator

@maxsharabayko maxsharabayko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address the above comment. 👆

@ethouris
Copy link
Collaborator Author

ethouris commented Aug 2, 2022

Ok, so I think we'll put this block that sets up the CMAKE_BUILD_TYPE basing on flags under NOT MICROSOFT condition. The problem here is that this should be blocked for all cases when the build type is not selected for configuration - which is known to be true currently for MSVC. Might be that there are also some other build systems that do it this way, but I'm not sure if cmake has a universal way to recognize it - this will be then a kind of temporary workaround.

@maxsharabayko maxsharabayko merged commit e48f43d into Haivision:master Aug 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[core] Area: Changes in SRT library core Type: Maintenance Work required to maintain or clean up the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants