[Toybox] New toy: tail

Rob Landley rob at landley.net
Fri Feb 24 09:48:17 PST 2012


On 02/23/2012 02:26 PM, Tim Elliott wrote:
> Hi all,
> 
> Attached is an implementation of tail. It is disabled by default
> because -f is not yet implemented.

Applied.  (Yesterday actually, I'm just slow rsyncing stuff up to the
server. :)

Generally what I've been doing with new commands is a quick 15-30 second
glance through the patch for basic sanity, apply it and make sure the
result compiles, and then commit.  Then when I get time I go through
line by line and optimize the code as much as I'm able to, but right now
I'm still only about 3/4 of the way through toys/kill.c (and haven't had
any time to look at it this lunch break; haven't even quite caught up on
email).

> This patch has a workaround for a limitation with toybox argument
> parsing. The -c and -n parameters are numeric arguments that can be
> prefixed with a + or - sign. If the sign is omitted, they default to
> negative. I would be happy to add something to the argument parser to
> handle this.

Yeah, I remember reading about that in the spec.

I could do a new "-" type.  Might be a touch confusing to use that
character, though.  I'll take a stab at that during my review pass this
weekend.

> This patch implements its own llist_add() helper. I wanted to put
> llist_add() in lib/llist.c, but I had difficulty getting the void
> pointer dereferencing right. Perhaps I should try again?

It's in toys/llist.c and it's dlist_add().  The singly linked lists I
generally just push at the head of, so it winds up in reverse order but
it's so little code to do it's not worth a function.  See
lib/getmountlist.c for an example.

When I need them to be in order, I do a doubly linked list. They start
out circular, and when you're done you can go list->prev->next=NULL to
break it, or just test for list->next == head as your termination
condition.  The current patch.c is doing that...

I should probably add some documentation about this to code.html...

> In certain situations, tail should seek and read from the end of the
> input. This current implementation always reads from the beginning.

Yeah, I remember a patch for that coming in when I maintained busybox.
The performance hit on large files of reading the whole thing is
significant, and worth the extra complexity. (Alas, you can't _always_
seek because "cat | tail" has to work.  It really does require two
codepaths.)

Rob

 1330105697.0


More information about the Toybox mailing list