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

Check for NDEBUG in logInfo [12218] #2089

Merged
merged 1 commit into from
Aug 13, 2021
Merged

Conversation

EduPonz
Copy link

@EduPonz EduPonz commented Jul 23, 2021

As it is right now, logInfo only produces messages under the following conditions:

  1. HAVE_LOG_NO_INFO is false AND
  2. Either FASTDDS_ENFORCE_LOG_INFO is defined OR
  3. __INTERNALDEBUG or _INTERNALDEBUG is defined AND _DEBUG or __DEBUG is defined.
  • HAVE_LOG_NO_INFO can be set by users with -DLOG_NO_INFO, which defaults to OFF in debug and ON in every other configuration.
  • FASTDDS_ENFORCE_LOG_INFO can be defined by the user and defaults to OFF
  • __INTERNALDEBUG is defined when compiling Fast DDS with -DINTERNAL_DEBUG, which defaults to OFF
  • _INTERNALDEBUG can be defined by the user. Fast DDS does not define it, nor Fast DDS CMakeLists
  • _DEBUG is defined when using Windows config generators in build type is debug
  • __DEBUG is defined iff CMAKE_BUILD_TYPE is set to exactly Debug (other casing will not match)

This means that a Linux user building in debug with CMAKE_BUILD_TYPE=DEBUG (or something other than Debug) and with -DINTERNAL_DEBUG can end up with no log infos. This PR adds an additional check for case 3, which is check for either _DEBUG, __DEBUG or that NDEBUG is not defined. NDEBUG is actually a C++ standard definition that must be set when building is something other than debug. CMake adds this definition by itself, so no CMakeLists.txt needs to be modified.

Signed-off-by: Eduardo Ponz Segrelles eduardoponz@eprosima.com

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz EduPonz changed the title Check for NDEBUG in logInfo Check for NDEBUG in logInfo [12218] Jul 23, 2021
Copy link
Contributor

@IkerLuengo IkerLuengo left a comment

Choose a reason for hiding this comment

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

LGTM

@EduPonz
Copy link
Author

EduPonz commented Aug 9, 2021

@richiware please test this

@EduPonz
Copy link
Author

EduPonz commented Aug 9, 2021

@richiprosima please test this

@EduPonz EduPonz merged commit 339bf2d into master Aug 13, 2021
@EduPonz EduPonz deleted the feature/log_check_ndebug branch August 13, 2021 09:31
SamuelWHalodi pushed a commit to SamuelWHalodi/Fast-DDS that referenced this pull request Oct 4, 2021
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
Signed-off-by: Samuel Wilhelmsson <samuel@halodi.com>
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

Successfully merging this pull request may close these issues.

2 participants