Re: The patch of pfinet

[zhengda]

olafBuddenhagen@gmx.net wrote:
> Hi,
>
> On Fri, Aug 08, 2008 at 08:09:24AM +0200, zhengda wrote:
>
>   
> > The patch enables the pfinet to work with the multiplexer and use the
> > filter rule that only accepts the packet whose destination  is the
> > pfinet server.
> >     
>
> This rather sounds like two totally orthogonal changes, that really
> should go into two separate patches?
>   
If you think it's better, I'll do it.
>   
> > -CFLAGS += -fno-strict-aliasing
> > +CFLAGS += -fno-strict-aliasing -DPCAP_SUPPORT
> > +
> > +LDFLAGS += -lpcap
> >     
>
> It is probably better not to enable this by default, to avoid problems
> with the external dependency...
>
> Of course, once you have autoconf checks/options for pcap, this won't be
> an issue anymore :-)
>
> BTW, HAVE_PCAP would be a more conventional name for this kind of thing
> I believe...
>   
done.
> > +/* The BPF instruction allows IP and ARP packets */
> > +static struct bpf_insn ether_filter[] =
> >     
> [...]
>
> Do I get it right that the static rule doesn't check the IP, and only
> when PCAP support is present, a proper rule will be generated?
>   
yes.
> > -  err = get_privileged_ports (0, &master_device);
> > -  if (err)
> > -    error (2, err, "cannot get device master port");
> > +  if (master_device_file)
> > +    {
> > +      master_device = file_name_lookup (master_device_file , 0 , 0);
> > +      if (master_device == MACH_PORT_NULL)
> > +        error (10, 0, "file_name_lookup %s", master_device_file);
> > +    }
> > +  else
> > +    {
> > +      err = get_privileged_ports (0, &master_device);
> > +      if (err)
> > +        error (2, err, "cannot get device master port");
> > +    }
> >     
>
> Is it really appropriate to use a different error code in the case of
> using an alternate master device port?...
>   
I don't know how to use the error code properly.
what is the rule to use it?
>   
> > diff -urN pfinet.old/pcap_filter.c pfinet/pcap_filter.c
> > --- pfinet.old/pcap_filter.c    1970-01-01 01:00:00.000000000 +0100
> > +++ pfinet/pcap_filter.c    2008-08-08 07:06:26.000000000 +0200
> > @@ -0,0 +1,76 @@
> > +/*
> > +   Copyright (C) 1993,94,95,96,97,98,99,2000,01,02,2006,2008
> > +   Free Software Foundation, Inc.
> >     
> [...]
>   
> > +/* Written by Zheng Da.  */
> >     
>
> This is contradictory: Either you wrote the file yourself, in which case
> the copyright years are obviously wrong; or you based it on some other
> file, in which case it is not written by you alone...
>
> If you indeed based it on some other file, please state so explicitely,
> to make it clear -- or just leave out the "written by" part alltogether.
>
> In either case, don't put it in an extra comment. If you want to include
> author information, do so right after the copyright statement, before
> the licensce boilerplate.
>   
it's not based on other files.
I saw someone write the author information in the an extra comment, so I 
thought it was also correct.
so is the correct copyright statement something like this?
/*
  Copyright (C) 2008
  Free Software Foundation, Inc.

  Written by Zheng Da.

  This file is part of the GNU Hurd.
...
>   
> > +  insn = (struct bpf_insn *) malloc ((program.bf_len + 1) * sizeof  
> > (*insn));
> >     
>
> Your mail client mangled the patch, inserting a spurious linebreak.
> (Also happened in a few other places.)
>   
I guess the reason is that the line is a little long, and my mail client 
break it automatically.
I think it always happens when a mail is sent to the mailing list.
Do you have any idea to prevent the mail client to do it?
I use thunderbird. I didn't find anything that can switch this function off.

Zheng Da