| Message ID | 20190110214346.8054-1-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix formatting of fixed-point numbers to not skip zeros | expand |
On Thu, 10 Jan 2019 22:43:46 +0100 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > When a timestamp that represents 1.00001 is converted to (for example) > seconds and microseconds this results in seconds=1 and useconds=10. To > format that into a string requires the format %lu.%06lu to yield > "1.000010". Some code parts used %lu.%lu however which results in "1.10" > which is wrong. Thanks Uwe! Yordan, can you review this patch and give an Acked-by or Reviewed-by tag? Thanks! -- Steve > > This was noticed because I failed to find a significant location in a trace > provided by a colleague as the reported timestamp was wrong :-| > > This change fixes all occurrences of this problem found by > $(grep -E '\.%[^0s]'). > --- > kernel-shark-qt/src/KsTraceGraph.cpp | 10 +++++----- > python/event-viewer.py | 4 ++-- > python/tracecmd.py | 2 +- > 3 files changed, 8 insertions(+), 8 deletions(-) > > diff --git a/kernel-shark-qt/src/KsTraceGraph.cpp b/kernel-shark-qt/src/KsTraceGraph.cpp > index 462918f8a37a..844c79494e9b 100644 > --- a/kernel-shark-qt/src/KsTraceGraph.cpp > +++ b/kernel-shark-qt/src/KsTraceGraph.cpp > @@ -279,7 +279,7 @@ void KsTraceGraph::_resetPointer(uint64_t ts, int cpu, int pid) > QString pointer; > > kshark_convert_nano(ts, &sec, &usec); > - pointer.sprintf("%lu.%lu", sec, usec); > + pointer.sprintf("%lu.%06lu", sec, usec); > _labelP2.setText(pointer); > > if (pid > 0 && cpu >= 0) { > @@ -315,7 +315,7 @@ void KsTraceGraph::_setPointerInfo(size_t i) > uint64_t sec, usec; > > kshark_convert_nano(e->ts, &sec, &usec); > - pointer.sprintf("%lu.%lu", sec, usec); > + pointer.sprintf("%lu.%06lu", sec, usec); > _labelP2.setText(pointer); > > comm.append("-"); > @@ -601,17 +601,17 @@ void KsTraceGraph::_updateTimeLegends() > QString tMin, tMid, tMax; > > kshark_convert_nano(_glWindow.model()->histo()->min, &sec, &usec); > - tMin.sprintf("%lu.%lu", sec, usec); > + tMin.sprintf("%lu.%06lu", sec, usec); > _labelXMin.setText(tMin); > > tsMid = (_glWindow.model()->histo()->min + > _glWindow.model()->histo()->max) / 2; > kshark_convert_nano(tsMid, &sec, &usec); > - tMid.sprintf("%lu.%lu", sec, usec); > + tMid.sprintf("%lu.%06lu", sec, usec); > _labelXMid.setText(tMid); > > kshark_convert_nano(_glWindow.model()->histo()->max, &sec, &usec); > - tMax.sprintf("%lu.%lu", sec, usec); > + tMax.sprintf("%lu.%06lu", sec, usec); > _labelXMax.setText(tMax); > } > > diff --git a/python/event-viewer.py b/python/event-viewer.py > index 651761a083c9..3a082b81fc3d 100755 > --- a/python/event-viewer.py > +++ b/python/event-viewer.py > @@ -203,8 +203,8 @@ class EventView(gtk.TreeView): > return False > > if data == "ts": > - cell.set_property("markup", "%d.%d" % (ev.ts/1000000000, > - ev.ts%1000000000)) > + cell.set_property("markup", "%d.%09d" % (ev.ts/1000000000, > + ev.ts%1000000000)) > data_func_cnt = data_func_cnt + 1 > if app: > app.inc_data_func() > diff --git a/python/tracecmd.py b/python/tracecmd.py > index 1267941aa388..f551a1489e25 100644 > --- a/python/tracecmd.py > +++ b/python/tracecmd.py > @@ -62,7 +62,7 @@ class Event(object, DictMixin): > self._format = format > > def __str__(self): > - return "%d.%d CPU%d %s: pid=%d comm=%s type=%d" % \ > + return "%d.%09d CPU%d %s: pid=%d comm=%s type=%d" % \ > (self.ts/1000000000, self.ts%1000000000, self.cpu, self.name, > self.num_field("common_pid"), self.comm, self.type) >
On 10.01.19 г. 23:48 ч., Steven Rostedt wrote: > On Thu, 10 Jan 2019 22:43:46 +0100 > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > >> When a timestamp that represents 1.00001 is converted to (for example) >> seconds and microseconds this results in seconds=1 and useconds=10. To >> format that into a string requires the format %lu.%06lu to yield >> "1.000010". Some code parts used %lu.%lu however which results in "1.10" >> which is wrong. > > Thanks Uwe! > > Yordan, can you review this patch and give an Acked-by or Reviewed-by > tag? > Very well spotted. Reviewed-by: Yordan Karadzhov <ykaradzhov@vmware.com> Just one minor thing, please sign the patch. Thanks a lot! Yordan > Thanks! > > -- Steve > >> >> This was noticed because I failed to find a significant location in a trace >> provided by a colleague as the reported timestamp was wrong :-| >> >> This change fixes all occurrences of this problem found by >> $(grep -E '\.%[^0s]'). >> --- >> kernel-shark-qt/src/KsTraceGraph.cpp | 10 +++++----- >> python/event-viewer.py | 4 ++-- >> python/tracecmd.py | 2 +- >> 3 files changed, 8 insertions(+), 8 deletions(-) >> >> diff --git a/kernel-shark-qt/src/KsTraceGraph.cpp b/kernel-shark-qt/src/KsTraceGraph.cpp >> index 462918f8a37a..844c79494e9b 100644 >> --- a/kernel-shark-qt/src/KsTraceGraph.cpp >> +++ b/kernel-shark-qt/src/KsTraceGraph.cpp >> @@ -279,7 +279,7 @@ void KsTraceGraph::_resetPointer(uint64_t ts, int cpu, int pid) >> QString pointer; >> >> kshark_convert_nano(ts, &sec, &usec); >> - pointer.sprintf("%lu.%lu", sec, usec); >> + pointer.sprintf("%lu.%06lu", sec, usec); >> _labelP2.setText(pointer); >> >> if (pid > 0 && cpu >= 0) { >> @@ -315,7 +315,7 @@ void KsTraceGraph::_setPointerInfo(size_t i) >> uint64_t sec, usec; >> >> kshark_convert_nano(e->ts, &sec, &usec); >> - pointer.sprintf("%lu.%lu", sec, usec); >> + pointer.sprintf("%lu.%06lu", sec, usec); >> _labelP2.setText(pointer); >> >> comm.append("-"); >> @@ -601,17 +601,17 @@ void KsTraceGraph::_updateTimeLegends() >> QString tMin, tMid, tMax; >> >> kshark_convert_nano(_glWindow.model()->histo()->min, &sec, &usec); >> - tMin.sprintf("%lu.%lu", sec, usec); >> + tMin.sprintf("%lu.%06lu", sec, usec); >> _labelXMin.setText(tMin); >> >> tsMid = (_glWindow.model()->histo()->min + >> _glWindow.model()->histo()->max) / 2; >> kshark_convert_nano(tsMid, &sec, &usec); >> - tMid.sprintf("%lu.%lu", sec, usec); >> + tMid.sprintf("%lu.%06lu", sec, usec); >> _labelXMid.setText(tMid); >> >> kshark_convert_nano(_glWindow.model()->histo()->max, &sec, &usec); >> - tMax.sprintf("%lu.%lu", sec, usec); >> + tMax.sprintf("%lu.%06lu", sec, usec); >> _labelXMax.setText(tMax); >> } >> >> diff --git a/python/event-viewer.py b/python/event-viewer.py >> index 651761a083c9..3a082b81fc3d 100755 >> --- a/python/event-viewer.py >> +++ b/python/event-viewer.py >> @@ -203,8 +203,8 @@ class EventView(gtk.TreeView): >> return False >> >> if data == "ts": >> - cell.set_property("markup", "%d.%d" % (ev.ts/1000000000, >> - ev.ts%1000000000)) >> + cell.set_property("markup", "%d.%09d" % (ev.ts/1000000000, >> + ev.ts%1000000000)) >> data_func_cnt = data_func_cnt + 1 >> if app: >> app.inc_data_func() >> diff --git a/python/tracecmd.py b/python/tracecmd.py >> index 1267941aa388..f551a1489e25 100644 >> --- a/python/tracecmd.py >> +++ b/python/tracecmd.py >> @@ -62,7 +62,7 @@ class Event(object, DictMixin): >> self._format = format >> >> def __str__(self): >> - return "%d.%d CPU%d %s: pid=%d comm=%s type=%d" % \ >> + return "%d.%09d CPU%d %s: pid=%d comm=%s type=%d" % \ >> (self.ts/1000000000, self.ts%1000000000, self.cpu, self.name, >> self.num_field("common_pid"), self.comm, self.type) >> >
Hello Yordan, On Fri, Jan 11, 2019 at 09:09:19AM +0000, Yordan Karadzhov wrote: > On 10.01.19 г. 23:48 ч., Steven Rostedt wrote: > > On Thu, 10 Jan 2019 22:43:46 +0100 > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > > >> When a timestamp that represents 1.00001 is converted to (for example) > >> seconds and microseconds this results in seconds=1 and useconds=10. To > >> format that into a string requires the format %lu.%06lu to yield > >> "1.000010". Some code parts used %lu.%lu however which results in "1.10" > >> which is wrong. > > > > Thanks Uwe! > > > > Yordan, can you review this patch and give an Acked-by or Reviewed-by > > tag? > > Very well spotted. > > Reviewed-by: Yordan Karadzhov <ykaradzhov@vmware.com> > > Just one minor thing, please sign the patch. It was a conscious decision to not sign-off the patch as it isn't formalized for trace-cmd what a Signed-off-by-line would mean. So if you insist on this formalism, I suggest you formalize it first :-) Best regards Uwe
On Fri, 11 Jan 2019 10:24:20 +0100 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > Hello Yordan, > > On Fri, Jan 11, 2019 at 09:09:19AM +0000, Yordan Karadzhov wrote: > > On 10.01.19 г. 23:48 ч., Steven Rostedt wrote: > > > On Thu, 10 Jan 2019 22:43:46 +0100 > > > Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > > > > >> When a timestamp that represents 1.00001 is converted to (for example) > > >> seconds and microseconds this results in seconds=1 and useconds=10. To > > >> format that into a string requires the format %lu.%06lu to yield > > >> "1.000010". Some code parts used %lu.%lu however which results in "1.10" > > >> which is wrong. > > > > > > Thanks Uwe! > > > > > > Yordan, can you review this patch and give an Acked-by or Reviewed-by > > > tag? > > > > Very well spotted. > > > > Reviewed-by: Yordan Karadzhov <ykaradzhov@vmware.com> > > > > Just one minor thing, please sign the patch. Nice catch. > > It was a conscious decision to not sign-off the patch as it isn't > formalized for trace-cmd what a Signed-off-by-line would mean. > > So if you insist on this formalism, I suggest you formalize it first :-) You're the first one to mention this, and I never even thought about it. It means the same thing as a Signed-off-by means for the Linux kernel. I can copy the text of the DCO and place it into a file called DCO if that makes you feel better. Could you resend with a Signed-off-by and I'll add your patch along with the file with the text below. Take this email as an official declaration of the sign off meaning. -- Steve (Copied from the Linux Kernel's Documentation/process/submitting-patches.rst) Sign your work - the Developer's Certificate of Origin ------------------------------------------------------ To improve tracking of who did what, especially with patches that can percolate to their final resting place in the kernel through several layers of maintainers, we've introduced a "sign-off" procedure on patches that are being emailed around. The sign-off is a simple line at the end of the explanation for the patch, which certifies that you wrote it or otherwise have the right to pass it on as an open-source patch. The rules are pretty simple: if you can certify the below: Developer's Certificate of Origin 1.1 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ By making a contribution to this project, I certify that: (a) The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or (b) The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or (c) The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it. (d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved. then you just add a line saying:: Signed-off-by: Random J Developer <random@developer.example.org> using your real name (sorry, no pseudonyms or anonymous contributions.) Some people also put extra tags at the end. They'll just be ignored for now, but you can do this to mark internal company procedures or just point out some special detail about the sign-off.
diff --git a/kernel-shark-qt/src/KsTraceGraph.cpp b/kernel-shark-qt/src/KsTraceGraph.cpp index 462918f8a37a..844c79494e9b 100644 --- a/kernel-shark-qt/src/KsTraceGraph.cpp +++ b/kernel-shark-qt/src/KsTraceGraph.cpp @@ -279,7 +279,7 @@ void KsTraceGraph::_resetPointer(uint64_t ts, int cpu, int pid) QString pointer; kshark_convert_nano(ts, &sec, &usec); - pointer.sprintf("%lu.%lu", sec, usec); + pointer.sprintf("%lu.%06lu", sec, usec); _labelP2.setText(pointer); if (pid > 0 && cpu >= 0) { @@ -315,7 +315,7 @@ void KsTraceGraph::_setPointerInfo(size_t i) uint64_t sec, usec; kshark_convert_nano(e->ts, &sec, &usec); - pointer.sprintf("%lu.%lu", sec, usec); + pointer.sprintf("%lu.%06lu", sec, usec); _labelP2.setText(pointer); comm.append("-"); @@ -601,17 +601,17 @@ void KsTraceGraph::_updateTimeLegends() QString tMin, tMid, tMax; kshark_convert_nano(_glWindow.model()->histo()->min, &sec, &usec); - tMin.sprintf("%lu.%lu", sec, usec); + tMin.sprintf("%lu.%06lu", sec, usec); _labelXMin.setText(tMin); tsMid = (_glWindow.model()->histo()->min + _glWindow.model()->histo()->max) / 2; kshark_convert_nano(tsMid, &sec, &usec); - tMid.sprintf("%lu.%lu", sec, usec); + tMid.sprintf("%lu.%06lu", sec, usec); _labelXMid.setText(tMid); kshark_convert_nano(_glWindow.model()->histo()->max, &sec, &usec); - tMax.sprintf("%lu.%lu", sec, usec); + tMax.sprintf("%lu.%06lu", sec, usec); _labelXMax.setText(tMax); } diff --git a/python/event-viewer.py b/python/event-viewer.py index 651761a083c9..3a082b81fc3d 100755 --- a/python/event-viewer.py +++ b/python/event-viewer.py @@ -203,8 +203,8 @@ class EventView(gtk.TreeView): return False if data == "ts": - cell.set_property("markup", "%d.%d" % (ev.ts/1000000000, - ev.ts%1000000000)) + cell.set_property("markup", "%d.%09d" % (ev.ts/1000000000, + ev.ts%1000000000)) data_func_cnt = data_func_cnt + 1 if app: app.inc_data_func() diff --git a/python/tracecmd.py b/python/tracecmd.py index 1267941aa388..f551a1489e25 100644 --- a/python/tracecmd.py +++ b/python/tracecmd.py @@ -62,7 +62,7 @@ class Event(object, DictMixin): self._format = format def __str__(self): - return "%d.%d CPU%d %s: pid=%d comm=%s type=%d" % \ + return "%d.%09d CPU%d %s: pid=%d comm=%s type=%d" % \ (self.ts/1000000000, self.ts%1000000000, self.cpu, self.name, self.num_field("common_pid"), self.comm, self.type)