While tracking bug https://bugs.freedesktop.org/show_bug.cgi?id=88896 it has been recognized that the dbus-monitor --profile do not print out any header, which makes it hard to interpret the displayed columns.
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.