Source-Changes archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: CVS commit: src



On Fri, Oct 10, 2008 at 11:35:48AM +0000, Andrew Doran wrote:
> > Module Name:    src
> > Committed By:   tls
> > Date:           Mon Aug  4 03:55:48 UTC 2008
> > 
> > Modified Files:
> >         src/sys/arch/i386/conf: GENERIC XEN2_DOM0 XEN2_DOMU
> >         src/sys/conf: files
> >         src/sys/kern: uipc_socket.c uipc_socket2.c
> >         src/sys/sys: socket.h socketvar.h
> >         src/usr.sbin/inetd: inetd.c
> > Added Files:
> >         src/sys/kern: uipc_accf.c
> >         src/sys/netinet: accept_filter.h accf_data.c accf_http.c
> > 
> > Log Message:
> > Add accept filters, ported from FreeBSD by Coyote Point Systems.  Add inetd
> > support for specifying an accept filter for a service (mostly as a usage
> > example, but it can be handy for other things).  Manual pages to follow
> > in a day or so.
> > 
> > OK core@.
> 
> - Why does this sprinkle #ifdef INET into the socket code?

Because it was there in FreeBSD.  Two other developers told me they'd
remove the ifdefs (it's safe to) but that clearly didn't happen so I
will do it Monday.

> - The locking is stubbed out which means it is unsafe.
> - It should use kmem_alloc() and not malloc().
> - Did core@ bother to review it?

I have no idea.  I posted the patch months in advance and asked for
comments, but got almost none on the code itself (I got some on the
API, but it seemed unwise to change it from what's in FreeBSD and OS X).
Another developer fairly urgently requested that it be checked in.

I see the stubbed out socket locks.  I missed this too (the code was
adapted to -current by another developer).  I can unstub them but this
will want some testing.  Do you see any particularly obvious problems?
(I haven't even thought about lock ordering between the a_f_mtx and
the softnet lock yet, ugh.

I can make the malloc->kmem_alloc change very quickly

I'm sorry about this.  I didn't really get a lot of comments on the
code itself (I got one helpful one from Darren but another developer
didn't like the change he suggeted) and then this got caught up in
the transition to the new socket locking scheme.  We'll make it work.

-- 
Thor Lancelot Simon                                        
tls%rek.tjls.com@localhost
    "Even experienced UNIX users occasionally enter rm *.* at the UNIX
     prompt only to realize too late that they have removed the wrong
     segment of the directory structure." - Microsoft WSS whitepaper


Home | Main Index | Thread Index | Old Index