From owner-freebsd-hackers@FreeBSD.ORG Sat Jan 6 19:27:49 2007 Return-Path: X-Original-To: freebsd-hackers@freebsd.org Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 6DAC416A416; Sat, 6 Jan 2007 19:27:49 +0000 (UTC) (envelope-from olli@lurza.secnetix.de) Received: from lurza.secnetix.de (lurza.secnetix.de [83.120.8.8]) by mx1.freebsd.org (Postfix) with ESMTP id 779BE13C4A8; Sat, 6 Jan 2007 19:27:48 +0000 (UTC) (envelope-from olli@lurza.secnetix.de) Received: from lurza.secnetix.de (bapqbq@localhost [127.0.0.1]) by lurza.secnetix.de (8.13.4/8.13.4) with ESMTP id l06JRXbD095829; Sat, 6 Jan 2007 20:27:39 +0100 (CET) (envelope-from oliver.fromme@secnetix.de) Received: (from olli@localhost) by lurza.secnetix.de (8.13.4/8.13.1/Submit) id l06JRWoB095827; Sat, 6 Jan 2007 20:27:32 +0100 (CET) (envelope-from olli) From: Oliver Fromme Message-Id: <200701061927.l06JRWoB095827@lurza.secnetix.de> To: imp@bsdimp.com (M. Warner Losh) Date: Sat, 6 Jan 2007 20:27:32 +0100 (CET) In-Reply-To: <20070106.113348.353676879.imp@bsdimp.com> X-Mailer: ELM [version 2.5 PL8] MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-2.1.2 (lurza.secnetix.de [127.0.0.1]); Sat, 06 Jan 2007 20:27:40 +0100 (CET) X-Mailman-Approved-At: Sat, 06 Jan 2007 21:25:48 +0000 Cc: erik.udo@gmail.com, freebsd-hackers@freebsd.org, dougb@freebsd.org Subject: Re: Init.c, making it chroot X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 06 Jan 2007 19:27:49 -0000 M. Warner Losh wrote: > > this patch looks good, however, one nit: > > In message: <200701052106.l05L60a6042599@lurza.secnetix.de> > Oliver Fromme writes: > : + if (stat("/dev", &stst) != 0) > : + warning("Can't stat /dev: %m"); > : + else { > : + if (stst.st_dev == root_devno) > : + devfs++; > : + } > > is more succinctly expressed as: > > + if (stat("/dev", &stst) != 0) > + warning("Can't stat /dev: %m"); > + else if (stst.st_dev == root_devno) > + devfs++; Agreed. I thought style(9) would forbid nesting "else if" like that, but I was wrong. > Also, kenv(KENV_GET, ... is used a lot. Maybe it makes sense to have > a simple kenvget call. Would make a few lines a little shorter if > nothing else. KENV_GET is used three times. Using a wrapper function would save 7 characters per call. I don't think it's really worth it. But if you insist, I can update the patch with such a function. By the way, how about declaring requested_transition volatile, as explained in my previus mail? Best regards Oliver -- Oliver Fromme, secnetix GmbH & Co. KG, Marktplatz 29, 85567 Grafing Dienstleistungen mit Schwerpunkt FreeBSD: http://www.secnetix.de/bsd Any opinions expressed in this message may be personal to the author and may not necessarily reflect the opinions of secnetix in any way. cat man du : where Unix geeks go when they die