[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: The patch of pfinet
From: |
olafBuddenhagen |
Subject: |
Re: The patch of pfinet |
Date: |
Wed, 13 Aug 2008 08:10:39 +0200 |
User-agent: |
Mutt/1.5.18 (2008-05-17) |
Hi,
On Tue, Aug 12, 2008 at 03:52:58PM +0200, zhengda wrote:
> pfinet1.patch. To make it open the virtual network interface.
> pfinet2.patch. To use the proper filter rule in pfinet.
> pfinet3.patch. To enable ioctl to set the network device into the
> promiscuous mode.
Would be nice for the file names to include a hint what the pathes do,
i.e. something like pfinet-virt_iface.patch, pfinet-use_IP_filter.patch,
pfinet-promiscuous_mode.patch :-)
> + master_device = file_name_lookup (master_device_file , 0 , 0);
> + if (master_device == MACH_PORT_NULL)
> + error (2, 0, "file_name_lookup %s", master_device_file);
Doesn't file_name_lookup() set errno?...
> for (in = h->interfaces; in < h->interfaces + h->num_interfaces; in++)
> - if (strcmp (in->device->name, arg) == 0)
> + if (strcmp (in->name, arg) == 0)
Wrong indentation.
> +#ifdef HAVE_PCAP
[...]
> +#else
> +
> +int ethernet_reset_ipfilter (struct device *dev, struct in_addr addr)
> +{
> + return 0;
> +}
> +
> +#endif
When using long #ifdef blocks, please comment the #else/#endif
statements -- see the GNU Coding Standards.
> + err = dev_change_flags(dev, flags);
> + err = machdev_change_flags (dev, flags);
The second call will overwrite the err returned by the first... You need
another condition here.
-antrik-