[bug-inetutils] non-portable code in inetutils, ping bugs

[Jim Balter]

libinetutils/setenv.c:
#ifndef 
HAVE_GNU_LD
# define __environ      environ
#endif
-------------------------
extern char** environ is missing

libinetutils.c/xmalloc.c:

#include "error.h"
...
        error (xalloc_exit_failure, 0, "%s", _(xalloc_msg_memory_exhausted));
--------------------------
error.h and error are glibc-specific, but aren't ifdefed.

There could be a lot of other bugs, I'm just building ping.
Speaking of which -- there are lots of other bugs.
ping -c1 <unreachable host>
is documented to stop after sending 1 packet but instead
runs forever, because of at least three bugs:

1) in ping_run in ping/ping.c,

        if (!ping->ping_count || ping->ping_num_recv < ping->ping_count)
          {
             send_echo (ping);
             ...

should obviously be

        if (!ping->ping_count || ping->ping_num_xmit < ping->ping_count)
          {
             send_echo (ping);
             ...

2) also in ping_run in ping/ping.c,

      int n, len;
      ...
      else if (n == 1)
        {
          len = ping_recv (ping);
          if (t == 0)
            {
              gettimeofday (&now, NULL);
              t = &now;
            }
          if (ping->ping_count && ping->ping_num_recv >= ping->ping_count)
          ...

should be

  int nresponses = 0;
  ...
      int n;
      ...
      else if (n == 1)
        {
          if (ping_recv(ping) == 0)
            nresponses++;
          if (t == 0)
            {
              gettimeofday (&now, NULL);
              t = &now;
            }
          if (ping->ping_count && nresponses >= ping->ping_count)
          ...

First, the return value from ping_recv is not a "len".
Second, ping_num_recv only counts responses from the target,
not from the router.  A response from either means nothing
else is expected back.  Using ping_num_recv would cause an
unnecessary MAX_WAIT delay.  (BTW, why the heck isn't that
a parameter?  Waiting 10 seconds is ridiculous.)

3) in ping_recv in libicmp/libping.c,

  switch (icmp->icmp_type)
    {
    case ICMP_ECHOREPLY:
    case ICMP_TIMESTAMPREPLY:
    case ICMP_ADDRESSREPLY:
      /*    case ICMP_ROUTERADV:*/
      if (icmp->icmp_id != p->ping_ident)
        return -1;
      if (rc)
        {
          fprintf (stderr, "checksum mismatch from %s\n",
                  inet_ntoa (p->ping_from.sin_addr));
        }

Gack!!  Both the icmp_id check and the checksum check
should be above the case!!!  With the icmp_id check
inside the case, the nresponses++ above in run_ping,
based on the return value from ping_recv (which isn't
currently being checked) would be bumped for responses
from routers to icmp packets sent from other processes
(ping runs in broadcast mode).  Yow!  You should try
running ping -v on a machine with other processes doing
lots of icmps to unreachable addresses and see what you get!
(It would also be nice if the code were remotely professional,
with comments on the routines specifying the meaning of
return values, and so on.)

That's just what I encountered for what I specifically needed
from ping.  Given the number of hits, somebody needs to be
paying a lot more attention to quality.

--
<J Q B>