From owner-cvs-src@FreeBSD.ORG Sat Apr 3 16:31:05 2004 Return-Path: Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id BB76B16A4CE; Sat, 3 Apr 2004 16:31:05 -0800 (PST) Received: from rwcrmhc11.comcast.net (rwcrmhc11.comcast.net [204.127.198.35]) by mx1.FreeBSD.org (Postfix) with ESMTP id A7EDE43D1D; Sat, 3 Apr 2004 16:31:05 -0800 (PST) (envelope-from DougB@freebsd.org) Received: from freebsd.org (c-24-130-160-161.we.client2.attbi.com[24.130.160.161]) by comcast.net (rwcrmhc11) with ESMTP id <200404040031050130015ecpe> (Authid: domain_name_tsar); Sun, 4 Apr 2004 00:31:05 +0000 Message-ID: <406F5748.2080603@FreeBSD.org> Date: Sat, 03 Apr 2004 16:31:04 -0800 From: Doug Barton Organization: http://www.freebsd.org/ User-Agent: Mozilla/5.0 (X11; U; FreeBSD i386; en-US; rv:1.6) Gecko/20040307 X-Accept-Language: en-us, en MIME-Version: 1.0 To: John Baldwin References: <200403270926.i2R9QMiP083177@repoman.freebsd.org> <200403291155.43206.jhb@FreeBSD.org> In-Reply-To: <200403291155.43206.jhb@FreeBSD.org> X-Enigmail-Version: 0.83.5.0 X-Enigmail-Supports: pgp-inline, pgp-mime Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit cc: cvs-src@FreeBSD.org cc: src-committers@FreeBSD.org cc: cvs-all@FreeBSD.org Subject: Re: cvs commit: src/etc/rc.d mixer X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 04 Apr 2004 00:31:05 -0000 John Baldwin wrote: > On Saturday 27 March 2004 04:26 am, Doug Barton wrote: > >>dougb 2004/03/27 01:26:22 PST >> >> FreeBSD src repository >> >> Modified files: >> etc/rc.d mixer >> Log: >> A few small cleanups: >> >> 1. Add the shutdown keyword so that the script is run at shutdown time, >> and the mixer* files are saved. > > > Huh, it seemed to work in my testing w/o this change. Maybe I botched > something. Actually I think it might depend on how the system gets shut down. Regardless, since you definitely want this to run at shutdown time, adding the shutdown key word is the right way to go. >> 2. Twiddle whitespace. > > > Style(9) mandates blank lines at the start of functions with no local > variables, so that's why I did that. Weird. Not that big a deal either way actually, but I'll try to keep this in mind next time. >> 3. Remove an unecessary function, and therefore collapse one variable. > > > Only unnecessary if you don't value abstraction. :) Now if you want to ever > change the names of the mixer saved files due to a conflict or some such, you > have to change it in multiple places rather than just one. You're right, and if there were more than just two spots, or the same file name was used in more than one script, it would be worth a level of abstraction. But since neither of those is true, (and since the file name is very unlikely to change), it's not worth it. Also, FYI, the way you did it is not the typical "Bourne shell'ish" way to abstract a file name. It would be much more common (and infinitely more efficient) to simply set a script variable with the file name, and then use that throughout the script. > I also did post > this script for review on @arch before it was committed, btw. Yes, and I appreciated that at the time, and gave it a cursory review. At the time I thought, "this is kind of weird, but doesn't look like it will break anything." :) It was only after it stopped working for me that I started digging into it more deeply, and then I figured if I'm here already, might as well clean up a couple more items. Note, nothing you did is really wrong, and I appreciate your contribution, as I'm sure the other rc.d folks do as well. Doug -- This .signature sanitized for your protection