Re: CVS commit: src/sys/net

[David Young <dyoung%pobox.com@localhost>]

On Wed, Jul 23, 2008 at 09:27:20AM -0700, Gregory McGarry wrote:
> > On Wed, Jul 23, 2008 at 03:21:55PM +0900, Masao Uebayashi wrote:
> > > In 1.167:
> > > 
> > >    1492         case SIOCSIFFLAGS:
> > >    1493                 switch (ifp->if_flags & (IFF_UP|IFF_RUNNING)) {
> > >    1494                         /*
> > >    1495                          * If interface is marked down and it is 
> > running,
> > >    1496                          * then stop and disable it.
> > >    1497                          */
> > >    1498                         (*ifp->if_stop)(ifp, 1);
> > >    1499                         break;
> > > 
> > > No case label here.
> > 
> > The proper fix is to add a 'case IFF_RUNNING:' below the switch ().
> > Please, make the proper fix, instead.
> 
> 
> I don't think that adding multiple exit points to functions is going
> to help the effort towards fine-grained locking.  Functions should
> be converted to have single exit points so that locking is less
> complicated.

I can see how that may be true, if you make certain assumptions.  I don't
think that we have made the same assumptions.  As I see it, multiple exit
points do not necessarily entail locking complexity, and ether_ioctl()
is not necessarily the best place to acquire and release locks.

Dave

-- 
David Young             OJC Technologies
dyoung%ojctech.com@localhost      Urbana, IL * (217) 278-3933 ext 24