On 11/01/2012 08:16:39 PM, Rob Landley wrote:
On 10/28/2012 10:42:32 PM, Roy Tam wrote:
It is broken for months, so I hackfixed it.
Sigh. Good point.
I was fixing cp properly before work and email and life went nuts.
Let's see...
TT.destname isn't getting initialized. Last commit that worked (before
the dirtree changes) was 528, so comparing with that. What was I
thinking?
http://landley.net/hg/toybox/file/538/toys/cp.c
First line of cp_main() used to be TT.destname =
toys.optargs[--toys.optc]; and the reason I removed that was:
A) I was thinking about someday adding NOFORK capability (the ability
for toysh or similar to just call cp_main() as a function, probably
through some variant of toy_exec(), and clean up after it in the same
process context without fork/exec overhead). This meant the ability to
free the argument memory, so permuting the arguments so it lost track
of one and potentially leaked memory would be a downside. But that's
bogus because toys.argc still has them all.
B) Adding the ability to tell the optstring to parse non-option
arguments into the global block, so the <2 fixed arguments would wing
up in TT without needing to do it by hand. (The idea was getting it to
automatically do the type conversions.) I poked at this a bit, but
couldn't work out a good way to express it that actually reduced
complexity.
So, putting that back in...
Next up, TT.destisnew was only ever set, not read, so I yanked that
case, tested for the other one, and unified the else cases to avoid the
goto that was combining the two error_exit() cases.
That gets us to the TODO, which is about this problem:
readlink -f ./nonexistent : prints absolute path of where it _would_
go.
realpath("./nonexistent", NULL): returns NULL
This is actually why toybox's readlink defaults to "n" for now, I need
to make this case work.
However, what I'm trying to do here is duplicate detection, as
mentioned in the previous reply...
Rob
diff -U8 toys/posix/cp.c toys/posix/cp.c_hf
--- toys/posix/cp.c 2012-10-27 10:15:31.000000000 +0800
+++ toys/posix/cp.c_hf 2012-10-29 11:41:01.000000000 +0800
@@ -171,37 +171,37 @@
// Loop through sources
for (i=0; i<toys.optc; i++) {
char *dst, *src = toys.optargs[i];
// Skip src==dest (TODO check inodes to catch "cp
blah ./blah").
- if (!strncmp(src, TT.destname)) continue;
+ if (!strcmp(src, TT.destname)) continue;
Sigh. Looks like I checked in half-finished work at some point.
(Possibly when I categorized commands into subdirectories.)
What I was going for here is trying to catch destinations that are
_under_ source, ala:
cp -r dir dir/sub
If I'm using cannonicalized absolute paths (with all the symlinks
resolved), then a
matching prefix tells me I've got an endless loop coming up. But I
also need to catch:
cp * .
Last I poked at this, I'm was trying to work out what all the cases
were and a
graceful way to catch them without a forest of special cases...
// Skip nonexistent sources.
TT.keep_symlinks = toys.optflags & (FLAG_d|FLAG_a);
if (TT.keep_symlinks ? lstat(src, &st) : stat(src,
&st)
- || (st.st_dev = dst.st_dev && st.st_ino ==
dst.dst_ino))
+ /*|| (st.st_dev = dst.st_dev && st.st_ino ==
dst.dst_ino)*/)
Another variant of the same thing. I was stepping back and trying to
collate it all into one test.
Unfortunately, "endless recursion with -r" and "copy over itself"
seem like the need to be two separate tests.
{
objection:
perror_msg("bad '%s'", src);
toys.exitval = 1;
continue;
}
// Copy directory or file.
if (TT.destisdir) {
char *s;
// Catch "cp -R .. ." and friends that would
go on forever
- if (dpath && (s = realpath(src, NULL)) {
+ if (dpath && (s = realpath(src, NULL))) {
Ok, that's just embarassing. But it also means "I never actually
tested this code, and it needs a largeish test suite entry because
it's a pile of corner cases with a bunch of command line options".
I'm poking at it.
Thanks,
Rob
_______________________________________________
Toybox mailing list
[email protected]
http://lists.landley.net/listinfo.cgi/toybox-landley.net
