ping: Add units to SIGINFO's statistics

[jlduran_gmail.com]

Summary

Printing numbers without units may not be of much help.
Also mention the fact that these times are round-trip.
Before:

2/2 packets received (100.0%) 0.068 min / 0.118 avg / 0.168 max

After:

2/2 packets received (100.0%) round-trip min/avg/max = 0.068/0.118/0.168 ms

Test Plan

Should we drop the void casts?

[asomers]

I like it. And yes I think you should add stddev to match the regular program output at termination.

[jlduran_gmail.com]

In D38485#875773, @asomers wrote:

I like it. And yes I think you should add stddev to match the regular program output at termination.

OK, will update the revision. OpenBSD reuses the same function, I'll try to come up with something similar.

[jlduran_gmail.com]

Address suggestions:
In this first iteration, simply duplicate the code from the final statistics section into SIGINFO's.

• Avoid a potential square root of a negative number calculation (based on an alternative to D38480, this change should not part of this revision)

I have a branch where it unifies the code into a single summary() function, however it changes a lot more of the code that I would like/have tests for at the moment. Besides, the code ends up being almost identical to ping6.c's. Let's leave the unification for later in the ping/ping6 series.
A few cosmetic things discovered from this exercise are:

• ping's SIGINFO prints out to stderr (ping6 to stdout)
• ping/ping6 should just become ping when displayed to the user
• ping uses the expression "somebody's printing up packets!", I prefer ping6's "somebody's duplicating packets!" when a duplicate packet is received
• ping calls the population standard deviation "stddev", while ping6 calls it "std-dev". I'm OK either way, but it should be the same for both

[markj]

In D38485#877727, @jlduran_gmail.com wrote:

Address suggestions:
In this first iteration, simply duplicate the code from the final statistics section into SIGINFO's.

I think this can be committed as-is. Is there a review up to unify them?

• Avoid a potential square root of a negative number calculation (based on an alternative to D38480, this change should not part of this revision)

I have a branch where it unifies the code into a single summary() function, however it changes a lot more of the code that I would like/have tests for at the moment. Besides, the code ends up being almost identical to ping6.c's. Let's leave the unification for later in the ping/ping6 series.
A few cosmetic things discovered from this exercise are:

• ping's SIGINFO prints out to stderr (ping6 to stdout)

It should be stderr.

• ping/ping6 should just become ping when displayed to the user

I agree.

• ping uses the expression "somebody's printing up packets!", I prefer ping6's "somebody's duplicating packets!" when a duplicate packet is received

I agree.

• ping calls the population standard deviation "stddev", while ping6 calls it "std-dev". I'm OK either way, but it should be the same for both

"stddev" seems fine to me.

[jlduran_gmail.com]

In D38485#888201, @markj wrote:

In D38485#877727, @jlduran_gmail.com wrote:

Address suggestions:
In this first iteration, simply duplicate the code from the final statistics section into SIGINFO's.

I think this can be committed as-is. Is there a review up to unify them?

I was planning on leaving that unification for last, but let me try to at least unify and test this user-facing part.
Maybe I should put all shared code in sbin/ping/utils.c... I'll work on that. Thank you!

[markj]

In D38485#888216, @jlduran_gmail.com wrote:

In D38485#888201, @markj wrote:

In D38485#877727, @jlduran_gmail.com wrote:

Address suggestions:
In this first iteration, simply duplicate the code from the final statistics section into SIGINFO's.

I think this can be committed as-is. Is there a review up to unify them?

I was planning on leaving that unification for last, but let me try to at least unify and test this user-facing part.
Maybe I should put all shared code in sbin/ping/utils.c... I'll work on that. Thank you!

Will you update this review with the result? I'll hold off on committing in that case.

[jlduran_gmail.com]

In D38485#888217, @markj wrote:

In D38485#888216, @jlduran_gmail.com wrote:

In D38485#888201, @markj wrote:

In D38485#877727, @jlduran_gmail.com wrote:

Address suggestions:
In this first iteration, simply duplicate the code from the final statistics section into SIGINFO's.

I think this can be committed as-is. Is there a review up to unify them?

I was planning on leaving that unification for last, but let me try to at least unify and test this user-facing part.
Maybe I should put all shared code in sbin/ping/utils.c... I'll work on that. Thank you!

Will you update this review with the result? I'll hold off on committing in that case.

Yes, but probably not today (the tests may take some time).

[jlduran_gmail.com]

In D38485#888217, @markj wrote:

In D38485#888216, @jlduran_gmail.com wrote:

In D38485#888201, @markj wrote:

