mtr icon indicating copy to clipboard operation
mtr copied to clipboard

Max 99,999 probes but counter doesn't loop properly

Open leo-brown opened this issue 8 years ago • 5 comments

The SENT counter loops from 99,999 back to 10,000. See screenshot (and thanks for MTR!) screen shot 2017-01-21 at 14 02 52

leo-brown avatar Jan 21 '17 14:01 leo-brown

This is a display bug, it's being truncated. I suspect 10 more probes, you'll see "10001" which is "10001x", and then 10 later "10002x".

ISTR I had this problem when I was leaving my mtr running for days and days, and I'd toyed with widening the width of those columns, but my weak-ass C++ skills did not allow me to fix it properly and so that change never got accepted.

On Sat, Jan 21, 2017 at 2:05 PM, netfuse [email protected] wrote:

The SENT counter loops from 99,999 back to 10,000. See screenshot (and thanks for MTR!) [image: screen shot 2017-01-21 at 14 02 52] https://cloud.githubusercontent.com/assets/1903144/22174952/b35e3ebe-dfe2-11e6-8c1b-16d420744235.png

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/traviscross/mtr/issues/185, or mute the thread https://github.com/notifications/unsubscribe-auth/AAO1QEqtX-irT4Gnig5_iAZagk0tsm7Bks5rUhC_gaJpZM4LqE1r .

dballing avatar Jan 21 '17 14:01 dballing

Interestingly, it's not truncated. I still have the same trace running, and the RHS digit is incrementing correctly at 1Hz. It's obviously not a major problem but I figured 20 years into MTR's development small bugs like this are welcomed.

image

leo-brown avatar Jan 21 '17 14:01 leo-brown

The proper way to fix it is not to use any "%d" like format, but to use a integer-to-pretty-printed-ascii. With 5 places of space, we can reasonably accurately go up to 99999, and then roll around to things like 100k 101k .... 999k 1.00M 1.01M....

I remember writing such a pretty-printer for a special case recently. In any case, it is easy.

write something like

switch (numplaces) {
   case 5: if (val < 100000) {sprintf (buf, "%5d", val);return buf;}
                else if (val < 999500) {sprintf (buf, "%3dk", val/1000); return buf;}
                ....
 

Test it by calling it from a command line program and throwing a bunch of bordercases at it.

I would accept the patch that includes this, even when it only implements pretty printing for 5 places for now.

rewolff avatar Jan 22 '17 12:01 rewolff

the RHS digit is incrementing correctly at 1Hz

Interesting. I think mine (v0.94) is incrementing at 0.01 Hz, suggesting it's lost two digits off the right, regardless of how wide my terminal is. The screenshot seems to be from a Mac, whereas I'm on Linux. If the source is https://github.com/traviscross/mtr/blob/e137a71a3c9e90d3c827b70ca72b2517865e518e/ui/mtr.c#L73 then https://pubs.opengroup.org/onlinepubs/9699919799/functions/printf.html doesn't afford libc any latitude in truncation ("In no case shall a nonexistent or small field width cause truncation of a field"). Nah, just stomps the back of it at https://github.com/traviscross/mtr/blob/master/ui/curses.c#L459 with the following " %5.1f" innit, though I'm seeing one more space between Snt and Last than I can explain.

martindorey avatar Mar 19 '24 02:03 martindorey

Ah:

o str set the columns to display, default str='LRS N BAWV'

... perhaps adds an extra space between S for Snt and N for (Newest for) Last.

martindorey avatar Mar 19 '24 02:03 martindorey