[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#29961: [PATCH] mv -n: do not overwrite the destination
From: |
Kamil Dudka |
Subject: |
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
- bug#29961: [PATCH] mv: document the missing atomicity of 'mv -n', Kamil Dudka, 2018/01/03
- bug#29961: [PATCH] mv: document the missing atomicity of 'mv -n', Pádraig Brady, 2018/01/03
- bug#29961: [PATCH] mv: document the missing atomicity of 'mv -n', Kamil Dudka, 2018/01/03
- bug#29961: [PATCH] mv -n: do not overwrite the destination, Kamil Dudka, 2018/01/04
- bug#29961: [PATCH] mv -n: do not overwrite the destination, Paul Eggert, 2018/01/04
- bug#29961: [PATCH] mv -n: do not overwrite the destination,
Kamil Dudka <=
- bug#29961: [PATCH] mv -n: do not overwrite the destination, Paul Eggert, 2018/01/04
- bug#29961: [PATCH] mv -n: do not overwrite the destination, Kamil Dudka, 2018/01/05
- bug#29961: [PATCH] mv -n: do not overwrite the destination, Pádraig Brady, 2018/01/05
- bug#29961: [PATCH] mv -n: do not overwrite the destination, Kamil Dudka, 2018/01/05
- bug#29961: [PATCH] mv -n: do not overwrite the destination, Paul Eggert, 2018/01/05
- bug#29961: [PATCH] mv -n: do not overwrite the destination, Bernhard Voelker, 2018/01/06
- bug#29961: [PATCH] mv -n: do not overwrite the destination, Paul Eggert, 2018/01/06
- bug#29961: [PATCH] mv -n: do not overwrite the destination, Pádraig Brady, 2018/01/06
- bug#29961: [PATCH] mv -n: do not overwrite the destination, Paul Eggert, 2018/01/10
- bug#29961: [PATCH] mv -n: do not overwrite the destination, Kamil Dudka, 2018/01/10