[PATCH] debloat lash by 4.803725881571%

Bernhard Fischer rep.nop at aon.at
Mon Jun 19 08:37:18 UTC 2006


On Sun, Jun 18, 2006 at 09:21:01PM -0400, Rob Landley wrote:
>On Thursday 15 June 2006 5:03 pm, Bernhard Fischer wrote:
>> Hi,
>>
>> The attached patch (which is only very lightly tested so far) does shave
>> some bloat off lash:
>>
>>    text	   data	    bss	    dec	    hex	filename
>>    7288	    168	     59	   7515	   1d5b	shell/lash.o.orig
>>    6927	    168	     59	   7154	   1bf2	shell/lash.o
>
>Nice.
>
>> 1) I'm not entirely sure that i got the bb_skip_whitespace stuff right,
>>    would be nice if someone could eye these for a second.
>> 2) Also i'm not sure if we want to put this into the repo just now or if
>>    we should wait for the 1.2 release-branch to be created and
>>    eventually apply it afterwards. I'll leave the decision whether to
>>    apply it for 1.2.0 or not to the maintainer..
>
>Since there's about a 50% chance lash will go away during 1.3 (replaced by 
>bbsh), might as well apply it now.
>
>Do the ATTRIBUTE_UNUSEDS actually save space, or is this fighting off 

It is silencing a warning about unused parameters, i.e. it sets
TREE_USED for these params to inhibit a warning.

>nonexistent compiler problems?  Because inside a function the compiler should 
>darn well be able to figure this stuff out on its own without annotation.  
>And if it's doing unit-at-a-time it should be able to figure this out for 
>static functions.

These are completely unrelated.

>
>Why do you do this sort of thing repeatedly?
>
>-				while(isalnum(*src) || *src=='_') src++;
>+				while((isalnum)(*src) || *src=='_') src++;

It's smaller to call the function in these places. It's not always
smaller to use the function as opposed to inlining the code via the
macro.
>
>You add #defines inside functions, which is just gross:
>
>+#define DONE (1)
>+#define SAW_QUOTE (2)

Why would that be gross?

Would you prefer global defines
#define LASH_OPT_DONE 1
#define LASH_OPT_SAW_QUOTE 2
>
>And this points out a bug in libbb:
>+	*command_ptr = (char*)bb_skip_whitespace(*command_ptr);
>
>The bug is that skip_whitespace has its return type declared const.  It has NO 
>BUSINESS making claims about what people who called it are going to do with 
>the data...  Ok, dealt with that.

Yes, that needed fixing.
>
>Huh.  I'm now about halfway through applying the lash cleanup, and the result 
>is noticeably _bigger_...

Every single hunk of the patch i sent made it smaller. Not sure what is
going on at your end..
>
>make bloatcheck
>function                                             old     new   delta
>run_command                                          271     983    +712
>builtin_export                                       373     366      -7
>builtin_read                                         382     366     -16
>builtin_source                                       229     204     -25
>.rodata                                           186778  186746     -32
>busy_loop                                           3857    3286    -571
>------------------------------------------------------------------------------
>(add/remove: 0/0 grow/shrink: 1/5 up/down: 712/-651)           Total: 61 bytes
>
>I have no idea why that is.  Attached is how far I got.  gcc 4.03 seems to 

You forgot the attachment..

>have inlined busy_loop in run_command (even though it's also called from 
>builtin_source...?)
>
>I'm going to go back to banging on the mdev thing now...
>
>Rob
>-- 
>Never bet against the cheap plastic solution.
>



More information about the busybox mailing list