On 12/15/2012 01:06:21 PM, Elie De Brauwer wrote:
Hi Rob and all,
I've been playing around with 'tac'. And as a result you can find two
patches
in attach. The first one removes some code from tac for that simple
reason that
the tac toy contains some logic (including a strlen) to remove a
trailing \n
(if present). But this \n is the seperator which get_line()
internally passes
to get_rawline() and that newline is already removed in get_line().
Sorry for the delay answering, bit crazy around here. I merged the
patches you sent, and I think I'm caught up now?
Next to this you can find some test for tac which are based upon the
cat tests. I
have however found two problems I'd like to throw in the group.
Problem 1:
On my system:
edb@lapedb:~/today$ echo -ne "abc\ndef" | tac
defabc\n
tac in toybox:
edb@lapedb:~/edb-stuff/toybox/toybox$ echo -ne "abc\ndef" | ./toybox
tac
def\n
abc\n
(I added the \n here for visibility). This however cannot be
trivially fixed because
of the implicit separator in get_line() as stated above. Nevertheless
the output of
the 'default' tac is such a corner case that probably no sane human
being would use
it that way (I hope).
We should make no newline at end of file work. I note that get_line()
dates back to before posix-2008, which standardized getline() and
getdelim(). It persists because it uses a file descripter instead of a
FILE *, but that's not necessarily enough justification. Taking another
look at this is on my todo list, but pretty far down.
In any case, it's a wrapper around get_rawline(), which we should
probably be using here.
Problem 2:
cat has a test which writes to /dev/full to see if the error gets
caught. cat however
doesn't use xwrite but uses xputs to put its output (also a cause for
problem 1). But
take a look at the following strace output of
echo "boe" | strace ./toybox tac > /dev/full
read(0, "b", 1) = 1
read(0, "o", 1) = 1
read(0, "e", 1) = 1
read(0, "\n", 1) = 1
read(0, "", 1) = 0
fstat64(1, {st_mode=S_IFCHR|0666, st_rdev=makedev(1, 7), ...}) = 0
ioctl(1, SNDCTL_TMR_TIMEBASE or TCGETS, 0xbfb6ae50) = -1 ENOTTY
(Inappropriate ioctl for device)
mmap2(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS,
-1, 0) = 0xb773d000
write(1, "boe\n", 4) = -1 ENOSPC (No space left on
device)
exit_group(0) = ?
True, xputs uses puts which ends up calling write() which returns -1
but this does
not ripple through to puts() and thus the error condition in xputs()
doesn't trigger
(it just happily return a '4').
Wow. That sounds like a libc bug? (Which libc are you using?)
So in order to fix the issues above I would make 'tac' more like
'cat', which would
probably mean not using get_line() but using get_rawline() (and thus
reintroducing
what I threw out in my patch ;-) ), not removing the newline and
using xwrite()
(eventually storing the length somewhere) to write the output. This
would still not
solve the root problem of xputs() not triggering when write throws
and ENOSPC at you.
What do you guys think ?
If xputs() doesn't work we should redo xputs() not to use puts(). I've
got it because puts is a common case that shaves several bytes vs
xprintf("%s\n", str), but it needs to work properly.
Rob
_______________________________________________
Toybox mailing list
[email protected]
http://lists.landley.net/listinfo.cgi/toybox-landley.net
