bug#29961: [PATCH] mv -n: do not overwrite the destination

2018-01-10 Thread Kamil Dudka
On Wednesday, January 10, 2018 9:53:07 AM CET Paul Eggert wrote: > No further comment, so I installed the proposed patches and I'm closing this > bug report. Sorry. There were just too many changes in your patches for me to review them quickly enough (and most of the changes were unrelated to my

bug#29961: [PATCH] mv -n: do not overwrite the destination

2018-01-10 Thread Paul Eggert
No further comment, so I installed the proposed patches and I'm closing this bug report.

bug#29961: [PATCH] mv -n: do not overwrite the destination

2018-01-06 Thread Paul Eggert
Bernhard Voelker wrote: as we're changing the behavior for some cases - it would be good to also add some tests That would make sense if the edge-case behavior was documented and the change is something that users could rely on. But the behavior is not documented, and I'm not sure we want to

bug#29961: [PATCH] mv -n: do not overwrite the destination

2018-01-06 Thread Pádraig Brady
On 05/01/18 22:59, Paul Eggert wrote: > On 01/05/2018 08:19 AM, Kamil Dudka wrote: > >> I am only fixing the case where the destination file is created after the >> lstat() call. In that case, the only result is ENOENT, which is harmless. >> > > Ah, you're right. Sorry, I misread your patch. It

bug#29961: [PATCH] mv -n: do not overwrite the destination

2018-01-06 Thread Bernhard Voelker
On 01/05/2018 11:59 PM, Paul Eggert wrote: Attached is a revised proposed patchset to do that. I didn't have a look at the details, but - as we're changing the behavior for some cases - it would be good to also add some tests. Have a nice day, Berny

bug#29961: [PATCH] mv -n: do not overwrite the destination

2018-01-05 Thread Paul Eggert
On 01/05/2018 08:19 AM, Kamil Dudka wrote: I am only fixing the case where the destination file is created after the lstat() call. In that case, the only result is ENOENT, which is harmless. Ah, you're right. Sorry, I misread your patch. It should work. On Friday, January 5, 2018 4:29:55

bug#29961: [PATCH] mv -n: do not overwrite the destination

2018-01-05 Thread Kamil Dudka
On Friday, January 5, 2018 4:29:55 PM CET Pádraig Brady wrote: > Thanks to both of you. > The approaches can be summarized as: > > Orig: > - > stat() => ENOENT > > rename() may clobber file > > Kamil's: > - > stat() => ENOENT > >

bug#29961: [PATCH] mv -n: do not overwrite the destination

2018-01-05 Thread Pádraig Brady
On 05/01/18 11:56, Kamil Dudka wrote: > On Friday, January 5, 2018 2:00:52 AM CET Paul Eggert wrote: >> On 01/04/2018 03:01 AM, Kamil Dudka wrote: >>> On Thursday, January 4, 2018 10:48:56 AM CET Paul Eggert wrote: Kamil Dudka wrote: > - if (rename (src_name, dst_name) == 0) > +

bug#29961: [PATCH] mv -n: do not overwrite the destination

2018-01-05 Thread Kamil Dudka
On Friday, January 5, 2018 2:00:52 AM CET Paul Eggert wrote: > On 01/04/2018 03:01 AM, Kamil Dudka wrote: > > 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

bug#29961: [PATCH] mv -n: do not overwrite the destination

2018-01-04 Thread Paul Eggert
On 01/04/2018 03:01 AM, Kamil Dudka wrote: 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

bug#29961: [PATCH] mv -n: do not overwrite the destination

2018-01-04 Thread Kamil Dudka
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 > > */ +

bug#29961: [PATCH] mv -n: do not overwrite the destination

2018-01-04 Thread Paul Eggert
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

bug#29961: [PATCH] mv -n: do not overwrite the destination

2018-01-04 Thread Kamil Dudka
... if it is created by another process after mv has checked its non-existence. * src/copy.c (copy_internal): Use renameat2 (..., RENAME_NOREPLACE) if called by mv -n. If it fails with EEXIST in that case, pretend successful rename as if the existing destination file was detected by the