Summary: | dbus-monitor --profile output format issue | ||
---|---|---|---|
Product: | dbus | Reporter: | Ralf Habacker <ralf.habacker> |
Component: | core | Assignee: | D-Bus Maintainers <dbus> |
Status: | RESOLVED FIXED | QA Contact: | D-Bus Maintainers <dbus> |
Severity: | normal | ||
Priority: | medium | ||
Version: | 1.8 | ||
Hardware: | Other | ||
OS: | All | ||
Whiteboard: | |||
i915 platform: | i915 features: | ||
Attachments: |
dbus-monitor.c: convert tabs to spaces
dbus-monitor.c: unify columns format in --profile mode and display column header. dbus-monitor: convert remaining hard tabs to 8 spaces dbus-monitor.c: unify columns format in --profile mode and display column header. dbus-monitor: clarify column headers dbus-monitor: Combine sec and usec columns into one timestamp column dbus-monitor: Remove empty column in profile mode |
Description
Ralf Habacker
2015-02-16 08:54:27 UTC
A research on the related code gives: type |---- timestamp ---| seconds usec serial path interface member sig 1424074437 962905 2 /org/freedesktop/DBus org.freedesktop.DBus NameAcquired type |---- timestamp ---| seconds usec serial sender path interface member mc 1424074437 962925 2 :1.8560 /org/freedesktop/DBus org.freedesktop.DBus AddMatch type |---- timestamp ---| ¦- serial -| destination seconds usec sender replay mr 1424074441 91430 78816 18470 :1.1174 type |---- timestamp ---| ¦- serial -| destination seconds usec sender replay err 1424074443 720703 64 5 :1.8561 There are three different formats, which makes it hard to define a common column headers. Created attachment 113523 [details] [review] dbus-monitor.c: convert tabs to spaces Created attachment 113524 [details] [review] dbus-monitor.c: unify columns format in --profile mode and display column header. Note: The header for the 'reply serial' value printed for method return and error has been named 'ref serial' because in fact it references the serial of the related method call (which is also wrong titled in --monitor mode) (In reply to Ralf Habacker from comment #2) > dbus-monitor.c: convert tabs to spaces As much as I'd like to say "yes, fix our whitespace", you seem to have converted hard tabs to 4 spaces, which leads to some bizarre indentation, like the "then" clause of an "if" being less indented than the "if" itself. Created attachment 113525 [details] [review] dbus-monitor: convert remaining hard tabs to 8 spaces (In reply to Ralf Habacker from comment #3) > Note: The header for the 'reply serial' value printed for method return and > error has been named 'ref serial' because in fact it references the serial > of the related method call (which is also wrong titled in --monitor mode) I realise it isn't an ideal name, but "reply_serial" is what this is called internally (e.g. REPLY_SERIAL in the Specification, and dbus_message_get_reply_serial()). I would suggest either keeping it as-is, or calling it "in reply to" which makes the purpose clear. Created attachment 113526 [details] [review] dbus-monitor.c: unify columns format in --profile mode and display column header. From: Ralf Habacker <ralf.habacker@freenet.de> [rebase onto correctly indented version -smcv] --- This is just Attachment #113524 [details] adjusted to apply after Attachment #113525 [details]. Created attachment 113527 [details] [review] dbus-monitor: clarify column headers - change "ref serial" to "in_reply_to" (avoiding whitespace for easy visual parsing) - prefix with # to clarify that these are not part of the data Comment on attachment 113526 [details] [review] dbus-monitor.c: unify columns format in --profile mode and display column header. Review of attachment 113526 [details] [review]: ----------------------------------------------------------------- ::: tools/dbus-monitor.c @@ +184,5 @@ > } > > + if (first) > + { > + profile_print_headers(); space after function name missing (my fault, I really need to have an astyle or indent setting) Comment on attachment 113525 [details] [review] dbus-monitor: convert remaining hard tabs to 8 spaces Review of attachment 113525 [details] [review]: ----------------------------------------------------------------- looks good Comment on attachment 113527 [details] [review] dbus-monitor: clarify column headers Review of attachment 113527 [details] [review]: ----------------------------------------------------------------- looks good Comment on attachment 113526 [details] [review] dbus-monitor.c: unify columns format in --profile mode and display column header. Review of attachment 113526 [details] [review]: ----------------------------------------------------------------- ::: tools/dbus-monitor.c @@ +131,5 @@ > > static void > +profile_print_headers (void) > +{ > + printf ("type\tsec\tusec\tserial\tsender\tdestination\tpath\tinterface\tmember\n"); For what is the separation between seconds an usecs good ? To calculate time difference it is always required to combine both columns, so this separation seems to make no sense. (In reply to Ralf Habacker from comment #12) > For what is the separation between seconds an usecs good ? No idea, I didn't add --profile format. Combining them into "%lu.%06lu", (unsigned long) sec, (unsigned long) usec would be fine IMO. (In reply to Ralf Habacker from comment #9) > space after function name missing (my fault, I really need to have an astyle > or indent setting) Fixed that and pushed all three patches for 1.9.12. I'll leave this bug open for now if you're thinking about combining sec and usec. Created attachment 113533 [details] [review] dbus-monitor: Combine sec and usec columns into one timestamp column Created attachment 113534 [details] [review] dbus-monitor: Remove empty column in profile mode Comment on attachment 113533 [details] [review] dbus-monitor: Combine sec and usec columns into one timestamp column Review of attachment 113533 [details] [review]: ----------------------------------------------------------------- Looks good, although the header-changing bit should ideally have been part of this commit and not the next one. Comment on attachment 113534 [details] [review] dbus-monitor: Remove empty column in profile mode Review of attachment 113534 [details] [review]: ----------------------------------------------------------------- Sure, ship it Comment on attachment 113533 [details] [review] dbus-monitor: Combine sec and usec columns into one timestamp column applied to master with merge of header changing bits Comment on attachment 113534 [details] [review] dbus-monitor: Remove empty column in profile mode applied to master with header column remove only For the record: parameter list of error returns does not match --monitor mode |
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.