| Summary: | TpDebugSender log handler conflicts with tp_debug_timestamped_log_handler | ||
|---|---|---|---|
| Product: | Telepathy | Reporter: | Simon McVittie <smcv> |
| Component: | tp-glib | Assignee: | Telepathy bugs list <telepathy-bugs> |
| Status: | RESOLVED FIXED | QA Contact: | Telepathy bugs list <telepathy-bugs> |
| Severity: | normal | ||
| Priority: | medium | Keywords: | patch |
| Version: | git master | ||
| Hardware: | Other | ||
| OS: | All | ||
| URL: | http://cgit.collabora.com/git/user/wjt/telepathy-glib.git/log/?h=qed | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Bug Depends on: | |||
| Bug Blocks: | 26517 | ||
Take, review; this is my patch. fe8dd650a78d r+, and please cherry-pick to 0.12 too. In the second commit, I'm a bit sad that tp_debug_sender_log_handler now potentially calls g_get_current_time twice. (In reply to comment #2) > fe8dd650a78d r+, and please cherry-pick to 0.12 too. Cheers. > In the second commit, I'm a bit sad that tp_debug_sender_log_handler now > potentially calls g_get_current_time twice. Yeah I wondered about that. Got a nice solution in mind? (In reply to comment #3) > (In reply to comment #2) > > fe8dd650a78d r+, and please cherry-pick to 0.12 too. > > Cheers. > > > In the second commit, I'm a bit sad that tp_debug_sender_log_handler now > > potentially calls g_get_current_time twice. > > Yeah I wondered about that. Got a nice solution in mind? If we could find a GSource kicking around we could use g_source_get_current_time() which is really cheap. Hello. + * If timestamps should be prepended to messages (like in + * tp_debug_timestamped_log_handler()), tp_debug_sender_set_messages() + * should also be called. That function doesn't exist. (In reply to comment #3) > Yeah I wondered about that. Got a nice solution in mind? Oh, it's literally twice in the same function, not indirected through a bunch of function calls. Float GTimeVal now = { 0, 0 }; out to the top-level scope in the function, and in the second branch only call g_get_current_time if now.tv_sec == 0? I don't think you're gonna get nicer than that. I have rebased the branch and fixed my own criticisms. looks good, +1 Merged for 0.15.5; thanks for the review! |
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.
tp_debug_timestamped_log_handler and tp_debug_sender_log_handler can't be used at the same time. It'd probably be more useful to have some way to set up tp_debug_sender_log_handler so it timestamps any messages that it sends to stdout/stderr, perhaps via a function like void tp_debug_sender_set_timestamps (gboolean maybe); which can be called conditionally on a FOO_TIMING environment variable. Alternatively, since we're fetching a GTimeVal for the debug sender's benefit *anyway*, we could just use timestamps unconditionally.