Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 18 Apr 2006 15:02:27 -0500
From:      Eric Anderson <anderson@centtech.com>
To:        Peter Jeremy <peterjeremy@optushome.com.au>
Cc:        freebsd-hackers@freebsd.org, cokane@cokane.org
Subject:   Re: [PATCH] Fancy rc startup style RFC
Message-ID:  <444545D3.5010405@centtech.com>
In-Reply-To: <20060418193018.GB694@turion.vk2pj.dyndns.org>
References:  <444515C8.3030406@centtech.com>	<20060418165709.GA17705@central.0xfce3.net>	<44452532.40703@centtech.com>	<20060418.114933.69380798.imp@bsdimp.com>	<346a80220604181102v3597a1edp3e05fa663b87e15c@mail.gmail.com> <20060418193018.GB694@turion.vk2pj.dyndns.org>

next in thread | previous in thread | raw e-mail | index | archive | help
Peter Jeremy wrote:
> On Tue, 2006-Apr-18 14:02:07 -0400, Coleman Kane wrote:
> A few comments on the shellscript:
> 
>> +			rcargsize=`echo $rc_arg`
>> +			rcargsize=${#rcargsize}
> 
> Try			rcargsize=$((${#rc_arg} + 1))
> 
>> -				    return 1
>> +				    (echo_fancy "FAILED" `expr 10 + $rcargsize - 1`) && return 1
> 
> Try				    echo_fancy "FAILED" $((10 + $rcargsize - 1)) && return 1
> 
>> +echo_fancy () {
> ...
>> +	namesize=`echo -n $name`
>> +	namesize=${#namesize}
> or	namesize=${#name}
> 
>> +	padding=""
>> +	paddingsize=$(($columns - 15 - $2 - $namesize))
>> +	until [ 0 = ${paddingsize} ]; do
>> +		padding=" $padding"
>> +		paddingsize=$(($paddingsize - 1))
>> +	done
> 
> This particular block of code appears unnecessary (since $padding is unused).

I must be missing something, because I'm pretty sure it's used.. What 
did I miss?



>> +			paddingsize=$((60 - $namesize - $rc_argsize))
>> +			until [ 0 = ${paddingsize} ]; do
>> +		  		padding=" $padding"
>> +		  		paddingsize=$(($paddingsize - 1))
>> +			done
> 
> For safety, the conditions should probably be [ 0 -ge ${paddingsize} ]
> I don't see any alternative to the until loop.  If efficiency turns out
> to be a real issue then you could try doing the expansion in multiple
> goes.  Eg:
> 
> 			until [ 8 -gt ${paddingsize} ]; do
> 		  		padding="        $padding"
> 		  		paddingsize=$(($paddingsize - 8))
> 			done
> 			until [ 0 -ge ${paddingsize} ]; do
> 		  		padding=" $padding"
> 		  		paddingsize=$(($paddingsize - 1))
> 			done

Thanks for the hints.  I was testing the same changes to the 
namesize/etc as you suggested, and it does work and is more readable and 
more efficient.

I've included your suggestions and put the latest changes here:

http://www.googlebit.com/freebsd/patches/rc_fancy.patch-3


Thanks for all the feedback!  Keep it coming! :)

Eric




-- 
------------------------------------------------------------------------
Eric Anderson        Sr. Systems Administrator        Centaur Technology
Anything that works is better than anything that doesn't.
------------------------------------------------------------------------



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?444545D3.5010405>