From owner-svn-src-head@FreeBSD.ORG Wed Oct 22 08:58:28 2014 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id A68FA4C7; Wed, 22 Oct 2014 08:58:28 +0000 (UTC) Received: from mail-wg0-x22c.google.com (mail-wg0-x22c.google.com [IPv6:2a00:1450:400c:c00::22c]) (using TLSv1 with cipher ECDHE-RSA-RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id C23DBDB5; Wed, 22 Oct 2014 08:58:27 +0000 (UTC) Received: by mail-wg0-f44.google.com with SMTP id y10so3156071wgg.27 for ; Wed, 22 Oct 2014 01:58:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=date:from:to:cc:subject:message-id:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; bh=DOX3P5uD40Taj1iZ9KXr+TMZ2hOfO02rwENraiQWRr0=; b=NPGmGAShXbEDknhyHDqippbxxzvryWpnmkT+476SYVZicosYiTF0hs12CDNjT6gAHR dsNdEQqY5nN4bH9LNbHWrDlfAH3I2JHKyhLmhDIKffwdspO32SHipDp+xCayT2KPO6vk GnCqLXAdoegytz3sOygVi0mXg2HRySm7rXbLo4LHXAB32E96XQINSxC+Roxpsv/3RhH/ DHHtD/xy7Heg2bnM6kCI+y+swDzMpJukLPSHjwpjya3HuT2c4KEpGoVnKWUohu2OEIXT hi1FMNP4wl/n3z5+MP6jbIH6ox8+i3+IFO7ysPctTTfY+Ydl4w0DLeU9k3fW0GBvP119 FD2A== X-Received: by 10.180.88.68 with SMTP id be4mr4125727wib.36.1413968305812; Wed, 22 Oct 2014 01:58:25 -0700 (PDT) Received: from dft-labs.eu (n1x0n-1-pt.tunnel.tserv5.lon1.ipv6.he.net. [2001:470:1f08:1f7::2]) by mx.google.com with ESMTPSA id j20sm11947570wjn.0.2014.10.22.01.58.24 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Wed, 22 Oct 2014 01:58:25 -0700 (PDT) Date: Wed, 22 Oct 2014 10:58:21 +0200 From: Mateusz Guzik To: Konstantin Belousov Subject: Re: svn commit: r273441 - in head/sys: kern sys Message-ID: <20141022085821.GC4393@dft-labs.eu> References: <201410220023.s9M0NiBX089974@svn.freebsd.org> <20141022075131.GG1877@kib.kiev.ua> <20141022082621.GB4393@dft-labs.eu> <20141022084138.GI1877@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20141022084138.GI1877@kib.kiev.ua> User-Agent: Mutt/1.5.21 (2010-09-15) Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Mateusz Guzik X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Oct 2014 08:58:28 -0000 On Wed, Oct 22, 2014 at 11:41:38AM +0300, Konstantin Belousov wrote: > 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 ? Oops, yeah, that should have been >= 3. I mean this kind of assertion is standard "will catch offenders, if they happen to execute it" as opposed to making sure such offenders are not possible to exist in the first place. I committed this MAPSS, if I come up with something better I'll update the code. > Old code used fd_lastfile, which, for purpose of the assert, is also > fine. fd_lastfile denotes the biggest fd currently in use, so it cannot be used in an assertion. -- Mateusz Guzik