Skip to content

Conversation

@jukkar
Copy link
Member

@jukkar jukkar commented Jul 10, 2018

If user has set this in .c file:

#define LOG_MODULE_NAME abc
#define LOG_LEVEL 0

#include <logging/log.h>
LOG_MODULE_REGISTER();

Here debug level is set to 0 which means that debug prints
are disabled. Then this compile error will be emitted:

include/logging/log_instance.h:83:47: error: ‘log_dynamic_abc’
undeclared (first use in this function);
did you mean ‘log_dynamic_xyz’?

The commit avoids this by setting debug macros to be no-op.

Signed-off-by: Jukka Rissanen [email protected]

If user has set this in .c file:

  #define LOG_MODULE_NAME abc
  #define LOG_LEVEL 0

  #include <logging/log.h>
  LOG_MODULE_REGISTER();

Here debug level is set to 0 which means that debug prints
are disabled. Then this compile error will be emitted:

  include/logging/log_instance.h:83:47: error: ‘log_dynamic_abc’
    undeclared (first use in this function);
    did you mean ‘log_dynamic_xyz’?

The commit avoids this by setting debug macros to be no-op.

Signed-off-by: Jukka Rissanen <[email protected]>
@codecov-io
Copy link

Codecov Report

Merging #8828 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8828      +/-   ##
==========================================
- Coverage   52.32%   52.32%   -0.01%     
==========================================
  Files         195      195              
  Lines       24730    24730              
  Branches     5140     5140              
==========================================
- Hits        12941    12940       -1     
  Misses       9715     9715              
- Partials     2074     2075       +1
Impacted Files Coverage Δ
boards/posix/native_posix/timer_model.c 93.54% <0%> (-1.62%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e854dd0...aa1504e. Read the comment docs.

@nordic-krch
Copy link
Contributor

I would like to avoid this solution because then macros are removed when logging is off. This tends to lead to some compilation warnings (e.g. when variables are created only for logging purpose). The initial goal was to keep logging macros pure C to avoid dead code.
I think that logger internals can be tweaked to handle that. I'll try to come up with the solution.

@jukkar
Copy link
Member Author

jukkar commented Jul 10, 2018

Sure, that makes sense. This was just a quick solution in order to get things compiled.

@pizi-nordic pizi-nordic self-requested a review July 10, 2018 10:57
@andrewboie
Copy link
Contributor

Can we do this with empty inline functions instead?
That would alleviate problems with unused variables.

@jukkar
Copy link
Member Author

jukkar commented Jul 11, 2018

Better solution in #8840 so no need to this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants