From owner-svn-src-head@FreeBSD.ORG Mon Jan 10 23:52:15 2011 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id EF2DE106566C; Mon, 10 Jan 2011 23:52:14 +0000 (UTC) (envelope-from yanegomi@gmail.com) Received: from mail-wy0-f182.google.com (mail-wy0-f182.google.com [74.125.82.182]) by mx1.freebsd.org (Postfix) with ESMTP id DF2B68FC08; Mon, 10 Jan 2011 23:52:13 +0000 (UTC) Received: by wyf19 with SMTP id 19so20314904wyf.13 for ; Mon, 10 Jan 2011 15:52:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:sender:received :in-reply-to:references:date:x-google-sender-auth:message-id:subject :from:to:cc:content-type:content-transfer-encoding; bh=a4xEFsDNmeGro190UH6vNgWe4jMEAtyK3bnggjgXYzM=; b=W3hbqwin7bjtTpd3znty3Hx37lx//5ZFy4ROowr64BGwigrNsuZvkd0g03RgEHup8v yb/782wAn43EpGZfQ+ifO0i59sPL4CBwlaiiprFZKaH/siZnHH5oGrsIsoHrdgqpPDzA d48heE9WudxYc7NV5VY1WA2dVueTR631PJ0Wk= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=ZsOBb9p11qpUK5BU507Zyj6oW1F+ubCA/Sgla6jLJCTNuv2Vm+2JV0tB7RsYE4mlwX Bx6AhRnAvk3DAMAhoAov48Pt9CnVfKgHMb649STNP8g3/aNHQnRfD68p7NRWBMhT9LoB vGgbroxOf9tz8KShN5lFkc6mfIkajG2RX0f+E= MIME-Version: 1.0 Received: by 10.216.49.15 with SMTP id w15mr322105web.1.1294703532754; Mon, 10 Jan 2011 15:52:12 -0800 (PST) Sender: yanegomi@gmail.com Received: by 10.216.254.226 with HTTP; Mon, 10 Jan 2011 15:52:12 -0800 (PST) In-Reply-To: <20110110220957.GB1923@garage.freebsd.pl> References: <201101101911.p0AJBQKG090310@svn.freebsd.org> <20110110220957.GB1923@garage.freebsd.pl> Date: Mon, 10 Jan 2011 15:52:12 -0800 X-Google-Sender-Auth: JIaPwvjwNu2BovUPf_ejYPguxqs Message-ID: From: Garrett Cooper To: Pawel Jakub Dawidek Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: Josh Paetzel , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r217229 - head/usr.sbin/pc-sysinstall/backend X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 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: Mon, 10 Jan 2011 23:52:15 -0000 On Mon, Jan 10, 2011 at 2:09 PM, Pawel Jakub Dawidek wrot= e: > On Mon, Jan 10, 2011 at 07:11:26PM +0000, Josh Paetzel wrote: > [...] >> =A0 =A0while read line >> =A0 =A0do >> =A0 =A0 =A0# Check for data on this slice >> - =A0 =A0echo $line | grep "^${DISKTAG}-part=3D" >/dev/null 2>/dev/null >> + =A0 =A0echo $line | grep "^${_dTag}-part=3D" >/dev/null 2>/dev/null > > You can just use 'grep -q' instead of redirecting grep's output to > /dev/null. > >> =A0 =A0 =A0if [ "$?" =3D "0" ] > > This will work, but more elegant way is [ $? -eq 0 ] - there is no need > to convert exit code to string. I've seen people claim that the two items above are to make shell code `more accessible' and `more portable' to developers, but I agree that in this case you're not gaining much but instead just dumbing down your audience. >> + =A0 =A0 =A0 =A0FOUNDROOT=3D"1" ; export FOUNDROOT > > 'export FOUNDROOT=3D"1"' should work too. `export FOUNDROOT=3D1' will do. >> + =A0 =A0 =A0 =A0if [ "${FS}" !=3D "UFS" -a "${FS}" !=3D "UFS+S" -a "${F= S}" !=3D "UFS+J" -a "${FS}" !=3D "UFS+SUJ" ] ; then > > Something like this should work too: > > =A0 =A0 =A0 =A0if [ "${FS%+*}" !=3D "UFS" ]; then Except they're catching less than that: $ FS=3DUFS+FOO $ echo ${FS%+*} UFS $ A case statement would be easier to digest: case "$FS" in UFS|UFS+J|UFS+S|UFS+SUJ) # Do something here. ;; esac >> + =A0 =A0 =A0 =A0dd if=3D/dev/zero of=3D${_pDisk}p${CURPART} count=3D204= 8 >/dev/null 2>/dev/null > > If you specify 'of=3D' there is no need to redirect standard output to > /dev/null, as it is already redirected somewhere else. +1. Please keep the 2>/dev/null though because that's noise from the summary output. >> + =A0 =A0 =A0 =A0if [ ! -z "${ENCPASS}" ] ; then > > '[ ! -z "{str}" ]' is equivalent of '[ -n "${str}" ]'. > >> + =A0 =A0 =A0 =A0CURPART=3D"`expr ${CURPART} + 1`" > > Simpler: CURPART=3D$((CURPART+1)) Or `: $(( CURPART +=3D 1 ))' >> + =A0if [ "$?" !=3D "0" ] ; then return ; fi > > [ $? -eq 0 ] || return if [ $? -eq 0 ]; then return fi is easier to follow for me because more people go buckwild with the one-liners (and in some cases have introduced bugs that way because they didn't properly think about precedence of the operations, etc). The one-line above if ... fi above is ugly though. >> - =A0rc_halt "gpart add -b 34 -s 128 -t freebsd-boot ${_intDISK}" >> + =A0rc_halt "gpart add -b 34 -s 64 -t freebsd-boot ${_intDISK}" > > Gptzfsboot in HEAD is 27463 bytes. Gptzfsboot in ZFSv28 is 29659, so > using 64 sectors leaves only 3109 bytes for it to grow. > Note that, eg. RAIDZ3 support is not yet implemented and I expect it > might be not be enough place left to implement it if you do that. > > PS. Only because those are shell scripts doesn't mean style is not > important. They could really be easier to read if they follow style used > in rcNG. Thanks, -Garrett