[PATCH] gen_build_files.sh: rewrite with sed

[Mike Frysinger]

On Wednesday, November 17, 2010 19:34:04 Christopher Barry wrote:
> On Wed, 2010-11-17 at 18:36 -0500, Mike Frysinger wrote:
> > On Tuesday, November 16, 2010 15:31:13 Cristian Ionescu-Idbohrn wrote:
> > > On Tue, 16 Nov 2010, Mike Frysinger wrote:
> > > > The shell parsing of files is incredibly slow on many systems.  With
> > > > one report, the process was taking a minute or two which made people
> > > > thing the build was hung.  So rewrite the craziness with sed and
> > > > proper shell functions.  On an idle system, this cut the runtime by
> > > > half.
> > > 
> > > Sounds good.  Still, you can surely explain the inconsistant and
> > > totally superfluous curly brackets around variable names.
> 
> IMHO, *ALL* variables should be in curly braces as a safe convention.

sure, and that's generally a convention i follow.  only real time i dont is 
when it comes to positional args (like $1 -- just a habit) or where i'm 
preserving existing code style ($srctree -- didnt feel the need to rewrite 
code that didnt matter to the underlying fix).

> > nothing superfluous about it.  your general usage of using carats instead
> > of properly quoting context however is obnoxious and not easily usable
> > unless i choose specifically monospace fonts in my e-mail client.
> 
> well, this is a convention mostly destroyed by LookOut.

i have no idea what "LookOut" is ... googling indicates something to do with 
mobile phones, but i'm not using those.

> > > > +generate()
> > > > +{
> > > > +	local src="$1" dst="$2" header="$3" insert="$4"
> > > 	
> > > 	^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > > 
> > > All of it or parts of it may go wrong if the default shell is dash
> > 
> > wrong.  the code works fine.
> 
> AFAIK, The only place where the simultaneous define and assignment
> breaks down is where the assignment is like:
> local foo=$(bar baz) || err_handler
> In that case, the 'local' keyword will always return true, causing the
> err_handler routine to not be executed if bar fails.
> For simple assignment, there is no issue at all. In fact, the code above
> is the most efficient.

yes, that is problematic for the reason you describe, but not applicable here.  
dash is also a pos and requires the quoting where sane shells like bash work 
fine with: local f=$(echo a b)

> > > > +		else
> > > > +			if [ -n "${insert}" ]; then
> > > > 
> > > 			     ^^
> > > 			
> > > 			     useless, this:
> > > 			if [ "$insert" ]; then
> > > 
> > > does the same job.
> > 
> > style wise, i prefer the braces to be *consistent* and not worry about
> > random inlining of vars with plain text.  so any feedback along these
> > lines is ignored.
> 
> One thing I would say is (in bash anyway) '[[' is a builtin, while '['
> is not, and thus will incur less overhead. If you're using bash, you
> should use this always (unless there is some good reason not to)

while i am a promoter/user of bash (and thus always [[]] over []), two things:
 - '[' is a builtin in pretty much every shell out there
 - this script is not bash and the busybox build system strives to avoid it
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20101118/3ebfd3ed/attachment-0001.pgp>