[BusyBox] mv is deleting files!

[Jason Schoon]

I'll drop this patch onto my system and give it a try.  Looks like a
good solution for 1.01 at least.

On 7/14/05, Rob Landley <rob at landley.net> wrote:
> 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
>