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

[utils] CLog: add Log and LogFunction function with additional component parameter #12676

Merged
merged 4 commits into from
Aug 16, 2017

Conversation

Rechi
Copy link
Member

@Rechi Rechi commented Aug 15, 2017

Description

Add additional Log and LogFunction functions to the CLog class which take a component parameter. The Functions check if component logging for that component is enabled and log in that case otherwise they do nothing.

Motivation and Context

make component logging easier

How Has This Been Tested?

compiled for windows

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves existing functionality)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the Code guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my change
  • All new and existing tests passed

@Rechi Rechi added Type: Improvement non-breaking change which improves existing functionality Logging v18 Leia labels Aug 15, 2017
@Montellese
Copy link
Member

Makes sense IMO but right now it only compiles on windows.

@popcornmix
Copy link
Member

Makes sense to me too.
One issue I have with LogF is __FUNCTION__ is not very informative. A lot of times this gives an ambiguous name like Create, Open, Close with no clues as to which component actually generated the message.

Most of the code I write uses:
CLog::Log(LOGDEBUG, "%s::%s Destroying pool", CLASSNAME, __FUNCTION__);
and I have to manually define CLASSNAME as it doesn't seem to be available from compiler.
This is a lot more useful information. I wonder if we could provide a standard way of achieving this?

It looks like gcc provides __PRETTY_FUNCTION__ (with msvc __FUNCSIG__ may be interesting, but I haven't investigated).

But __PRETTY_FUNCTION__ evaluates to something like:
MMAL::CMMALPool::CMMALPool(const char*, bool, uint32_t, uint32_t, uint32_t, MMAL::MMALState)
which is too verbose for logging, but truncating the string at the ( might give something useful.

@Rechi
Copy link
Member Author

Rechi commented Aug 15, 2017

now it compiles on all platforms

@Rechi Rechi added this to the L 18.0-alpha1 milestone Aug 16, 2017
@Rechi Rechi merged commit 2a54fd8 into xbmc:master Aug 16, 2017
@Rechi Rechi deleted the componentLogging branch August 16, 2017 11:39
@Rechi Rechi mentioned this pull request Aug 16, 2017
10 tasks
@MartijnKaijser
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Improvement non-breaking change which improves existing functionality v18 Leia
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants