New version of tac.c

Natanael Copa natanael.copa at gmail.com
Wed Jan 9 02:11:04 PST 2008


On Tue, 2008-01-08 at 22:42 +0100, Tito wrote:
> Hi,
> this new version of tac.c handles embedded nulls in a correct way.
> I tested it in comparison with the real tac on most of the files 
> of my hard disk with the attached script and it seems to work.

Good work! It's nice to work with you :)

> Before posting a patch I would like the list members to take
> a look at this new approach for hints, improvements and critics.
> For the same reason I have not checked the size increase yet.

Is it not horribly slow to call xrealloc for every single char? Its
probably not a big deal for normal tac use (small files).

Should xrealloc really be used? (or should the comment about NOEXEC be
removed). I guess something like this is more appropiate if we want to
keep it as a NOEXEC app:
	line = realloc(...);
	if (line == NULL)
		goto clean_up_and_exit_with_err;


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 */
				...

(besides, the ' || ch == EOF' will never be evaulated since if ch is
EOF, then had the goto above already jumped over this part)

You test script didnt work so well for me. I attatched an improved one
(use 'cmp' rather than 'diff')

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


Bloatcheck on x86_64 compared to Tito's latest:
function                                             old     new   delta
tac_main                                             333     294     -39
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-39)             Total: -39
bytes
   text	   data	    bss	    dec	    hex	filename
  68352	   1374	    256	  69982	  1115e	busybox_old
  68313	   1374	    256	  69943	  11137	busybox_unstripped

Size reduction here is mostly due to use of a struct with both size and
buf, lstring.

> Ciao,
> Tito
> _______________________________________________
> busybox mailing list
> busybox at busybox.net
> http://busybox.net/cgi-bin/mailman/listinfo/busybox
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bb-tac2.patch
Type: text/x-patch
Size: 1812 bytes
Desc: not available
Url : http://busybox.net/lists/busybox/attachments/20080109/ea401a29/attachment.bin 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tac_test.sh
Type: application/x-shellscript
Size: 509 bytes
Desc: not available
Url : http://busybox.net/lists/busybox/attachments/20080109/ea401a29/attachment-0001.bin 


More information about the busybox mailing list