bug-inetutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: ping


From: Sergey Poznyakoff
Subject: Re: ping
Date: Wed, 20 Jun 2001 10:32:12 +0300

Hello,

Thanks again for your fixes.

> > The router
> > discovery and path MTU discovery requests will follow soon.
> 
> Cool, is this traceroute?
Not exactly. It is a method of determining the maximum MTU allowed
to use over a certain path. It is described in RFC1191. The only
considerable difficulty in implementing it is to find a portable
way to set the "don't fragment" flag in an IP header.

> 1) Buglet
> On Neutrino pid_t is 32 bit type and it is use fully 8-) here is a
> small output of 'ps':
<..snip..>
> +  p->ping_ident = ident & 0xFFFF;

Actually on both Linux and Solaris pid_t is also 32 bits long, but the
upper halv of them seem never to be used. Patch applied.

> 2) <signal.h>
> As stated in my previous message, there is no such thing as <sys/signal.h>
> I mean we should rather use <signal.h> which is guarantee to exist.
> QNX/Neutrino does not provide <sys/signal.h> and it is perfectly legal
> and POSIX compliant ... this time I've checked 8-)
Oh, I'm sorry, I seem to have forgotten about this when committing the
stuff... Thanks.

> 3) buglet
> icmp_address.c(icmp_addrss_encode)
> The icmp should be initialise before accessing the icmp_mask field.

Oops! That one is very serious.

> 4)
>       icmp_cksum.c:29: warning: no previous prototype for `icmp_cksum'
> Prototype missing for icmp_cksum().  I'm not sure you wanted
> to export this or rather make static and only use in icmp_echo.c.

I'm not sure yet. It may come handy later, so it is extern for the
time being. If it's not needed I'll move it away to icmp_echo and
will make it static. Anyway, the proto is in icmp.h. 

> 5)
>       libping.c:313: warning: declaration of `sin' shadows global 
> declaration> 'sin' is a bad name for a variable, it shadows a global name

Yes, but sin() is never used in this function (and in the library
also), so it doesn't hurt. Anyway I have renamed it :^)

> 6)
>  Some inconsistencies in signess 8)
> 
>       libping.c:151: warning: comparison between signed and unsigned
>       icmp_echo.c:65: warning: comparison between signed and unsigned
I have made ping_datalen member to be of type size_t to avoid this.

Patches applied. 

Merci beaucoup!

--
Sergey



reply via email to

[Prev in Thread] Current Thread [Next in Thread]