[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