On Mon, Oct 18, 2010 at 13:07:01 +0200, Alexander Reichle-Schmehl wrote: > Hi! > > I would like to upload iftop to "unstable" and let it migrate to > testing. The most important change would be added IPv6 support. > > > Overall, I would like to upload with the following changes: > > debian/patches/07-spelling-and-usage-text.patch | 87 + > Fixes some spelling error in the manpage > > debian/patches/08-segfault-duplicate-options.patch | 55 + > Fixes #425488/#598376 a sefault when using the same command line > argument twice. > > debian/patches/09-incomplete-hash-func.patch | 51 > Fixes #595169, which is actually only wishlist, but quite nasty. > > debian/patches/10-lladdr-inclusion-kfreebsd.patch | 37 > Fixes #598367, an usage problem on kfreebds-* > I don't think this is correct. The usual way to detect kfreebsd is __FreeBSD_kernel__, not 'defined __GNUC__ && ! defined __linux__'. > debian/patches/11-implement-ipv6-support.patch | 1120 +++++++++++++++++++++ > Is the missig IPv6 support. > I'm not convinced it's the right time to make that sort of changes... Besides, why does this add seemingly random __inline__ attributes instead of letting the compiler do its job? ++ struct tcphdr *thdr = ((void *) ip6tr) + 40; this wants to be char *, not void *. (void pointer arithmetic is a gcc-ism) The code seems to assume an interface has only one ipv4 or ipv6 address (e.g. packet_init)? This: ++ /* If the upper three (network byte order) uint32-parts ++ * are null, then there ought to be an IPv4 address here. ++ * Any such IPv6 would have to be 'xxxx::'. Neglectable? */ ++ probe = (uint32_t *) addr; ++ af = (probe[1] || probe[2] || probe[3]) ? AF_INET6 : AF_INET; is confusing. It talks about the upper three bytes, and then goes to check the lower three? > All these patches have been reviewsed and accepted by upstream in the > meantime. > > The laste change I seek your approval would be the following: > iftop-0.17/debian/rules | 3 > debian/NEWS | 11 > > Here I disabled an quite unexpected feature of iftop: It allows you to > open a subshell. As iftop is often used in combination with sudo, > sysadmins might be suprised, to give other users full root by granting > them access to iftop (granted; only if the don't configure sudo > properly); but as it is only a minimal change, I hope you allow that, > too. > I don't think this warrants a news entry, but oh well. > > Just for completness, I also changed the following files: > debian/watch | 4 > iftop-0.17/debian/changelog | 27 > iftop-0.17/debian/control | 2 > iftop-0.17/debian/patches/series | 5 > > > I've uploaded the complete debdiff between 0.17-6 (currently in Squeeze) > and the proposed 0.17-8 upload at > http://people.debian.org/~tolimar/tmp/iftop-debdiff_0.17-6_0.17-8.diff. > Cheers, Julien
Attachment:
signature.asc
Description: Digital signature