[Bug 37741] Plugins have no common debug infrastructure so tuning the output level is hard

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=37741

Simon McVittie <simon.mcvittie at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|NB#239794                   |NB#239794, review-

--- Comment #1 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2011-05-30 04:01:13 PDT ---
+typedef enum {
+ MCP_DEBUG_NONE = 0,
+ MCP_DEBUG_ACCOUNT = 1 << 0,
+ MCP_DEBUG_ACCOUNT_STORAGE = 1 << 1,
+ MCP_DEBUG_DBUS_ACL = 1 << 2,
+ MCP_DEBUG_DISPATCH_OPERATION = 1 << 3,
+ MCP_DEBUG_DISPATCH_OPERATION_POLICY = 1 << 4,
+ MCP_DEBUG_LOADER = 1 << 5,
+ MCP_DEBUG_REQUEST = 1 << 6,
+ MCP_DEBUG_REQUEST_POLICY = 1 << 7,
+} McpDebugType;
+
+gboolean mcp_debugging (McpDebugType type);

I'd prefer mcp_is_debugging() and McpDebugFlags.

This takes an implementation detail (the set of debug domains) and makes it API
- be careful with that sort of thing.

> #define DEBUG(format, ...) \
> - _mcp_debug ("%s: " format, G_STRFUNC, ##__VA_ARGS__)
> + if (mcp_debugging (MCP_DEBUG_TYPE)) \
> + g_debug ("%s: " format, G_STRLOC, ##__VA_ARGS__)

All function-like macros that expand to more than one statement should have
their body wrapped in G_STMT_START { ... } G_STMT_END to avoid astonishing
precedence. (If in doubt, copy from telepathy-glib...)

> +++ b/mission-control-plugins/debug.h
> +#ifdef ENABLE_DEBUG

Not in a public header, please. Public headers should never rely on anything
from config.h, because anyone could #include them (not just MC itself).

In-tree plugins should get their MCP_DEBUG() (or just DEBUG() - up to you,
because it shouldn't be public API anyway, for this reason) from an
in-tree-only header like "mission-control-plugins/debug-internal.h".

Out-of-tree plugins should have their own debug-internal.h (per project) that
does its own #ifdef, which depends whether the project providing the plugin has
been compiled with debug enabled.

Rule of thumb: none of the observable functionality of a library should depend
on its configure options, and its API certainly shouldn't "change shape". (It'd
be OK if mcp_is_debugging() always returned FALSE if compiled without
debugging, or if mcp_debug() did nothing if compiled without debugging, or
something.)

> Squash a recent compiler warning
> + {

This commit shouldn't have been needed. Use G_STMT_START properly and it can be
removed.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.