[BusyBox] mv is deleting files!

[Rainer Weikusat]

Josh Malone <jmalone at applieddata.net> writes:

[...]

> So - it only seems to happen to me when I cross filesystems.  Sounds
> like a clue.  Can anybody try this under a newer busybox to see if it
> has been fixed?

Ups ... I should have thought of that myself ... anyway, there is no
need to "try" this and the buggy code is this (from mv.c):

	else if ((source_exists = cp_mv_stat(*argv, &source_stat)) >= 0) {
		if (dest_exists) {
			if (dest_exists == 3) {
				if (source_exists != 3) {
					bb_error_msg(fmt, "", "non-");
					goto RET_1;
				}
			} else {
				if (source_exists == 3) {
					bb_error_msg(fmt, "non-", "");
					goto RET_1;
				}
			}
			if (unlink(dest) < 0) {
				bb_perror_msg("cannot remove `%s'", dest);
				goto RET_1;
			}
		}

cp_mv_stat returns -1 in case of an error, 3 if the stat'et path name
was a directory, 1 if it existed and was not a directory and 0 if
the stat failed with errno set to ENOENT. The unlink gets execute
if the cp_mv_stat-return value is 0 or greater. A quick fix would
be change the

	if (unlink(dest) < 0)

to

	if (source_exists && unlink(dest) < 0)

but IMHO this is still wrong, because

	a) source may go away before it is copied and after
           dest is removed

	b) the copy may fail for "other reasons"

I would suggest that the unlink should be removed and replaced
by a copy to a temporary file and an (atomic) ln-call that causes dest
to go away after a copy of source is actually in place. But this is
not entirely trivial to implement due to a couple of corner cases. I
will nevertheless do so and post the patch to the list 'somewhat
later'.