In D38485#877727, @jlduran_gmail.com wrote:

Address suggestions:
In this first iteration, simply duplicate the code from the final statistics section into SIGINFO's.

I think this can be committed as-is. Is there a review up to unify them?

I was planning on leaving that unification for last, but let me try to at least unify and test this user-facing part.
Maybe I should put all shared code in sbin/ping/utils.c... I'll work on that. Thank you!

Will you update this review with the result? I'll hold off on committing in that case.

Actually, I'll be submitting a separate review, it's a long diff.
I placed the unified code into main.c, as more elements are unified, they'll also be placed in main.c?
I'll wait over the weekend to let these changes soak in, before formally creating the review. But, if you want to take a preliminary look:
https://github.com/jlduran/freebsd-src/tree/ping-interrupts-ping6-unification

[jlduran_gmail.com]

In D38485#888290, @jlduran_gmail.com wrote:

In D38485#888217, @markj wrote:

In D38485#888216, @jlduran_gmail.com wrote:

In D38485#888201, @markj wrote:

In D38485#877727, @jlduran_gmail.com wrote:

Address suggestions:
In this first iteration, simply duplicate the code from the final statistics section into SIGINFO's.

I think this can be committed as-is. Is there a review up to unify them?

I was planning on leaving that unification for last, but let me try to at least unify and test this user-facing part.
Maybe I should put all shared code in sbin/ping/utils.c... I'll work on that. Thank you!

Will you update this review with the result? I'll hold off on committing in that case.

Actually, I'll be submitting a separate review, it's a long diff.
I placed the unified code into main.c, as more elements are unified, they'll also be placed in main.c?
I'll wait over the weekend to let these changes soak in, before formally creating the review. But, if you want to take a preliminary look:
https://github.com/jlduran/freebsd-src/tree/ping-interrupts-ping6-unification

The code has been rebased on GitHub, and the proposal of this diff unified has been submitted as D39126.
Thank you!

[markj]

In D38485#890556, @jlduran_gmail.com wrote:

In D38485#888290, @jlduran_gmail.com wrote:

In D38485#888217, @markj wrote:

In D38485#888216, @jlduran_gmail.com wrote:

In D38485#888201, @markj wrote:

In D38485#877727, @jlduran_gmail.com wrote:

Address suggestions:
In this first iteration, simply duplicate the code from the final statistics section into SIGINFO's.

I think this can be committed as-is. Is there a review up to unify them?

I was planning on leaving that unification for last, but let me try to at least unify and test this user-facing part.
Maybe I should put all shared code in sbin/ping/utils.c... I'll work on that. Thank you!

Will you update this review with the result? I'll hold off on committing in that case.

Actually, I'll be submitting a separate review, it's a long diff.
I placed the unified code into main.c, as more elements are unified, they'll also be placed in main.c?
I'll wait over the weekend to let these changes soak in, before formally creating the review. But, if you want to take a preliminary look:
https://github.com/jlduran/freebsd-src/tree/ping-interrupts-ping6-unification

The code has been rebased on GitHub, and the proposal of this diff unified has been submitted as D39126.
Thank you!

Do I understand correctly that this review is now obsolete in favour of D39126?

[jlduran_gmail.com]

In D38485#891006, @markj wrote:

In D38485#890556, @jlduran_gmail.com wrote:

In D38485#888290, @jlduran_gmail.com wrote:

In D38485#888217, @markj wrote:

In D38485#888216, @jlduran_gmail.com wrote:

In D38485#888201, @markj wrote:

In D38485#877727, @jlduran_gmail.com wrote:

Address suggestions:
In this first iteration, simply duplicate the code from the final statistics section into SIGINFO's.

I think this can be committed as-is. Is there a review up to unify them?

I was planning on leaving that unification for last, but let me try to at least unify and test this user-facing part.
Maybe I should put all shared code in sbin/ping/utils.c... I'll work on that. Thank you!

Will you update this review with the result? I'll hold off on committing in that case.

Actually, I'll be submitting a separate review, it's a long diff.
I placed the unified code into main.c, as more elements are unified, they'll also be placed in main.c?
I'll wait over the weekend to let these changes soak in, before formally creating the review. But, if you want to take a preliminary look:
https://github.com/jlduran/freebsd-src/tree/ping-interrupts-ping6-unification

The code has been rebased on GitHub, and the proposal of this diff unified has been submitted as D39126.
Thank you!

Do I understand correctly that this review is now obsolete in favour of D39126?

Yes. If there are no objections, I'll abandon this revision once D39126 lands.