Re: [bug-inetutils] [PATCH] ping/ping6 --deadline option

[Alfred M. Szmidt]

The ChangeLog needs work, it doesn't document the changes you did, and
it documents things incorrectly.

   If you can suggest something better here I will change the name
   accordingly. But as there is a conflict I have kept it ping_timeout,
   which seemed apt to me.

I don't know, figure something out that is nice :-)

    Unreleased
    Version 1.7:

   +* New option --timeput/-w for ping and ping6.  It allows to set
   +  timeout before ping can exit.

The way we do it is more like this:

| * ping
|
|   New option --timeout=N, stop sending packets after N seconds


   address@hidden -w @var{n}
   address@hidden address@hidden
   address@hidden -w
   address@hidden --timeout
   +Specifies the number of seconds before which ping exits.

This is clearer: Stop after @var{n} seconds.


   +  /* Notes down start time, required for timeout. */

This comment isn't very useful.


   +      ping_timeout = ping_cvt_number (arg, 0, 0);

Use ping_cvt_number (arg, INT_MAX, 0) instead, no need for size check.

   +      if (ping_timeout_p(&ping->ping_start_time, ping_timeout))

Space after parentheses.

   +/*
   + * ping_check_timeout --
   + * Check whether timeout has expired already.
   + */

I'd skip the comment... It is clear what ping_timeout_p does.

   +int
   +ping_timeout_p (struct timeval *ptr_start_time, int ping_timeout)
   +{
   +  struct timeval now;
   +  gettimeofday (&now, NULL);
   +  if (ping_timeout != -1)
   +    {
   +      tvsub(&now, ptr_start_time);
   +      /* now here has difference */

Skip the comment.

   +int ping_timeout_p (struct timeval *ptr_start_time, int ping_timeout);

I don't see the value of saying ptr_.. It is clear that it is a
pointer...

int ping_timeout_p (struct timeval *start_time, int ping_timeout);