Re: Stats Collector Error 7.4beta1 and 7.4beta2

Jan Wieck <JanWieck(at)Yahoo(dot)com> writes:
> I still believe that this is just garbage in the padding bytes after the
> IPV4 address.

Yes. I have been looking at the behavior on my own Linux box, on which
it turns out stats are broken too in CVS tip. It is very clear that
getsockname() is returning garbage --- it says that the address length
is 16 bytes, but only the first 8 are actually used, and the values put
into the other 8 bytes vary from run to run. recvfrom() also returns 16
bytes, but it seems to be careful to zero the extra space. Ugh.

The reason 7.4 breaks where 7.3 worked is that 7.3 compared the
addresses like this:

if (fromaddr.sin_addr.s_addr != pgStatAddr.sin_addr.s_addr)
continue;
if (fromaddr.sin_port != pgStatAddr.sin_port)
continue;

where 7.4 has

if (memcmp(&fromaddr, &pgStatAddr, fromlen))
continue;

> The code currently bind()'s and connect()'s explicitly to
> an AF_INET address.

Not in 7.4. Kurt rewrote that stuff so it would still work on a machine
with only IPV6 addressing. It should work for either AF_INET or
AF_INET6 sockets. (I'm unconvinced that assuming "localhost" can be
looked up is an improvement over assuming "127.0.0.1" can be used, but
that's not our immediate problem.)

> After reading Kurt's quoting of the SUS manpage I have to agree with Tom
> in that we cannot skip the check entirely. It says it limits for recv()
> but we are using recvfrom() ... there might be a little difference on
> that platform ...

I was about to say "I give up, let's just take out the comparison".
Your point is interesting but easily avoided; if we aren't going to check
fromaddr anymore then there's no need to use recvfrom(), it could as
well be recv() and save the kernel a few cycles.

regards, tom lane