renameat2
Linux has a system call 'renameat2' which is like renameat but has an extra
'flags' argument. In particular, one can pass the RENAME_NOREPLACE flag to
cause the rename to fail with EEXIST if the target of the rename exists. See
http://man7.org/linux/man-pages/man2/rename.2.html
macOS has a similar functionality, provided by the function 'renameatx_np' with
the flag RENAME_EXCL.
There's also a recently introduced Gnulib module 'renameat2', but it requires
two system calls on Cygwin (one to test existence and the second to do the
rename), so that there is a race condition. On Linux and macOS it uses
renameat2 and renameatx_np to avoid the race.
The attached patch implements renameat2 on Cygwin (but only supporting the
RENAME_NOREPLACE flag). I've written it so that a rename that just changes
case on a case-insensitive file system succeeds.
If the patch is accepted, I'll submit a second patch that documents the new
function.
Here's a simple test program:
$ cat rename_noreplace.c
#include
#include
#include
int
main ()
{
int res = renameat2 (AT_FDCWD, "foo", AT_FDCWD, "bar", RENAME_NOREPLACE);
if (res < 0)
perror ("renameat2");
}
$ gcc -o rename_noreplace rename_noreplace.c
$ touch foo bar
$ ./rename_noreplace.exe
renameat2: File exists
$ ls foo bar
bar foo
$ rm bar
$ ./rename_noreplace.exe
$ ls foo bar
ls: cannot access 'foo': No such file or directory
bar
Ken
From 23edb25c9c0b6a21aefb10372db945807b2b8e3f Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Thu, 17 Aug 2017 09:12:15 -0400
Subject: [PATCH] cygwin: Implement renameat2
Define the RENAME_* flags in as defined on Linux in
, but support only RENAME_NOREPLACE.
---
newlib/libc/include/stdio.h| 3 +++
winsup/cygwin/common.din | 1 +
winsup/cygwin/include/cygwin/fs.h | 5 +
winsup/cygwin/include/cygwin/version.h | 4 +++-
winsup/cygwin/syscalls.cc | 41 +-
5 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/newlib/libc/include/stdio.h b/newlib/libc/include/stdio.h
index 5d8cb1092..331a1cf07 100644
--- a/newlib/libc/include/stdio.h
+++ b/newlib/libc/include/stdio.h
@@ -384,6 +384,9 @@ int _EXFUN(vdprintf, (int, const char *__restrict, __VALIST)
#endif
#if __ATFILE_VISIBLE
int_EXFUN(renameat, (int, const char *, int, const char *));
+# ifdef __CYGWIN__
+int_EXFUN(renameat2, (int, const char *, int, const char *, unsigned int));
+# endif
#endif
/*
diff --git a/winsup/cygwin/common.din b/winsup/cygwin/common.din
index 8da432b8a..ca6ff3cf9 100644
--- a/winsup/cygwin/common.din
+++ b/winsup/cygwin/common.din
@@ -1168,6 +1168,7 @@ remquof NOSIGFE
remquol NOSIGFE
rename SIGFE
renameat SIGFE
+renameat2 SIGFE
res_close = __res_close SIGFE
res_init = __res_init SIGFE
res_mkquery = __res_mkquery SIGFE
diff --git a/winsup/cygwin/include/cygwin/fs.h
b/winsup/cygwin/include/cygwin/fs.h
index f606ffc39..14454376f 100644
--- a/winsup/cygwin/include/cygwin/fs.h
+++ b/winsup/cygwin/include/cygwin/fs.h
@@ -19,4 +19,9 @@ details. */
#define BLKPBSZGET 0x127b
#define BLKGETSIZE64 0x00041268
+/* Flags for renameat2, from /usr/include/linux/fs.h. */
+#define RENAME_NOREPLACE (1 << 0)
+#define RENAME_EXCHANGE (1 << 1)
+#define RENAME_WHITEOUT (1 << 2)
+
#endif
diff --git a/winsup/cygwin/include/cygwin/version.h
b/winsup/cygwin/include/cygwin/version.h
index efd4ac017..7640abfad 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -481,12 +481,14 @@ details. */
314: Export explicit_bzero.
315: Export pthread_mutex_timedlock.
316: Export pthread_rwlock_timedrdlock, pthread_rwlock_timedwrlock.
+ 317: Export renameat2. Add RENAME_NOREPLACE, RENAME_EXCHANGE,
+ RENAME_WHITEOUT.
Note that we forgot to bump the api for ualarm, strtoll, strtoull,
sigaltstack, sethostname. */
#define CYGWIN_VERSION_API_MAJOR 0
-#define CYGWIN_VERSION_API_MINOR 316
+#define CYGWIN_VERSION_API_MINOR 317
/* There is also a compatibity version number associated with the shared memory
regions. It is incremented when incompatible changes are made to the shared
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 885931632..d756f5f35 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -60,6 +60,7 @@ details. */
#include "tls_pbuf.h"
#include "sync.h"
#include "child_info.h"
+#include /* needed for RENAME_NOREPLACE */
#undef _close
#undef _lseek
@@ -2048,8 +2049,12 @@ nt_path_has_executable_suffix (PUNICODE_STRING upath)
return false;
}
-extern "C" int
-rename (const char *oldpath, const char *newpath)
+/* If newpath names an existing file and the RENAME_NOREPLACE flag is
+ specified, fail with EEXIST. Exception: Don't fail if the purpose
+ of the rename is just to change the case of oldpath on a
+ case-insensitive file system. */
+static int
+rename2 (const char *oldpath, const char *newpath, unsigne
Re: renameat2
On 08/18/2017 08:21 AM, Ken Brown wrote: > Linux has a system call 'renameat2' which is like renameat but has an > extra 'flags' argument. In particular, one can pass the > RENAME_NOREPLACE flag to cause the rename to fail with EEXIST if the > target of the rename exists. See > > http://man7.org/linux/man-pages/man2/rename.2.html > > macOS has a similar functionality, provided by the function > 'renameatx_np' with the flag RENAME_EXCL. > > > Define the RENAME_* flags in as defined on Linux in > , but support only RENAME_NOREPLACE. > --- > +++ b/winsup/cygwin/include/cygwin/fs.h > @@ -19,4 +19,9 @@ details. */ > #define BLKPBSZGET 0x127b > #define BLKGETSIZE64 0x00041268 > > +/* Flags for renameat2, from /usr/include/linux/fs.h. */ > +#define RENAME_NOREPLACE (1 << 0) > +#define RENAME_EXCHANGE (1 << 1) > +#define RENAME_WHITEOUT (1 << 2) Please only define RENAME_NOREPLACE for now; that way, software can probe what is defined to know what will work (defining a flag that will always be an error is not as useful as leaving it undefined - and while we may add RENAME_EXCHANGE support, I don't see how we can ever do RENAME_WHITEOUT). > + > #endif > diff --git a/winsup/cygwin/include/cygwin/version.h > b/winsup/cygwin/include/cygwin/version.h > index efd4ac017..7640abfad 100644 > --- a/winsup/cygwin/include/cygwin/version.h > +++ b/winsup/cygwin/include/cygwin/version.h > @@ -481,12 +481,14 @@ details. */ >314: Export explicit_bzero. >315: Export pthread_mutex_timedlock. >316: Export pthread_rwlock_timedrdlock, pthread_rwlock_timedwrlock. > + 317: Export renameat2. Add RENAME_NOREPLACE, RENAME_EXCHANGE, > + RENAME_WHITEOUT. This needs tweaks to match. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org signature.asc Description: OpenPGP digital signature
Re: renameat2
On 8/18/2017 10:33 AM, Eric Blake wrote:
Please only define RENAME_NOREPLACE for now; that way, software can
probe what is defined to know what will work (defining a flag that will
always be an error is not as useful as leaving it undefined - and while
we may add RENAME_EXCHANGE support, I don't see how we can ever do
RENAME_WHITEOUT).
Thanks for the feedback. Revised patch attached.
From 136b0dfd53e147002e134048658d20f452402c9f Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Thu, 17 Aug 2017 09:12:15 -0400
Subject: [PATCH] cygwin: Implement renameat2
Define the RENAME_NOREPLACE flag in as defined on Linux
in . The other RENAME_* flags defined on Linux are not
supported.
---
newlib/libc/include/stdio.h| 3 +++
winsup/cygwin/common.din | 1 +
winsup/cygwin/include/cygwin/fs.h | 6 +
winsup/cygwin/include/cygwin/version.h | 3 ++-
winsup/cygwin/syscalls.cc | 41 +-
5 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/newlib/libc/include/stdio.h b/newlib/libc/include/stdio.h
index 5d8cb1092..331a1cf07 100644
--- a/newlib/libc/include/stdio.h
+++ b/newlib/libc/include/stdio.h
@@ -384,6 +384,9 @@ int _EXFUN(vdprintf, (int, const char *__restrict, __VALIST)
#endif
#if __ATFILE_VISIBLE
int_EXFUN(renameat, (int, const char *, int, const char *));
+# ifdef __CYGWIN__
+int_EXFUN(renameat2, (int, const char *, int, const char *, unsigned int));
+# endif
#endif
/*
diff --git a/winsup/cygwin/common.din b/winsup/cygwin/common.din
index 8da432b8a..ca6ff3cf9 100644
--- a/winsup/cygwin/common.din
+++ b/winsup/cygwin/common.din
@@ -1168,6 +1168,7 @@ remquof NOSIGFE
remquol NOSIGFE
rename SIGFE
renameat SIGFE
+renameat2 SIGFE
res_close = __res_close SIGFE
res_init = __res_init SIGFE
res_mkquery = __res_mkquery SIGFE
diff --git a/winsup/cygwin/include/cygwin/fs.h
b/winsup/cygwin/include/cygwin/fs.h
index f606ffc39..48b0cca45 100644
--- a/winsup/cygwin/include/cygwin/fs.h
+++ b/winsup/cygwin/include/cygwin/fs.h
@@ -19,4 +19,10 @@ details. */
#define BLKPBSZGET 0x127b
#define BLKGETSIZE64 0x00041268
+/* Flags for renameat2, from /usr/include/linux/fs.h. For now we
+ support only RENAME_NOREPLACE. */
+#define RENAME_NOREPLACE (1 << 0)
+/* #define RENAME_EXCHANGE (1 << 1) */
+/* #define RENAME_WHITEOUT (1 << 2) */
+
#endif
diff --git a/winsup/cygwin/include/cygwin/version.h
b/winsup/cygwin/include/cygwin/version.h
index efd4ac017..0384f77da 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -481,12 +481,13 @@ details. */
314: Export explicit_bzero.
315: Export pthread_mutex_timedlock.
316: Export pthread_rwlock_timedrdlock, pthread_rwlock_timedwrlock.
+ 317: Export renameat2. Add RENAME_NOREPLACE.
Note that we forgot to bump the api for ualarm, strtoll, strtoull,
sigaltstack, sethostname. */
#define CYGWIN_VERSION_API_MAJOR 0
-#define CYGWIN_VERSION_API_MINOR 316
+#define CYGWIN_VERSION_API_MINOR 317
/* There is also a compatibity version number associated with the shared memory
regions. It is incremented when incompatible changes are made to the shared
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index 885931632..d756f5f35 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -60,6 +60,7 @@ details. */
#include "tls_pbuf.h"
#include "sync.h"
#include "child_info.h"
+#include /* needed for RENAME_NOREPLACE */
#undef _close
#undef _lseek
@@ -2048,8 +2049,12 @@ nt_path_has_executable_suffix (PUNICODE_STRING upath)
return false;
}
-extern "C" int
-rename (const char *oldpath, const char *newpath)
+/* If newpath names an existing file and the RENAME_NOREPLACE flag is
+ specified, fail with EEXIST. Exception: Don't fail if the purpose
+ of the rename is just to change the case of oldpath on a
+ case-insensitive file system. */
+static int
+rename2 (const char *oldpath, const char *newpath, unsigned int flags)
{
tmp_pathbuf tp;
int res = -1;
@@ -2068,6 +2073,12 @@ rename (const char *oldpath, const char *newpath)
__try
{
+ if (flags & ~RENAME_NOREPLACE)
+ /* RENAME_NOREPLACE is the only flag currently supported. */
+ {
+ set_errno (ENOTSUP);
+ __leave;
+ }
if (!*oldpath || !*newpath)
{
/* Reject rename("","x"), rename("x",""). */
@@ -2337,6 +2348,13 @@ rename (const char *oldpath, const char *newpath)
__leave;
}
+ /* Should we replace an existing file? */
+ if ((removepc || dstpc->exists ()) && (flags & RENAME_NOREPLACE))
+ {
+ set_errno (EEXIST);
+ __leave;
+ }
+
/* Opening the file must be part of the transaction. It's not sufficient
to call only NtSetInformationFile under the transaction. Therefore we
have to start the transaction here, if necessary. */
@@ -2578,6 +2596,12 @@ rename (con
Re: renameat2
Hi Ken,
On Aug 18 09:21, Ken Brown wrote:
> Linux has a system call 'renameat2' which is like renameat but has an
> extra 'flags' argument. In particular, one can pass the
> RENAME_NOREPLACE flag to cause the rename to fail with EEXIST if the
> target of the rename exists. See
>
> http://man7.org/linux/man-pages/man2/rename.2.html
>
> macOS has a similar functionality, provided by the function
> 'renameatx_np' with the flag RENAME_EXCL.
>
> There's also a recently introduced Gnulib module 'renameat2', but it
> requires two system calls on Cygwin (one to test existence and the
> second to do the rename), so that there is a race condition. On Linux
> and macOS it uses renameat2 and renameatx_np to avoid the race.
>
> The attached patch implements renameat2 on Cygwin (but only supporting
> the RENAME_NOREPLACE flag). I've written it so that a rename that
> just changes case on a case-insensitive file system succeeds.
>
> If the patch is accepted, I'll submit a second patch that documents
> the new function.
Neat stuff, but there are a few points for discussion, see below.
> --- a/winsup/cygwin/include/cygwin/fs.h
> +++ b/winsup/cygwin/include/cygwin/fs.h
> @@ -19,4 +19,9 @@ details. */
> #define BLKPBSZGET 0x127b
> #define BLKGETSIZE64 0x00041268
>
> +/* Flags for renameat2, from /usr/include/linux/fs.h. */
> +#define RENAME_NOREPLACE (1 << 0)
> +#define RENAME_EXCHANGE (1 << 1)
> +#define RENAME_WHITEOUT (1 << 2)
Given that there's no standard for this call (yet), do we really want to
expose flag values we don't support? I would opt for only RENAME_NOREPLACE
for now and skip the others.
> +
> #endif
> diff --git a/winsup/cygwin/include/cygwin/version.h
> b/winsup/cygwin/include/cygwin/version.h
> index efd4ac017..7640abfad 100644
> --- a/winsup/cygwin/include/cygwin/version.h
> +++ b/winsup/cygwin/include/cygwin/version.h
> @@ -481,12 +481,14 @@ details. */
>314: Export explicit_bzero.
>315: Export pthread_mutex_timedlock.
>316: Export pthread_rwlock_timedrdlock, pthread_rwlock_timedwrlock.
> + 317: Export renameat2. Add RENAME_NOREPLACE, RENAME_EXCHANGE,
> + RENAME_WHITEOUT.
You can drop the flag values here. renameat2 is sufficient.
> +rename2 (const char *oldpath, const char *newpath, unsigned int flags)
> {
>tmp_pathbuf tp;
>int res = -1;
> @@ -2068,6 +2073,12 @@ rename (const char *oldpath, const char *newpath)
>
>__try
> {
> + if (flags & ~RENAME_NOREPLACE)
> + /* RENAME_NOREPLACE is the only flag currently supported. */
> + {
> + set_errno (ENOTSUP);
That should ideally be EINVAL. Unsupported bit values in a flag argument?
EINVAL, please.
> + __leave;
> + }
>if (!*oldpath || !*newpath)
> {
> /* Reject rename("","x"), rename("x",""). */
> @@ -2337,6 +2348,13 @@ rename (const char *oldpath, const char *newpath)
> __leave;
> }
>
> + /* Should we replace an existing file? */
> + if ((removepc || dstpc->exists ()) && (flags & RENAME_NOREPLACE))
> + {
> + set_errno (EEXIST);
> + __leave;
> + }
> +
Do we really need this test here? If you check at this point and then
go ahead preparing the actual rename operation, you have the atomicity
problem again which renameat2 is trying to solve.
But there's light. NtSetInformationFile(FileRenameInformation) already
supports RENAME_NOREPLACE :)
Have a look at line 2494 (prior to your patch):
pfri->ReplaceIfExists = TRUE;
if you replace this with something like
pfri->ReplaceIfExists = !(flags & RENAME_NOREPLACE);
it should give us the atomic behaviour of renameat2 on Linux.
Another upside is, the status code returned is STATUS_OBJECT_NAME_COLLISION,
which translates to Win32 error ERROR_ALREADY_EXISTS, which in turn is
already converted to EEXIST by Cygwin, so there's nothing more to do :)
What do you think?
Thanks,
Corinna
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat
signature.asc
Description: PGP signature
Re: renameat2
Hi Corinna,
On 8/18/2017 11:15 AM, Corinna Vinschen wrote:
Hi Ken,
On Aug 18 09:21, Ken Brown wrote:
Linux has a system call 'renameat2' which is like renameat but has an
extra 'flags' argument. In particular, one can pass the
RENAME_NOREPLACE flag to cause the rename to fail with EEXIST if the
target of the rename exists. See
http://man7.org/linux/man-pages/man2/rename.2.html
macOS has a similar functionality, provided by the function
'renameatx_np' with the flag RENAME_EXCL.
There's also a recently introduced Gnulib module 'renameat2', but it
requires two system calls on Cygwin (one to test existence and the
second to do the rename), so that there is a race condition. On Linux
and macOS it uses renameat2 and renameatx_np to avoid the race.
The attached patch implements renameat2 on Cygwin (but only supporting
the RENAME_NOREPLACE flag). I've written it so that a rename that
just changes case on a case-insensitive file system succeeds.
If the patch is accepted, I'll submit a second patch that documents
the new function.
Neat stuff, but there are a few points for discussion, see below.
--- a/winsup/cygwin/include/cygwin/fs.h
+++ b/winsup/cygwin/include/cygwin/fs.h
@@ -19,4 +19,9 @@ details. */
#define BLKPBSZGET 0x127b
#define BLKGETSIZE64 0x00041268
+/* Flags for renameat2, from /usr/include/linux/fs.h. */
+#define RENAME_NOREPLACE (1 << 0)
+#define RENAME_EXCHANGE (1 << 1)
+#define RENAME_WHITEOUT (1 << 2)
Given that there's no standard for this call (yet), do we really want to
expose flag values we don't support? I would opt for only RENAME_NOREPLACE
for now and skip the others.
+
#endif
diff --git a/winsup/cygwin/include/cygwin/version.h
b/winsup/cygwin/include/cygwin/version.h
index efd4ac017..7640abfad 100644
--- a/winsup/cygwin/include/cygwin/version.h
+++ b/winsup/cygwin/include/cygwin/version.h
@@ -481,12 +481,14 @@ details. */
314: Export explicit_bzero.
315: Export pthread_mutex_timedlock.
316: Export pthread_rwlock_timedrdlock, pthread_rwlock_timedwrlock.
+ 317: Export renameat2. Add RENAME_NOREPLACE, RENAME_EXCHANGE,
+ RENAME_WHITEOUT.
You can drop the flag values here. renameat2 is sufficient.
+rename2 (const char *oldpath, const char *newpath, unsigned int flags)
{
tmp_pathbuf tp;
int res = -1;
@@ -2068,6 +2073,12 @@ rename (const char *oldpath, const char *newpath)
__try
{
+ if (flags & ~RENAME_NOREPLACE)
+ /* RENAME_NOREPLACE is the only flag currently supported. */
+ {
+ set_errno (ENOTSUP);
That should ideally be EINVAL. Unsupported bit values in a flag argument?
EINVAL, please.
+ __leave;
+ }
if (!*oldpath || !*newpath)
{
/* Reject rename("","x"), rename("x",""). */
@@ -2337,6 +2348,13 @@ rename (const char *oldpath, const char *newpath)
__leave;
}
+ /* Should we replace an existing file? */
+ if ((removepc || dstpc->exists ()) && (flags & RENAME_NOREPLACE))
+ {
+ set_errno (EEXIST);
+ __leave;
+ }
+
Do we really need this test here? If you check at this point and then
go ahead preparing the actual rename operation, you have the atomicity
problem again which renameat2 is trying to solve.
But there's light. NtSetInformationFile(FileRenameInformation) already
supports RENAME_NOREPLACE :)
Have a look at line 2494 (prior to your patch):
pfri->ReplaceIfExists = TRUE;
if you replace this with something like
pfri->ReplaceIfExists = !(flags & RENAME_NOREPLACE);
it should give us the atomic behaviour of renameat2 on Linux.
Another upside is, the status code returned is STATUS_OBJECT_NAME_COLLISION,
which translates to Win32 error ERROR_ALREADY_EXISTS, which in turn is
already converted to EEXIST by Cygwin, so there's nothing more to do :)
What do you think?
Thanks for the improvements! A revised patch is attached. As you'll
see, I still found a few places where I thought I needed to explicitly
set the errno to EEXIST. Let me know if any of these could be avoided.
Thanks.
Ken
From d5798b371ceabfe6a7912472edd32da1ebd7dcb7 Mon Sep 17 00:00:00 2001
From: Ken Brown
Date: Thu, 17 Aug 2017 09:12:15 -0400
Subject: [PATCH] cygwin: Implement renameat2
Define the RENAME_NOREPLACE flag in as defined on Linux
in . The other RENAME_* flags defined on Linux are not
supported.
---
newlib/libc/include/stdio.h| 3 ++
winsup/cygwin/common.din | 1 +
winsup/cygwin/include/cygwin/fs.h | 6 +++
winsup/cygwin/include/cygwin/version.h | 3 +-
winsup/cygwin/syscalls.cc | 67 +++---
5 files changed, 73 insertions(+), 7 deletions(-)
diff --git a/newlib/libc/include/stdio.h b/newlib/libc/include/stdio.h
index 5d8cb1092..331a1cf07 100644
--- a/newlib/libc/include/stdio.h
+++ b/newlib/libc/include/stdio.h
@@ -384,6 +384,9 @@ int _EXFUN(vdprintf, (int, const char *__restrict, __VALIST)
#endif
