GNU bug report logs - #29961
[PATCH] mv: document the missing atomicity of 'mv -n'

Previous Next

Full log

Message #26 received at 29961 <at> debbugs.gnu.org (full text, mbox):

From: Kamil Dudka <kdudka <at> redhat.com>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 29961 <at> debbugs.gnu.org, Pádraig Brady <P <at> draigbrady.com>
Subject: Re: bug#29961: [PATCH] mv -n: do not overwrite the destination
Date: Thu, 04 Jan 2018 12:01:28 +0100

On Thursday, January 4, 2018 10:48:56 AM CET Paul Eggert wrote:
> Kamil Dudka wrote:
> > -      if (rename (src_name, dst_name) == 0)
> > +      int flags = 0;
> > +      if (x->interactive == I_ALWAYS_NO)
> > +        /* do not replace DST_NAME if it was created since our last check
> > */ +        flags = RENAME_NOREPLACE;
> 
> By then it's too late, as multiple decisions have been made on the basis of
> stat/lstat calls that are still subject to races.

Do you mean in the case of mv -n?  Which decisions exactly?

> It's better to try the
> renameat2 with RENAME_NOREPLACE first thing, and fall back on the existing
> code only if renameat2 fails with errno != EEXIST.
> 
> Plus, there are some other problems when combining -u and -n.

Sounds like a corner case.  Please consider writing a separate patch for that.

> How about the attached patch instead?

I had difficulties trying to evaluate the patch.  It does not compile
with gcc-7.2.1:

src/copy.c: In function 'copy_internal':
src/copy.c:2340:17: error: 'earlier_file' may be used uninitialized in this function [-Werror=maybe-uninitialized]
           if (! create_hard_link (earlier_file, dst_name, true, x->verbose,
                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                   dereference))
                                   ~~~~~~~~~~~~
src/copy.c:2050:14: error: 'return_now' may be used uninitialized in this function [-Werror=maybe-uninitialized]
           if (return_now)
              ^

So I added initializers to those two variables but then I saw multiple
test-cases failing because of the patch:

FAIL: tests/mv/hard-link-1
FAIL: tests/mv/mv-special-1
FAIL: tests/mv/part-fail
FAIL: tests/mv/part-hardlink
FAIL: tests/mv/part-rename
FAIL: tests/mv/part-symlink

Kamil

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.