Bug 68390

Created attachment 84400 [details] [review]
TpConnectionManager, TpProtocol: improve debug output

Created attachment 84401 [details] [review]
TpConnectionManager: add more debug for  tp_list_connection_managers()

Based on a patch by Guillaume Desmottes.

Created attachment 84402 [details] [review]
inspect-cm example: allow inspecting all CMs

Also print a bit more information about protocols, and allow requesting
timestamps in debug output.

Comment on attachment 84400 [details] [review]
TpConnectionManager, TpProtocol: improve debug output

Review of attachment 84400 [details] [review]:
-----------------------------------------------------------------

++

Comment on attachment 84401 [details] [review]
TpConnectionManager: add more debug for  tp_list_connection_managers()

Review of attachment 84401 [details] [review]:
-----------------------------------------------------------------

::: telepathy-glib/connection-manager.c
@@ +1853,4 @@
>  {
>    _ListContext *list_context = user_data;
>    const gchar * const *name_iter;
> +  const gchar *method;

Shouldn't you guard this with #idef ENABLE_DEBUG ?

Comment on attachment 84402 [details] [review]
inspect-cm example: allow inspecting all CMs

Review of attachment 84402 [details] [review]:
-----------------------------------------------------------------

++

Created attachment 86446 [details] [review]
TpConnectionManager: add more debug for  tp_list_connection_managers()

Based on a patch by Guillaume Desmottes.

---

(In reply to comment #5)
> Comment on attachment 84401 [details] [review]
> TpConnectionManager: add more debug for  tp_list_connection_managers()
> > +  const gchar *method;
> 
> Shouldn't you guard this with #idef ENABLE_DEBUG ?

Yeah, I suppose so.

Comment on attachment 86446 [details] [review]
TpConnectionManager: add more debug for  tp_list_connection_managers()

Review of attachment 86446 [details] [review]:
-----------------------------------------------------------------

++

(In reply to comment #7)
> > Shouldn't you guard this with #idef ENABLE_DEBUG ?
> 
> Yeah, I suppose so.

Actually, the answer is "no". DEBUG() unconditionally evaluates its arguments these days, because of a 2011 commit from wjt (see below).

He didn't say why in the commit message, but I suspect the answer is "so we don't have to wrap everything in #ifdef ENABLE_DEBUG to avoid unused-variable warnings".

I'm about this close --->||<--- to deleting the --disable-debug option altogether.

commit 3e2df72ead78c9f96beb0cefb77e766733a390f1
Author: Will Thompson <will.thompson@collabora.co.uk>
Date:   2011-07-07 17:45:31 +0100

    Make DEBUG a no-op static inline with --disable-debug.

diff --git a/telepathy-glib/debug-internal.h b/telepathy-glib/debug-internal.h
index 583e3d7..bad08aa 100644
--- a/telepathy-glib/debug-internal.h
+++ b/telepathy-glib/debug-internal.h
@@ -100,7 +100,16 @@ G_END_DECLS
           G_STRFUNC, ##__VA_ARGS__)
 #   define DEBUGGING _tp_debug_flag_is_set (DEBUG_FLAG)
 #else /* !defined (ENABLE_DEBUG) */
-#   define DEBUG(format, ...) do {} while (0)
+#   ifndef DEBUG_STUB_DEFINED
+static inline void
+DEBUG (
+    const gchar *format,
+    ...)
+{
+#   define DEBUG_STUB_DEFINED 1
+#   endif // ifndef DEBUG_STUB_DEFINED
+

(In reply to comment #9)
> I'm about this close --->||<--- to deleting the --disable-debug option
> altogether.

I agree. Has it ever been used?

Comment on attachment 84401 [details] [review]
TpConnectionManager: add more debug for  tp_list_connection_managers()

Review of attachment 84401 [details] [review]:
-----------------------------------------------------------------

++ for this patch after all.

Applied Attachment #84401 [details] for 0.23.1, thanks. I already applied the other two patches.

(In reply to comment #9)
> I'm about this close --->||<--- to deleting the --disable-debug option
> altogether.

Done in 0.99.5, FYI. I'm not going to bother with this for 0.x.