[BusyBox] mv is deleting files!

[Rob Landley]

On Thursday 14 July 2005 10:38, Jason Schoon wrote:
> 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.

Cross-filesystem is the only way to trigger the actual copy logic.  Otherwise
mv is a rename syscall, and this ain't a kernel bug. :)

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

The bug is that our funky stat lib returns 0 if stat fails with errno=ENOENT,
but that this line:

else if ((source_exists = cp_mv_stat(*argv, &source_stat)) >= 0) {

Passes 0 through as a success.

Changing the >= to a > fixes the behavior, but then we don't get an error
message.  The interesting question is why the heck does cp_mv_stat.c want
to single out ENOENT?  The case I can think of is that when we can access a
file in a directory we can't list, we still want cp to work.  (mv doesn't care
so much, I'll have to test what the gnu version does in that case, but
failing entirely would be a pretty reasonable response...)

Okay, the response from the gnu tools to some of these corner cases is just
_weird_.  Try to chmod a directory to 444 and then ls its contents.  You get
a list of the files in it with "permission denied" for each one.  (Not even
ls -l, just ls, which should be doing about the same as "echo dir/*", which
works by the way...)  Needless to say, gnu "cat", "cp", and "mv" won't
touch 'em unless the directory they're in has the x bit.  So why are we
jumping through all these hoops to distinguish "didn't exist" with "couldn't
stat for a reason other than didn't exist?"

So: if we _just_ want to fix mv, then we can change the >= to > and add an
error message.  That's the 1.0.1 fix, and here's the least intrusive patch I
could come up with.

Index: coreutils/mv.c
===================================================================
--- coreutils/mv.c (revision 10824)
+++ coreutils/mv.c (working copy)
@@ -99,10 +99,10 @@
    struct stat source_stat;
    int source_exists;
 
-   if (errno != EXDEV) {
+   if (errno != EXDEV ||
+    (source_exists = cp_mv_stat(*argv, &source_stat)) < 1) {
     bb_perror_msg("unable to rename `%s'", *argv);
-   }
-   else if ((source_exists = cp_mv_stat(*argv, &source_stat)) >= 0) {
+   } else {
     if (dest_exists) {
      if (dest_exists == 3) {
       if (source_exists != 3) {

For 1.1, I think I'd like to take a stab at untangling this logic a bit...

Let me know if that fixes the problem and I'll throw it in 1.0.1-rc1.

Rob