search

CyberShadow / dhcptest

Cross-platform DHCP test client
https://blog.cy.md/2013/01/10/dhcp-test-client
363 stars 57 forks source link

Add option --queryall #4

Closed DarrenWhite99 closed 9 years ago

DarrenWhite99 commented 9 years ago

Adding option --queryall which will print all DHCP responses until the timeout has elapsed instead of exiting after the first response is received. I created this modification to support performing on-demand DHCP tests and determining if multiple DHCP servers are active.

DarrenWhite99 commented 9 years ago

I have no preference, that would be a minor tweak to my patch and I think it is logical. Would you like me to code that change and update my branch for you to pull from?

DarrenWhite99 commented 9 years ago

I went ahead and modified my branch to support "--wait" instead of "--queryall". If it appears clean to you, pull it over.

CyberShadow commented 9 years ago

Nits:

Looks good, fix the nits or not (I'll merge and fix them later)

DarrenWhite99 commented 9 years ago

I'm adjusting them now. Slightly unrelated, but the default query behavior is to send 1 packet and wait forever. Does that really make sense? If the request was missed, there will never be a reply and the program will never exit. It would seem that in --query mode it would be better to set a timeout beyond any reasonable expectation of a response, but still allow the program to exit and report a failure. 10 seconds is a reasonable expectation for response and makes sense with multiple replies. How about a timeout of 60 seconds instead of "forever" when only trying once I don't believe the "torever" timeout is documented anywhere, and it seems unusual that anyone would want that behavior. Specifying a timeout of "0" could equal forever if anyone really wanted that.,,

DarrenWhite99 commented 9 years ago

I think the --wait code is ready. Use of --wait with --query is now enforced and the documentation now covers --wait. I didn't make any changes to the "forever" timeout

CyberShadow commented 9 years ago

Yes, that makes sense. I think the "forever" default is a compatibility remnant from before --timeout.

CyberShadow commented 9 years ago

Documentation still mentions --queryall.

DarrenWhite99 commented 9 years ago

Arrgh! :) Looks like I didn't update that file in previous commit. I thought I had the Readme.md selected in the prior commit... Should be good now.