svn commit: r273441 - in head/sys: kern sys

[Konstantin Belousov]

On Wed, Oct 22, 2014 at 10:26:21AM +0200, Mateusz Guzik wrote:
> On Wed, Oct 22, 2014 at 10:51:31AM +0300, Konstantin Belousov wrote:
> > On Wed, Oct 22, 2014 at 12:23:44AM +0000, Mateusz Guzik wrote:
> > > Author: mjg
> > > Date: Wed Oct 22 00:23:43 2014
> > > New Revision: 273441
> > > URL: https://svnweb.freebsd.org/changeset/base/273441
> > > 
> > > Log:
> > >   filedesc: cleanup setugidsafety a little
> > >   
> > >   Rename it to fdsetugidsafety for consistency with other functions.
> > >   
> > >   There is no need to take filedesc lock if not closing any files.
> > >   
> > >   The loop has to verify each file and we are guaranteed fdtable has space
> > >   for at least 20 fds. As such there is no need to check fd_lastfile.
> > ^^^^^^^^^^^^^^^^^^^^^^^^ *
> > 
> [..]
> > >  	fdp = td->td_proc->p_fd;
> > >  	KASSERT(fdp->fd_refcnt == 1, ("the fdtable should not be shared"));
> > > -	FILEDESC_XLOCK(fdp);
> > > -	for (i = 0; i <= fdp->fd_lastfile; i++) {
> > > -		if (i > 2)
> > > -			break;
> > > +	for (i = 0; i <= 2; i++) {
> > [*] This requires an assert.
> > 
> 
> I was considering adding one, but failed to come up with anything good.
> 
> Ideally we would compile-time assert that NDFILE is at least 3, but that
> seems weirdly circumventable by sufficient accident.
> 
> MPASS(fdp->fd_nfiles > 3) does not guarantee we wont run into trouble,
> has a potential to help.
What troubles do you mean ?  Also, why > 3, and not >= 3 ?
Old code used fd_lastfile, which, for purpose of the assert, is also
fine.