coreutils
[Top][All Lists]
Advanced

[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: Pádraig Brady
Subject: Re: [PATCH] mv: add --swap (-s) option to atomically swap 2 paths
Date: Sat, 2 Mar 2024 13:59:31 +0000
User-agent: Mozilla Thunderbird

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.

cheers,
Pádraig



reply via email to

[Prev in Thread] Current Thread [Next in Thread]