[patch] misc minor shrinkage and smalltype question

[Bernhard Fischer]

On Sat, Jan 20, 2007 at 09:58:15PM +0100, Denis Vlasenko wrote:
>On Saturday 20 January 2007 21:22, Bernhard Fischer wrote:
>> Hi,
>> 
>> I'm attaching misc small shrinkage and wanted to know if it's ok to add
>> and use the hunks against platform.h
>> 
>> Also, why was read_stduu()/read_base64() returning an int?
>> 
>> Thoughts?
>
>-       smallint resume_copy;
>+       bool resume_copy;
>
>Is it smaller or same codesize-wise?
>
>I'm a little but worried about this... do you trust gcc
>that gcc's idea of where it makes sense to use byte-sized
>storage will always match yours? What if on some arch
>gcc will insist on using int sized one? I prefer to have
>more control.

Well, for some, using bool instead of "smallint" or the "u" variant is
definately benefical.

[strip agreements]
>Index: libbb/xfuncs.c
>===================================================================
>--- libbb/xfuncs.c      (revision 17363)
>+++ libbb/xfuncs.c      (working copy)
>@@ -564,7 +564,7 @@ void xstat(char *name, struct stat *stat
> 
> /* It is perfectly ok to pass in a NULL for either width or for
>  * height, in which case that value will not be set.  */
>-int get_terminal_width_height(int fd, int *width, int *height)
>+int get_terminal_width_height(const int fd, int *width, int *height)
>
>Does const help? Smaller code or what?

Here it is used to make sure that noone ever has the idea to write to
it. Precaution.
>
> {
>        struct winsize win = { 0, 0, 0, 0 };
>        int ret = ioctl(fd, TIOCGWINSZ, &win);
>@@ -572,7 +572,8 @@ int get_terminal_width_height(int fd, in
>        if (height) {
>                if (!win.ws_row) {
>                        char *s = getenv("LINES");
>-                       if (s) win.ws_row = atoi(s);
>+                       if (s)
>+                               win.ws_row = atoi(s); /* XXX: xatoi_u(s) ? */
>                }
>                if (win.ws_row <= 1 || win.ws_row >= 30000)
>                        win.ws_row = 24;
>@@ -582,7 +583,8 @@ int get_terminal_width_height(int fd, in
>        if (width) {
>                if (!win.ws_col) {
>                        char *s = getenv("COLUMNS");
>-                       if (s) win.ws_col = atoi(s);
>+                       if (s)
>+                               win.ws_col = atoi(s); /* XXX: xatoi_u(s) ? */
>                }
>                if (win.ws_col <= 1 || win.ws_col >= 30000)
>                        win.ws_col = 80;
>
>
>Hard to decide on this one. Maybe it's better to default to
>standard size (24 rows/80 cols) if conversion fails,
>so that applets won't mysteriously die left and right.
>You will need to use bb_strtou here then.
>Pity, code will be larger...

So let's leave it as it currently is since that's good enough for now.
>
>
>-CFLAGS += \
>-       -Wall -Wstrict-prototypes -Wshadow -Werror -Wundef \
>+# all these _have_to_ be run through (one) check_cc call !
>+# I'd split them into one check for all the -W and group optimizations into
>+# separate calls.
>+# Same holds true for the -std=gnu99 in CPPFLAGS..
>+CFLAGS += -frtl-abstract-sequences \
>+       -Wall -Wstrict-prototypes -Wshadow -Wno-error -Wundef \
>        -funsigned-char -fno-builtin-strlen -finline-limit=0 -static-libgcc \
>        -Os -falign-functions=1 -falign-jumps=1 -falign-loops=1 \
>        -fomit-frame-pointer -ffunction-sections -fdata-sections
>
>-frtl-abstract-sequences, -Wno-error - what they are doing?

Wno-error turns off -Werror. I should have removed it alltogether
instead.

       -frtl-abstract-sequences
           It is a size optimization method. This option is to find identical
           sequences of code, which can be turned into pseudo-procedures and
           then  replace  all  occurrences with  calls to  the  newly created
           subroutine. It is kind of an opposite of -finline-functions.  This
           optimization runs at RTL level.

It saved more than 2k for me. Quick'n easy :)
>
>BTW my gcc 4.1.1 gives me strange small variations whenever I touch
>headers, in _totally_ unrelated places.
>I thought -fno-guess-branch-probability will help (that one
>said to disable non-deterministic (i.e. semirandom) branch probability
>prediction by gcc), but no...

I think there is something amiss with the dependency tracking, too.
With the tty changes from above, the tty.o was not reliably rebuilt if
one toggles the SUSv2 .config option

Thanks for your comments and cheers for the 1.4.0 release! :)