[BusyBox] mv is deleting files!

[Jason Schoon]

I actually saw this issue some time ago and posted it to the list, but
never got back to it.  The cross-filesystem method (I was using tmpfs
to jffs2 as well) was the only way I could reproduce the issue as
well.

If you come up with a patch, I will happily try it out on my system here.


On 7/14/05, Rainer Weikusat <rainer.weikusat at sncag.com> wrote:
> 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'.
> 
> 
> 
> 
> 
> 
> 
> _______________________________________________
> busybox mailing list
> busybox at mail.busybox.net
> http://busybox.net/mailman/listinfo/busybox
> 
> 
>