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

Ping 'count' param throws error for large values, and is signed. #87

Open
dabell-cc opened this issue Aug 30, 2022 · 5 comments
Open

Ping 'count' param throws error for large values, and is signed. #87

dabell-cc opened this issue Aug 30, 2022 · 5 comments

Comments

@dabell-cc
Copy link
Contributor

Describe the bug
The ping() function can't be called with a large value for count. It will throw an error.
Setting count to a negative number does not result in an error (but it should).
Seems the count variable is ended up a signed 16-bit int for some reason.

To Reproduce
Steps to reproduce the behavior:
Large value error:

from pythonping import ping
ping('127.0.0.1', count=100000)
result: struct.error: short format requires (-32768) <= number <= 32767

Accept negative number:

from pythonping import ping
ping('127.0.0.1', count=-10)
result: Round Trip Times min/avg/max is 0/0/0 ms

Expected behavior
Expect to be able to set count to a positive int 0 .. sys.maxsize.
Large ping counts are helpful for long ping-flood tests to check for networking stability.

Expect error when setting count to a negative value.

Desktop (please complete the following information):

  • OS: Windows 10
  • Browser [e.g. chrome, safari]: n/a
  • Version [e.g. 22]: python 3.10.6
@BoboTiG
Copy link

BoboTiG commented Sep 7, 2022

It would be cool to have infinite ping using count=None or count=-1.

@z4id
Copy link

z4id commented Sep 12, 2022

Integer (int) has no max limit in Python3.

import sys

sys.maxsize
# 9223372036854775807

And sys.maxsize is not the max value of int, Python3's int can handle any large number as long as memory can handle it.


Not reproduce-able

ping('127.0.0.1', count=100000)
result: struct.error: short format requires (-32768) <= number <= 32767

  • Python 3.10.4
  • pythonping==1.1.3
  • MacOS 12.4

Agreed

ping('127.0.0.1', count=-10)
result: Round Trip Times min/avg/max is 0/0/0 ms

Agreed on this. For negative numbers, it should throw an error instead of just displaying default Round Trip Times min/avg/max is 0/0/0 ms

Expected Behaviour:
ValueError:count must be greater than or equal to 0

Handling float count values

Float values should be rounded off round(count) instead of math.ceil(count)

from pythonping import ping
ping('127.0.0.1', count=2.1)
Current Behaviour:
Reply from 127.0.0.1, 29 bytes in 0.03ms
Reply from 127.0.0.1, 29 bytes in 0.03ms
Reply from 127.0.0.1, 29 bytes in 0.02ms

Round Trip Times min/avg/max is 0.02/0.03/0.03 ms
Expected Behaviour:
Reply from 127.0.0.1, 29 bytes in 0.03ms
Reply from 127.0.0.1, 29 bytes in 0.03ms

Round Trip Times min/avg/max is 0.02/0.03/0.03 ms

@pythonping_owner or @pythonping_contributors do let me know if these changes align with project contribution guideline, so that I can raise a PR to handle these edge cases.

@dabell-cc
Copy link
Contributor Author

@z4id regarding sys.maxsize: fair enough, no need to set an upper limit.
Interesting that it works on Mac with python 3.10.4, I wanted to add I also used pythonping==1.1.3.

The PR I opened here did solve the large number and negative number issue for me - perhaps you could take a look and try it out on your system.

@z4id
Copy link

z4id commented Sep 13, 2022

The PR I opened #89 did solve the large number and negative number issue for me - perhaps you could take a look and try it out on your system.

@dabell-cc, that's great. I'll have a look on it.
Glad it's been actively followed.

@alessandromaggio
Copy link
Owner

I will try to review this ASAP

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

No branches or pull requests

4 participants