[PATCH] gen_build_files.sh: rewrite with sed

[Mike Frysinger]

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.

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.

> > +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.

> > +	#chk "${dst}"
> > +	(
> 
> 	^
> 	this will open and run a subshell (fork); is that necessary?
> 
> Would not:
> 
> 	{
> 		cmd1
> 		cmd2
> 		...
> 	} | sed ...
> 
> workas well; forkless?

because of the pipe, the number of forks is the same.  try stracing:
	bash -c '( echo; ) | cat'
	bash -c '{ echo; } | cat

> > +		echo "${header}"
> > +		if grep -qs '^INSERT$' "${src}"; then
> > +			sed -n '1,/^INSERT$/p' "${src}"
> > +			echo "${insert}"
> > +			sed -n '/^INSERT$/,$p' "${src}"
> 			                   ^
> 			                   may be a typo

no idea what you're talking about.  i see no typo.

> > +		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.

> >  s=`sed -n 's@^//applet:@@p' -- "$srctree"/*/*.c "$srctree"/*/*/*.c`
> 
> I read somewhere backtick is obsolete, and easier nestable $(...) should
> be used instead.  Is it true?

you cant make multiple statements and expect a single true/false response to 
cover it.  backticks are not obsolete and they're not going anywhere.  
obviously backticks are a pita to nest, but there is no nesting.  ive left the 
existing code unchanged.

> > +generate \
> > +	"$srctree/include/applets.src.h" \
> > +	"include/applets.h" \
> 	^                 ^
> 	now, why does this need to be quoted?

consistency.  obviously $srctree needs quoting.

> >  exit 0
> 
> What's the purpose with that 'exit 0'?  If some or all the above failed,
> pretend it succeeded (as I don't see errexit is used)?

ive let it alone.  dont care either way.
-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/20101117/3f499e96/attachment.pgp>