Page MenuHomeFreeBSD

ping: Remove vestigial code
ClosedPublic

Authored by jlduran on Feb 9 2023, 11:16 PM.
Tags
None
Referenced Files
F102430396: D38475.diff
Tue, Nov 12, 4:18 AM
Unknown Object (File)
Fri, Nov 8, 10:25 PM
Unknown Object (File)
Sat, Nov 2, 6:05 PM
Unknown Object (File)
Sep 19 2024, 8:04 PM
Unknown Object (File)
Sep 16 2024, 7:47 AM
Unknown Object (File)
Sep 2 2024, 4:19 PM
Unknown Object (File)
Aug 21 2024, 3:25 PM
Unknown Object (File)
Aug 21 2024, 3:25 PM

Details

Summary

Ping used to provide some sort of packet sniffing capabilities, this was in an era where hubs were used and tcpdump wasn't invented.
pr_iph() is a function that prints the IP header of the packet.
pr_retip() is essentially a wrapper function to pr_iph(), that also displays the source and destination ports of a TCP or UDP packet.
After ef9e6dc7eebe9830511602904d3ef5218d964080 some of this functionality was almost removed, to only display packets sent by us (26+ years ago).
At this point, reaching this code path was only possible by doctoring the original packet.
After 46d7b45a267b3d78c5054b210ff7b6c55bfca42b this code path can never be reached.
Remove the code.
Note that this essentially turns pr_retip() into pr_iph().

Test Plan

DISCLAIMER: My preferred route would be to implement D38431, however for reasons detailed in that review, we'll resort to fixing what we currently have.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

sbin/ping/ping.c
1723–1724

Why do we need this function at all?

sbin/ping/ping.c
1723–1724

Precisely! From the summary:

Note that this essentially turns pr_retip() into pr_iph().

Let me know if the unification should go on a separate commit or squash it with this one?

sbin/ping/ping.c
1723–1724

I think you should just squash it into this one.

jlduran marked an inline comment as not done.Feb 10 2023, 3:54 PM
jlduran added inline comments.
sbin/ping/ping.c
1723–1724

Will do!

jlduran marked 2 inline comments as done.
  • Remove pr_retip()
This revision is now accepted and ready to land.Feb 13 2023, 2:41 PM
This revision was automatically updated to reflect the committed changes.