Bug 26642

Ah, sorry, I said this on the wrong bug:

I notice your branch removes an extern function. Please don't do that (it would break osso-mission-control on Maemo unless this function is unused, and I happen to know that mcd_debug_set_level() is called by osso-mission-control).

Sorry, didn't realize there were external consumers.

I returned a cleaner version of mcd_debug_set_level() to my branch.

Bah. I'm sure I reviewed this already, but the combination of Bugzilla and unstable 3G on a train ate it.

Basically, the problem is that osso-mission-control uses mcd-debug.h, and because a lot of that header is inline, it hard-codes the version that was current at the time it was compiled. Also, I don't want to expand MC's API/ABI - I'm trying to stop src/ having any public API at all (in favour of just having OMC use libmission-control-plugins) but we're not there yet.

So, I think we should introduce a debug-internal.h, which defines McdDebugFlags, _mcd_debug_flag_is_set (prefix an underscore to make it internal to MC), and your version of DEBUG() (only if DEBUG_FLAG is defined). All MC files should include debug-internal.h instead of mcd-debug.h.

mcd-debug.h should keep defining DEBUG() too (to support OMC), but only if DEBUG_FLAG is *not* defined.

mcd-debug.h should continue to define _mcd_debug_get_level for source compatibility.

> -gint mcd_debug_level = 0;

This must continue to exist; otherwise, OMC will abort on startup with a linker error. (Don't remove extern symbols.)

-------------------

Review comments that don't relate to the general approach:

> +                       mcp_set_debug ((flags != 0));

This should check MCD_DEBUG_PLUGIN - mcp is the plugins library

> + #define DEBUGGING mcd_debug_flag_is_set (DEBUG_FLAG)

Parenthesize the definition, since it's an expression: #define DEBUGGING (_mcd_debug_flag_is_set (DEBUG_FLAG))

>  {
> -    mcd_debug_level = level;
> +               if (level == 1)
> +               {
> +                flags = ~MCD_DEBUG_TREES;

What's going on with the indentation? 4-space indentation for existing files, Telepathy style (GNUish 2+2 space indentation) for entirely new files, and no tabs anywhere, please.

Please reinstate the patch keyword when this is ready for review again.

Did the changes you suggested.
Hope I found all the whitespace issues.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/telepathy/telepathy-mission-control/issues/20.