While debugging a hang in empathy-accounts (which I think is in fact just telepathy-haze suffering from <https://bugzilla.gnome.org/show_bug.cgi?id=698081>), Guillaume and I noticed that there isn't a whole lot of debug info in TpConnectionManager or tp_list_connection_managers[_async](). Also, the debug messages don't usually say which CM or protocol they're talking about, which makes them less useful than they could be.
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.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.