New version of tac.c

Denys Vlasenko vda.linux at googlemail.com
Wed Jan 9 15:04:32 PST 2008


On Wednesday 09 January 2008 13:50, Tito wrote:
> Previous version marked as noexec used xmalloc_fgets(f),
> so I thought it is ok to use xrealloc directly.

You can use xfuncs in NOEXEC, they are not allowed
only in NOFORK.
 
> > I dont know if it makes any difference in size but:
> >                         if (ch != EOF)
> > 	                        line[i++] = ch;
> >                         if (ch == '\n' || ch == EOF) {
> >                                 /* Save the size of the line to the list */
> > 				...
> > 
> > is more readable than:
> >                         if (ch == EOF)
> >                                 goto line_end;
> >                         line[i] = ch;
> >                         i++;
> >                         if (ch == '\n' || ch == EOF) {
> >  line_end:
> >                                 /* Save the size of the line to the list */
> > 				...
> 
> Haven't checked the size but your version looks cleaner.
> 
> > 
> > (besides, the ' || ch == EOF' will never be evaulated since if ch is
> > EOF, then had the goto above already jumped over this part)
> > 
> 
> Was a left-over of a previous version and could be removed.
> 
> > You test script didnt work so well for me. I attatched an improved one
> > (use 'cmp' rather than 'diff')
> 
> Yours work for me :-)
> 
> > 
> > I have added a patch (against svn).
> > 
> > Bloatcheck on x86_64 compared to current svn:
> > function                                             old     new   delta
> > tac_main                                             227     294     +67
> > xmalloc_fgets                                         19       -     -19
> > bb_get_chunk_from_file                               153       -    -153
> > ------------------------------------------------------------------------------
> > (add/remove: 0/2 grow/shrink: 1/0 up/down: 67/-172)          Total: -105
> > bytes
> >    text	   data	    bss	    dec	    hex	filename
> >   68517	   1374	    256	  70147	  11203	busybox_old
> >   68313	   1374	    256	  69943	  11137	busybox_unstripped

Nice.

I committed slightly modified version which doesn't realloc that often.
It's somewhat bigger :(

Thanks guys.
--
vda


More information about the busybox mailing list