Re: [PATCH] statetop hostname resolution and bug fixes

On Sun, Apr 03, 2005 at 04:07:47AM -0700, Phil Dibowitz wrote:
> OK I made the changes I mentioned before as well as updating the man
> page for ipfstat. This should be a fully functional patch.
>
> It would be nice if someone would test this with IPv6, but short of
> that, I think it's ready and done.
>
> As always, comments welcome. Here's the updated changelog:
>

Looks useful. I've some comments on the source, see below.

IPv6 statetop still works fine for me, but resolving of IPv6
addresses does not.

One suggestion: maybe implement port lookups as well (by using portname()
from lib/portname.c) ?

>
> + sname = malloc(STSTRSIZE);
> + dname = malloc(STSTRSIZE);
> +

malloc() can fail, better check for NULL, or use static allocation.

> + hostinfo = gethostbyaddr((char *)&ips.is_src,sizeof(ips.is_src), AF_INET);

I've two problems with this, the sizeof is wrong because is_src is actually
much bigger than you want. Better replace this with sizeof(struct in_addr).
Or use hostname() from lib/hostname.c. On NetBSD/sparc64, the resolving
doesn't works unless you do one of the above. And secondly, this only
works for AF_INET since you've hardcoded that.

> + if (length < 2) {
> + fprintf(stderr,"Error, screen too small\n");
> + exit(0);
> + }

This doesn't restore the terminal.

> +SIGWINCH doesn't seem to get through to STATETOP.

Resizing works for me (NetBSD-3.99.2/sparc64).

Does handle_resize ever get set to 1 when you resize?


Cheers,
--
Peter Postma