traviscross / mtr

Official repository for mtr, a network diagnostic tool
http://www.bitwizard.nl/mtr/
GNU General Public License v2.0
2.65k stars 338 forks source link

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

Open leo-brown opened 7 years ago

leo-brown commented 7 years ago

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
dballing commented 7 years ago

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 notifications@github.com 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 .

leo-brown commented 7 years ago

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

rewolff commented 7 years ago

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.

martindorey commented 6 months ago

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 commented 6 months ago

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.