Issue #15154: Add "dir_fd" parameter to os.rmdir, remove "rmdir"
parameter from os.remove / os.unlink.
Patch written by Georg Brandl. (I'm really looking forward to George
getting commit privileges so I don't have to keep doing checkins on his
behalf.)
diff --git a/Doc/library/os.rst b/Doc/library/os.rst
index 3b3bd3d..65a0ac1 100644
--- a/Doc/library/os.rst
+++ b/Doc/library/os.rst
@@ -1750,14 +1750,10 @@
The *dir_fd* argument.
-.. function:: remove(path, *, dir_fd=None, rmdir=False)
+.. function:: remove(path, *, dir_fd=None)
- Remove (delete) the file *path*. This function is identical to
- :func:`os.unlink`.
-
- Specify ``rmdir=True`` if *path* is a directory. Failing to do so
- will raise an exception; likewise, specifying ``rmdir=True`` when
- *path* is not a directory will also raise an exception.
+ Remove (delete) the file *path*. If *path* is a directory, :exc:`OSError`
+ is raised. Use :func:`rmdir` to remove directories.
If *dir_fd* is not ``None``, it should be a file descriptor referring to a
directory, and *path* should be relative; path will then be relative to
@@ -1771,10 +1767,12 @@
be raised; on Unix, the directory entry is removed but the storage allocated
to the file is not made available until the original file is no longer in use.
+ This function is identical to :func:`unlink`.
+
Availability: Unix, Windows.
.. versionadded:: 3.3
- The *dir_fd* and *rmdir* arguments.
+ The *dir_fd* argument.
.. function:: removedirs(path)
@@ -1872,14 +1870,25 @@
.. versionadded:: 3.3
-.. function:: rmdir(path)
+.. function:: rmdir(path, *, dir_fd=None)
Remove (delete) the directory *path*. Only works when the directory is
empty, otherwise, :exc:`OSError` is raised. In order to remove whole
directory trees, :func:`shutil.rmtree` can be used.
+ If *dir_fd* is not ``None``, it should be a file descriptor referring to a
+ directory, and *path* should be relative; path will then be relative to
+ that directory. (If *path* is absolute, *dir_fd* is ignored.)
+ *dir_fd* may not be supported on your platform;
+ you can check whether or not it is available using
+ :data:`os.supports_dir_fd`. If it is unavailable, using it will raise
+ a :exc:`NotImplementedError`.
+
Availability: Unix, Windows.
+ .. versionadded:: 3.3
+ The *dir_fd* parameter.
+
.. data:: XATTR_SIZE_MAX
@@ -2235,9 +2244,9 @@
.. versionadded:: 3.3
-.. function:: unlink(path, *, dir_fd=None, rmdir=False)
+.. function:: unlink(path, *, dir_fd=None)
- Remove (delete) the file *path*. This is the same function as
+ Remove (delete) the file *path*. This function is identical to
:func:`remove`; the :func:`unlink` name is its traditional Unix
name. Please see the documentation for :func:`remove` for
further information.
@@ -2245,7 +2254,7 @@
Availability: Unix, Windows.
.. versionadded:: 3.3
- The *dir_fd* and *rmdir* parameters.
+ The *dir_fd* parameter.
.. function:: utime(path, times=None, *, ns=None, dir_fd=None, follow_symlinks=True)
diff --git a/Lib/os.py b/Lib/os.py
index b5ad1b5..4a40cfe 100644
--- a/Lib/os.py
+++ b/Lib/os.py
@@ -157,6 +157,7 @@
_add("HAVE_RENAMEAT", "rename")
_add("HAVE_SYMLINKAT", "symlink")
_add("HAVE_UNLINKAT", "unlink")
+ _add("HAVE_UNLINKAT", "rmdir")
_add("HAVE_UTIMENSAT", "utime")
supports_dir_fd = _set
@@ -214,10 +215,6 @@
_add("MS_WINDOWS", "stat")
supports_follow_symlinks = _set
- _set = set()
- _add("HAVE_UNLINKAT", "unlink")
- supports_remove_directory = _set
-
del _set
del _have_functions
del _globals
diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
index cc1120f..654dd23 100644
--- a/Lib/test/test_os.py
+++ b/Lib/test/test_os.py
@@ -785,7 +785,10 @@
os.unlink(name, dir_fd=rootfd)
for name in dirs:
st = os.stat(name, dir_fd=rootfd, follow_symlinks=False)
- os.unlink(name, dir_fd=rootfd, rmdir=stat.S_ISDIR(st.st_mode))
+ if stat.S_ISDIR(st.st_mode):
+ os.rmdir(name, dir_fd=rootfd)
+ else:
+ os.unlink(name, dir_fd=rootfd)
os.rmdir(support.TESTFN)
diff --git a/Misc/NEWS b/Misc/NEWS
index 031550e..fadf214 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -47,6 +47,9 @@
Library
-------
+- Issue #15154: Add "dir_fd" parameter to os.rmdir, remove "rmdir"
+ parameter from os.remove / os.unlink.
+
- Issue #4489: Add a shutil.rmtree that isn't susceptible to symlink attacks.
It is used automatically on platforms supporting the necessary os.openat()
and os.unlinkat() functions. Main code by Martin von Löwis.
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index 30b797c..bfe32b8 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -4084,17 +4084,62 @@
}
PyDoc_STRVAR(posix_rmdir__doc__,
-"rmdir(path)\n\n\
-Remove a directory.");
+"rmdir(path, *, dir_fd=None)\n\n\
+Remove a directory.\n\
+\n\
+If dir_fd is not None, it should be a file descriptor open to a directory,\n\
+ and path should be relative; path will then be relative to that directory.\n\
+dir_fd may not be implemented on your platform.\n\
+ If it is unavailable, using it will raise a NotImplementedError.");
static PyObject *
-posix_rmdir(PyObject *self, PyObject *args)
+posix_rmdir(PyObject *self, PyObject *args, PyObject *kwargs)
{
-#ifdef MS_WINDOWS
- return win32_1str(args, "rmdir", "y:rmdir", RemoveDirectoryA, "U:rmdir", RemoveDirectoryW);
+ path_t path;
+ int dir_fd = DEFAULT_DIR_FD;
+ static char *keywords[] = {"path", "dir_fd", NULL};
+ int result;
+ PyObject *return_value = NULL;
+
+ memset(&path, 0, sizeof(path));
+ if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O&|$O&:rmdir", keywords,
+ path_converter, &path,
+#ifdef HAVE_UNLINKAT
+ dir_fd_converter, &dir_fd
#else
- return posix_1str(args, "O&:rmdir", rmdir);
+ dir_fd_unavailable, &dir_fd
#endif
+ ))
+ return NULL;
+
+ Py_BEGIN_ALLOW_THREADS
+#ifdef MS_WINDOWS
+ if (path.wide)
+ result = RemoveDirectoryW(path.wide);
+ else
+ result = RemoveDirectoryA(path.narrow);
+ result = !result; /* Windows, success=1, UNIX, success=0 */
+#else
+#ifdef HAVE_UNLINKAT
+ if (dir_fd != DEFAULT_DIR_FD)
+ result = unlinkat(dir_fd, path.narrow, AT_REMOVEDIR);
+ else
+#endif
+ result = rmdir(path.narrow);
+#endif
+ Py_END_ALLOW_THREADS
+
+ if (result) {
+ return_value = path_error("rmdir", &path);
+ goto exit;
+ }
+
+ return_value = Py_None;
+ Py_INCREF(Py_None);
+
+exit:
+ path_cleanup(&path);
+ return return_value;
}
@@ -4186,68 +4231,54 @@
#endif /* MS_WINDOWS */
PyDoc_STRVAR(posix_unlink__doc__,
-"unlink(path, *, dir_fd=None, rmdir=False)\n\n\
+"unlink(path, *, dir_fd=None)\n\n\
Remove a file (same as remove()).\n\
\n\
If dir_fd is not None, it should be a file descriptor open to a directory,\n\
and path should be relative; path will then be relative to that directory.\n\
dir_fd may not be implemented on your platform.\n\
- If it is unavailable, using it will raise a NotImplementedError.\n\
-If rmdir is True, unlink will behave like os.rmdir().");
+ If it is unavailable, using it will raise a NotImplementedError.");
PyDoc_STRVAR(posix_remove__doc__,
-"remove(path, *, dir_fd=None, rmdir=False)\n\n\
+"remove(path, *, dir_fd=None)\n\n\
Remove a file (same as unlink()).\n\
\n\
If dir_fd is not None, it should be a file descriptor open to a directory,\n\
and path should be relative; path will then be relative to that directory.\n\
dir_fd may not be implemented on your platform.\n\
- If it is unavailable, using it will raise a NotImplementedError.\n\
-If rmdir is True, remove will behave like os.rmdir().");
+ If it is unavailable, using it will raise a NotImplementedError.");
static PyObject *
posix_unlink(PyObject *self, PyObject *args, PyObject *kwargs)
{
path_t path;
int dir_fd = DEFAULT_DIR_FD;
- int remove_dir = 0;
- static char *keywords[] = {"path", "dir_fd", "rmdir", NULL};
+ static char *keywords[] = {"path", "dir_fd", NULL};
int result;
PyObject *return_value = NULL;
memset(&path, 0, sizeof(path));
- if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O&|$O&p:unlink", keywords,
+ if (!PyArg_ParseTupleAndKeywords(args, kwargs, "O&|$O&:unlink", keywords,
path_converter, &path,
#ifdef HAVE_UNLINKAT
- dir_fd_converter, &dir_fd,
+ dir_fd_converter, &dir_fd
#else
- dir_fd_unavailable, &dir_fd,
+ dir_fd_unavailable, &dir_fd
#endif
- &remove_dir))
+ ))
return NULL;
Py_BEGIN_ALLOW_THREADS
#ifdef MS_WINDOWS
- if (remove_dir) {
- if (path.wide)
- result = RemoveDirectoryW(path.wide);
- else
- result = RemoveDirectoryA(path.narrow);
- }
- else {
- if (path.wide)
- result = Py_DeleteFileW(path.wide);
- else
- result = DeleteFileA(path.narrow);
- }
+ if (path.wide)
+ result = Py_DeleteFileW(path.wide);
+ else
+ result = DeleteFileA(path.narrow);
result = !result; /* Windows, success=1, UNIX, success=0 */
#else
- if (remove_dir && (dir_fd == DEFAULT_DIR_FD))
- result = rmdir(path.narrow);
- else
#ifdef HAVE_UNLINKAT
if (dir_fd != DEFAULT_DIR_FD)
- result = unlinkat(dir_fd, path.narrow, remove_dir ? AT_REMOVEDIR : 0);
+ result = unlinkat(dir_fd, path.narrow, 0);
else
#endif /* HAVE_UNLINKAT */
result = unlink(path.narrow);
@@ -10806,7 +10837,9 @@
{"replace", (PyCFunction)posix_replace,
METH_VARARGS | METH_KEYWORDS,
posix_replace__doc__},
- {"rmdir", posix_rmdir, METH_VARARGS, posix_rmdir__doc__},
+ {"rmdir", (PyCFunction)posix_rmdir,
+ METH_VARARGS | METH_KEYWORDS,
+ posix_rmdir__doc__},
{"stat", (PyCFunction)posix_stat,
METH_VARARGS | METH_KEYWORDS,
posix_stat__doc__},