Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print returned TOS value #335

Merged
merged 1 commit into from
Jul 28, 2024
Merged

Print returned TOS value #335

merged 1 commit into from
Jul 28, 2024

Conversation

gsnw
Copy link
Contributor

@gsnw gsnw commented Jul 14, 2024

Added tos to the header again, but only for IPv4.
IPv6 uses ds field here
Currently, TOS is only output in one place as in the ticket example.

Issue #68

@gsnw gsnw force-pushed the issue/68 branch 2 times, most recently from 093bae5 to 5d55aa8 Compare July 15, 2024 18:40
@coveralls
Copy link

coveralls commented Jul 15, 2024

Coverage Status

coverage: 85.512% (+0.05%) from 85.46%
when pulling 3798851 on gsnw:issue/68
into 8ecda7f on schweikert:develop.

src/fping.c Outdated Show resolved Hide resolved
@auerswal
Copy link
Collaborator

auerswal commented Jul 21, 2024

I like the general approach of using an option to enable printing of received TOS values. I also like to approach of starting with the default output (<target> is alive (TOS <value>)).

@gsnw
Copy link
Contributor Author

gsnw commented Jul 21, 2024

I have changed the logic a bit and --print-tos can be used independently of the -O parameter.
However, it is only displayed if the value is present in the IP header.

Copy link
Collaborator

@auerswal auerswal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please adjust the documentation (man page and help output), and adjust the code to the changes by my just merged pull request #336.

doc/fping.pod Outdated Show resolved Hide resolved
src/fping.c Show resolved Hide resolved
src/fping.c Show resolved Hide resolved
src/fping.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@auerswal auerswal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Looks good to me!

@gsnw-sebast
Copy link
Collaborator

Perfect, can you merge that please? I made a few changes to my github account and lost my permissions as a result

@auerswal auerswal merged commit 04950d2 into schweikert:develop Jul 28, 2024
9 checks passed
@gsnw-sebast gsnw-sebast deleted the issue/68 branch July 31, 2024 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants