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.
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.