[patch] misc minor shrinkage and smalltype question

Denis Vlasenko vda.linux at googlemail.com
Sat Jan 20 20:58:15 UTC 2007


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.
 
-       smallint need_another_block;
-       smallint end_reached;
+       bool need_another_block;
+       bool end_reached;
-static int inflate_block(STATE_PARAM smallint *e)
+static int inflate_block(STATE_PARAM bool *e)
...[rest skipped]...

Same as above.

-       char status = EXIT_SUCCESS;
-       char in_word;
+       smallint status = EXIT_SUCCESS;
+       smallint in_word;

Definitely agree on these!

-static int read_stduu(FILE *src_stream, FILE *dst_stream)
+static void read_stduu(FILE *src_stream, FILE *dst_stream)
...
-                       return EXIT_SUCCESS;
+                       return;

Yes, makes sense (read_base64 too).

-       char term_count = 0;
+       int term_count = 0;

ok.


--- coreutils/tee.c     (revision 17363)
+++ coreutils/tee.c     (working copy)
@@ -20,23 +20,23 @@ int tee_main(int argc, char **argv)
        FILE **fp;
        char **names;
        char **np;
-       int flags;
-       int retval = EXIT_SUCCESS;
+       char retval;
 #if ENABLE_FEATURE_TEE_USE_BLOCK_IO
        ssize_t c;
 # define buf bb_common_bufsiz1
 #else
        int c;
 #endif
-       flags = getopt32(argc, argv, "ia");     /* 'a' must be 2nd */
+       retval = getopt32(argc, argv, "ia");    /* 'a' must be 2nd */
        argc -= optind;
        argv += optind;
 
-       mode += (flags & 2);    /* Since 'a' is the 2nd option... */
+       mode += (retval & 2);   /* Since 'a' is the 2nd option... */
 
-       if (flags & 1) {
+       if (retval & 1) {
                signal(SIGINT, SIG_IGN); /* TODO - switch to sigaction. */
        }
+       retval = EXIT_SUCCESS;
        /* gnu tee ignores SIGPIPE in case one of the output files is a pipe
         * that doesn't consume all its input.  Good idea... */
        signal(SIGPIPE, SIG_IGN);       /* TODO - switch to sigaction. */

ok.

Index: coreutils/tty.c
===================================================================
--- coreutils/tty.c     (revision 17363)
+++ coreutils/tty.c     (working copy)
@@ -18,12 +18,12 @@
 int tty_main(int argc, char **argv)
 {
        const char *s;
-       int silent;             /* Note: No longer relevant in SUSv3. */
+       USE_INCLUDE_SUSv2(int silent;)  /* Note: No longer relevant in SUSv3. */
        int retval;
 
        xfunc_error_retval = 2; /* SUSv3 requires > 1 for error. */
 
-       silent = getopt32(argc, argv, "s");
+       USE_INCLUDE_SUSv2(silent = getopt32(argc, argv, "s");)
 
        /* gnu tty outputs a warning that it is ignoring all args. */
        bb_warn_ignoring_args(argc - optind);
@@ -36,10 +36,8 @@ int tty_main(int argc, char **argv)
                s = "not a tty";
                retval = 1;
        }
-
-       if (!silent) {
-               puts(s);
-       }
+       USE_INCLUDE_SUSv2(if (!silent) puts(s);)
+       SKIP_INCLUDE_SUSv2(puts(s);)
 
        fflush_stdout_and_exit(retval);
 }

ok


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?

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


-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?

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


More information about the busybox mailing list