[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] mv: add --swap (-s) option to atomically swap 2 paths
From: |
Petr Malat |
Subject: |
Re: [PATCH] mv: add --swap (-s) option to atomically swap 2 paths |
Date: |
Sun, 3 Mar 2024 00:18:53 +0100 |
On Sat, Mar 02, 2024 at 01:59:31PM +0000, P??draig Brady wrote:
> On 02/03/2024 12:46, Dominique Martinet wrote:
> >
> > Thanks for remembering me; didn't try the patch yet but overall looks
> > good to me.
> >
> > Just one nitpick on the not supported message check:
> > P??draig Brady wrote on Sat, Mar 02, 2024 at 11:01:42AM +0000:
> > > + if (renameatu (AT_FDCWD, file[0], AT_FDCWD, file[1],
> > > + RENAME_EXCHANGE))
> > > + {
> > > + if (errno == EINVAL || is_ENOTSUP (errno))
> >
> > checking the man page, renameeat with RENAME_EXCHANGE on a dir and
> > something inside it also fails with EINVAL, so that will print the
> > operation isn't supported when it can probably be considered a user
> > error (I did't take the time to try this patch, using another tool to
> > illustrate but it should be the same):
> > $ mkdir a
> > $ touch a/b
> > $ ./renameat2 --exchange a a/b
> > renameat2: Invalid argument
>
> For comparison, mv now does:
>
> $ mv -x a a/b
> mv: atomic swap of 'a' and 'a/b' is not supported
>
> > I guess there's not much we can do about this though, EINVAL could also
> > really mean not supported in this case and checking if one path is a
> > prefix of another seems overkill to me...
>
> Right. Since the syscall doesn't distinguish errors in this case,
> our error is no better or worse I think. Having mv distinguish
> based on path processing would be overkill I agree.
> Also it's a bit of an edge case anyway.
We could call renameat2 with both arguments targeting the first file.
If it's supported, it's no-op, but if it's not supported, it returns
an error. Or some common file cold be used, e.g. /dev/null.
Petr
Re: [PATCH] mv: add --swap (-s) option to atomically swap 2 paths, Rob Landley, 2024/03/01
Re: [PATCH] mv: add --swap (-s) option to atomically swap 2 paths, Kaz Kylheku, 2024/03/01