Page MenuHome

Logging: experimental CLOG_DEBUG code
Needs ReviewPublic

Authored by Andrew Williams (sobakasu) on Jan 18 2019, 4:57 AM.

Details

Summary

so I noticed there is a G.debug and a lot of different --debug options to enable debugging in various parts of the code. this seems to have some overlap with what the clog code does. this is my attempt at adding debugging support to clog and removing more printf statements.

Summary of changes

  • added a CLOG_DEBUG macro
  • changed a bunch of printf statements to use CLOG_DEBUG
  • second attempt at refactoring duplicated code out of CLG_log/CLG_logf
  • refactored some repeated code out of blf.c
  • removed lots of err_out parameters from some function calls (not sure if this was a good idea or not now)
  • attempt to refactor duplicated code from report.c
  • the --debug-* command line options also set some corresponding clog filters

Diff Detail

Repository
rB Blender

Event Timeline

Andrew Williams (sobakasu) updated this revision to Diff 13253.

diff fixes

Andrew Williams (sobakasu) marked 5 inline comments as done.Jan 18 2019, 5:33 AM
Andrew Williams (sobakasu) added inline comments.
.editorconfig
9
extern/bullet2/src/LinearMath/btScalar.h
110 ↗(On Diff #13253)

I changed this to use a %s argument for #x, because if you have '%' in the assert condition it causes a compiler warning

intern/clog/CLG_log.h
172

the idea of this is so that code can test if debugging is enabled, e.g.

if (CLOG_TEST_DEBUG(&LOG)) {
   ... debugging statements here
}
195

the while(0) change makes it work in an if statement without brackets

source/creator/creator_args.c
108

this is the code that ties together --debug-* and clog filter names
e.g. --debug-wm could enable --log "wm.*"

removed parts of diff that weren't supposed to be in this set of changes

Some useful changes, but mixed in other decisions I'd rather not include.

  • Removing error messages from the GPU module makes reports (which uses see in error popups) and Python exceptions less useful.
  • Would not mix --debug-* with logging, instead we can migrate towards logging and only use --debug-* options for changed behavior, unrelated to printing.
  • Not convinced we should mix logging with BKE_report, reporting are issues users should know about - when tools fail for example. Logging is more internal information for developers and technical users.

Will apply parts of this patch, in future would prefer patches like this be split into categories, eg:

  • Use of logging in areas where printf's are used.
  • Changes to command line argument handling.
  • Changes to CLOG API/behavior.
  • Changes to error handling and how messages are passed between modules (as was done for GPU module in this patch).

This is just a rule of thumb so review goes smoother.

intern/clog/CLG_log.h
96

Not used anywhere.

203–206

Good catch, unneeded varargs use.

intern/clog/clog.c
166–169

Would rather not do this, logging calls can just not add newlines to begin with.

241

Would not add this, since it's not a level (glog which clog is loosely based on doesn't have a debug level).

Would use info with verbosity level of 1 or more in this case.

intern/ghost/intern/GHOST_SystemWin32.cpp
1056 ↗(On Diff #13254)

Seems unrelated.

source/blender/blenfont/intern/blf.c
210 ↗(On Diff #13254)

This isn't related to logging, should be a separate patch.

extern/bullet2/src/LinearMath/btScalar.h
110 ↗(On Diff #13253)

This isn't maintained by Blender, (extern/ modules typically aren't), changes should be made up-stream.

ok, thanks for those tips, i'll keep them in mind for next time

Andrew Williams (sobakasu) marked an inline comment as done.Jan 19 2019, 12:14 AM
Andrew Williams (sobakasu) added inline comments.
extern/bullet2/src/LinearMath/btScalar.h
110 ↗(On Diff #13253)

oops.. i just checked bullet source code and it looks like it's been fixed there in master

  • reverted err_out parameter related changes.
  • removed blen_font refactored code
  • added an exciting new ERROR_OUT macro for gpu_*