[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [bug-inetutils] Added new option --timeout-response to ping/ping6
From: |
Alfred M. Szmidt |
Subject: |
Re: [bug-inetutils] Added new option --timeout-response to ping/ping6 |
Date: |
Thu, 22 Oct 2009 06:01:51 -0400 |
Alfred (or anyone interested :), may you review this one ?
Please format your code according to the GNU Coding Standards (spaces
between arithmetic operators, curlies on seperate lines and indented
accordingly, etc).
+2009-10-22 Rakesh Pandit <address@hidden>
+
+ * ping/ping_common.c (ping_set_timer): New function.
+ * ping/ping_common.h (ping_set_timer): New prototype.
+ * ping/ping6.c (argp_options): New option --timeout-response.
+ (parse_opt): New option --timeout-response and check for edge
+ values.
Seeing that it is a new option, there is no need to explain what is
being checked for. For parse_opt, I'd write `Handle
--timeout-response', since the option is already mentioned as new, you
can't add two new options of the same name...
There is no entry for what you did in ping.c.
+ (ping_run): Set timer for response timeout via ping_set_timer and
+ corresponding signal for SIGALRM.
I'd write: Set signal action to SIGALRM for `sig_int', and handle
--timeout-response.
diff --git a/doc/inetutils.texi b/doc/inetutils.texi
index aba0c28..235505c 100644
--- a/doc/inetutils.texi
+++ b/doc/inetutils.texi
@@ -474,6 +474,12 @@ the 8 bytes of ICMP header data.
@opindex -w
@opindex --timeout
Stop after @var{n} seconds.
+
address@hidden -W @var{n}
address@hidden address@hidden
address@hidden -W
address@hidden --timeout-response
+Wait @var{n} seconds for response.
What happens after N seconds? That should be mentioned.
@@ -182,6 +184,15 @@ parse_opt (int key, char *arg, struct argp_state *state)
timeout = ping_cvt_number (arg, INT_MAX, 0);
break;
+ case 'W':
+ timeout_response = strtoul (arg, &endptr, 0);
+ if (*endptr || timeout_response < 0
+ || timeout_response > INT_MAX/1000000) {
+ error (EXIT_FAILURE, 0, "invalid response timeout value (%s)", arg);
GNU style error messages should be of the form `value: explanation',
so the above should be changed to `%s: invalid response timeout
value'. E.g. `filename: no such file or directory'.
Is -W a standard option?
+ }
+ timeout_response *= 1000000;
+ break;
There should be a small comment stating what this number means, maybe
make it a macro since its value is used in a few places.
diff --git a/ping/ping6.c b/ping/ping6.c
index a65b875..5924796 100644
--- a/ping/ping6.c
+++ b/ping/ping6.c
@@ -55,6 +55,7 @@ size_t count = DEFAULT_PING_COUNT;
size_t interval;
int socket_type;
int timeout = -1;
+unsigned long timeout_response = -1;
Should be signed long if you wish to use -1.