On Sun, Nov 08, 2015 at 01:18:22PM +0100, Alexander Bluhm wrote:
> On Sun, Nov 08, 2015 at 02:37:58AM +0100, Alexander Bluhm wrote:
> > > + for (i = 0; (i < size) && (rv == 0); i++) {
> > 
> > rv is unitialized in the first interation
> > 
> > > +         io.pfrio_buffer = addr++;
> > > +         rv = ioctl(dev, DIOCRADDADDR, &io);
> > 
> > I would suggest to return (-1) if ioctl fails...
> > 
> > > +         add++;
> > > + }
> 
> To keep the illusion of an atomic operation, we could remove the
> addresses we just added before the one add failed.
> 

actually pfctl_radix.c is just tip of the iceberg,  there are other tools than
pfctl, which manipulate with PF-tables:
        authpf
        bgpd
        pfutils

The more I'm thinking about s/SIOCADDADDRS/SIOCADDADDR the less I like it.  I
feel good about s/pfr_add_addrs/pfr_add_addr. The pfr_add_addr() should be a
back end for SIOCADDADDRS ioctl operation, which I think should go back.  The
ioctl in kernel will iterate over the array of addresses coming from userland.
It seems to me as more convenient approach. I'm working on prototype, I
hope I'll send updated patches soon.

thanks and
regards
sasha

Reply via email to