From 123b2033dee09ad5caf5b05d4a02be5a6cd825bb Mon Sep 17 00:00:00 2001 From: William Orr Date: Sat, 20 May 2017 20:44:36 -0700 Subject: [PATCH 01/10] Add support for extended attributes on FreeBSD Extended attributes are supported on FreeBSD, but have a different API than on Linux. This implements a compatibility layer so that the same set of python functions map to the equivalent FreeBSD functions. --- Lib/test/test_os.py | 22 +- .../2018-03-02-23-38-36.bpo-12978.GmUrw4.rst | 1 + Modules/posixmodule.c | 523 +++++++++++++++--- configure | 2 +- configure.ac | 2 +- pyconfig.h.in | 3 + 6 files changed, 482 insertions(+), 71 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2018-03-02-23-38-36.bpo-12978.GmUrw4.rst diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 4f8a2a7e19d5d2..6c4d0967130aab 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -2781,18 +2781,21 @@ def supports_extended_attributes(): @unittest.skipUnless(supports_extended_attributes(), "no non-broken extended attribute support") -# Kernels < 2.6.39 don't respect setxattr flags. -@support.requires_linux_version(2, 6, 39) class ExtendedAttributeTests(unittest.TestCase): def _check_xattrs_str(self, s, getxattr, setxattr, removexattr, listxattr, **kwargs): fn = support.TESTFN + if sys.platform.startswith("freebsd"): + xattr_errno = errno.ENOATTR + else: + xattr_errno = errno.ENODATA + self.addCleanup(support.unlink, fn) create_file(fn) with self.assertRaises(OSError) as cm: getxattr(fn, s("user.test"), **kwargs) - self.assertEqual(cm.exception.errno, errno.ENODATA) + self.assertEqual(cm.exception.errno, xattr_errno) init_xattr = listxattr(fn) self.assertIsInstance(init_xattr, list) @@ -2811,7 +2814,7 @@ def _check_xattrs_str(self, s, getxattr, setxattr, removexattr, listxattr, **kwa with self.assertRaises(OSError) as cm: setxattr(fn, s("user.test2"), b"bye", os.XATTR_REPLACE, **kwargs) - self.assertEqual(cm.exception.errno, errno.ENODATA) + self.assertEqual(cm.exception.errno, xattr_errno) setxattr(fn, s("user.test2"), b"foo", os.XATTR_CREATE, **kwargs) xattr.add("user.test2") @@ -2820,7 +2823,7 @@ def _check_xattrs_str(self, s, getxattr, setxattr, removexattr, listxattr, **kwa with self.assertRaises(OSError) as cm: getxattr(fn, s("user.test"), **kwargs) - self.assertEqual(cm.exception.errno, errno.ENODATA) + self.assertEqual(cm.exception.errno, xattr_errno) xattr.remove("user.test") self.assertEqual(set(listxattr(fn)), xattr) @@ -2840,14 +2843,23 @@ def _check_xattrs(self, *args, **kwargs): self._check_xattrs_str(os.fsencode, *args, **kwargs) support.unlink(support.TESTFN) + # Kernels < 2.6.39 don't respect setxattr flags. + @support.requires_linux_version(2, 6, 39) + @support.requires_freebsd_version(5) def test_simple(self): self._check_xattrs(os.getxattr, os.setxattr, os.removexattr, os.listxattr) + # Kernels < 2.6.39 don't respect setxattr flags. + @support.requires_linux_version(2, 6, 39) + @support.requires_freebsd_version(5) def test_lpath(self): self._check_xattrs(os.getxattr, os.setxattr, os.removexattr, os.listxattr, follow_symlinks=False) + # Kernels < 2.6.39 don't respect setxattr flags. + @support.requires_linux_version(2, 6, 39) + @support.requires_freebsd_version(5) def test_fds(self): def getxattr(path, *args): with open(path, "rb") as fp: diff --git a/Misc/NEWS.d/next/Library/2018-03-02-23-38-36.bpo-12978.GmUrw4.rst b/Misc/NEWS.d/next/Library/2018-03-02-23-38-36.bpo-12978.GmUrw4.rst new file mode 100644 index 00000000000000..eb84ae0c46a2dd --- /dev/null +++ b/Misc/NEWS.d/next/Library/2018-03-02-23-38-36.bpo-12978.GmUrw4.rst @@ -0,0 +1 @@ +Add support for extended attributes on FreeBSD diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 6bba8ee26e1670..5e4e988bd64bcb 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -105,12 +105,422 @@ corresponding Unix manual entries for more information on calls."); #undef HAVE_SCHED_SETAFFINITY #endif -#if defined(HAVE_SYS_XATTR_H) && defined(__GLIBC__) && !defined(__FreeBSD_kernel__) && !defined(__GNU__) +#if defined(HAVE_SYS_XATTR_H) && defined(__GLIBC__) && !defined(__GNU__) +#define USE_XATTRS +#elif defined(HAVE_SYS_EXTATTR_H) && defined(__FreeBSD__) #define USE_XATTRS #endif -#ifdef USE_XATTRS +#if defined(USE_XATTRS) && defined(HAVE_SYS_XATTR_H) #include +#elif defined(USE_XATTRS) && defined(HAVE_SYS_EXTATTR_H) +#include +#define XATTR_SIZE_MAX 4096 +#define XATTR_LIST_MAX 4096 +#define XATTR_CREATE 1 +#define XATTR_REPLACE 2 +#endif + +#if defined(USE_XATTRS) && defined(__FreeBSD__) + +/* Turns a qualified string into a namespace id on FreeBSD. + * This ensures that that the namespace is qualified + * ((user|system).) and that is non-null. + */ +static int +xattrnamespace(int *namespace, const char *fullattr) +{ + char *sep = strchr(fullattr, '.'); + size_t nslen = 0; + int ret = 0; + + *namespace = -1; + // return ENOATTR on unqualified names + if (sep == NULL || *(sep + 1) == '\0') + return ENOATTR; + + if (sep < fullattr) + return ENOATTR; + + nslen = sep - fullattr + 1; + + if (strncmp(fullattr, "user", (4 < nslen ? 4 : nslen)) == 0) { + *namespace = EXTATTR_NAMESPACE_USER; + } else if (strncmp(fullattr, "system", (6 < nslen ? 6 : nslen)) == 0) { + *namespace = EXTATTR_NAMESPACE_SYSTEM; + } + + if (*namespace == -1) + ret = ENOATTR; + + return ret; +} + +/* xattr lists come back from FreeBSD without the leading namespace. + * We need to translate this into a linux-like format + */ +static ssize_t +normalize_xattr_list(int namespace, char *attrs, size_t size, ssize_t extattr_ret) +{ + char *scratch = NULL, *scratch_ptr; + ssize_t ret = 0; + ssize_t i = 0, left = size, attr_size; + + /* FreeBSD only returns the size of the attribute names, not including the + * namespaces. In order to guess a buffer size, we multiply the size + * returned by the longest possible prefix - "system.". This accounts for + * the case where each attribute is 1 character in length, and is a system + * attribute. + * + * This does unfortunately give us longer buffers than we need, but since + * the extattr(2) interface gives us no way to detect overflow, this is + * the best we can do to guess the size. + */ + if (! attrs && extattr_ret) { + if (SSIZE_MAX / extattr_ret <= strlen("system.")) { + return -ERANGE; + } + return strlen("system.") * extattr_ret; + } + + /* overflow */ + if (size - SSIZE_MAX < SSIZE_MAX) + return -ERANGE; + + if (! (scratch = calloc(1, size))) + return -ENOMEM; + + scratch_ptr = scratch; + + for (i = 0; i < size && attrs[i] != 0;) { + attr_size = attrs[i]; + if (namespace == EXTATTR_NAMESPACE_USER) { + strncat(scratch_ptr, "user.", left); + scratch_ptr += strlen("user."); + left -= strlen("user."); + } else if (namespace == EXTATTR_NAMESPACE_SYSTEM) { + strncat(scratch_ptr, "system.", left); + scratch_ptr += strlen("system."); + left -= strlen("system."); + } + + if (attr_size > left) { + ret = -ENOMEM; + goto bad; + } + + memcpy(scratch_ptr, attrs + i + 1, attr_size); + scratch_ptr += attr_size + 1; + i += attr_size + 1; + left -= attr_size + 1; + } + + memcpy(attrs, scratch, size); + ret = size - left; + +bad: + free(scratch); + + return ret; +} + +static ssize_t +getxattr(const char *path, const char *name, void *value, size_t size) +{ + int nsid = 0; + int err = 0; + char *attr_name = NULL; + + if ((err = xattrnamespace(&nsid, name))) { + errno = err; + return -1; + } + + attr_name = strchr(name, '.') + 1; + + return extattr_get_file(path, nsid, attr_name, value, size); +} + +static ssize_t +lgetxattr(const char *path, const char *name, void *value, size_t size) +{ + int nsid = 0; + int err = 0; + char *attr_name = NULL; + + if ((err = xattrnamespace(&nsid, name))) { + errno = err; + return -1; + } + + attr_name = strchr(name, '.') + 1; + + return extattr_get_link(path, nsid, attr_name, value, size); +} + +static ssize_t +fgetxattr(int fd, const char *name, void *value, size_t size) +{ + int nsid = 0; + int err = 0; + char *attr_name = NULL; + + if ((err = xattrnamespace(&nsid, name))) { + errno = err; + return -1; + } + + attr_name = strchr(name, '.') + 1; + + return extattr_get_fd(fd, nsid, attr_name, value, size); +} + +static int +setxattr(const char *path, const char *name, const void *value, size_t size, int flags) +{ + int nsid = 0; + int err = 0; + char *attr_name = NULL; + ssize_t get_ret = 0; + + if ((err = xattrnamespace(&nsid, name))) { + errno = err; + return -1; + } + + attr_name = strchr(name, '.') + 1; + + if (flags) { + get_ret = extattr_get_file(path, nsid, attr_name, NULL, 0); + if (flags & XATTR_REPLACE && get_ret == -1) { + errno = ENOATTR; + return -1; + } else if (flags & XATTR_CREATE && get_ret >= 0) { + errno = EEXIST; + return -1; + } + } + + if (extattr_set_file(path, nsid, attr_name, value, size) != -1) + return 0; + return -1; +} + +static int +lsetxattr(const char *path, const char *name, const void *value, size_t size, int flags) +{ + int nsid = 0; + int err = 0; + ssize_t get_ret = 0; + char *attr_name = NULL; + + if ((err = xattrnamespace(&nsid, name))) { + errno = err; + return -1; + } + + attr_name = strchr(name, '.') + 1; + + if (flags) { + get_ret = extattr_get_link(path, nsid, attr_name, NULL, 0); + if (flags & XATTR_REPLACE && get_ret == -1) { + errno = ENOATTR; + return -1; + } else if (flags & XATTR_CREATE && get_ret >= 0) { + errno = EEXIST; + return -1; + } + } + + if (extattr_set_link(path, nsid, attr_name, value, size) != -1) + return 0; + return -1; +} + +static int +fsetxattr(int fd, const char *name, const void *value, size_t size, int flags) +{ + int nsid = 0; + int err = 0; + ssize_t get_ret = 0; + char *attr_name = NULL; + + if ((err = xattrnamespace(&nsid, name))) { + errno = err; + return -1; + } + + attr_name = strchr(name, '.') + 1; + + if (flags) { + get_ret = extattr_get_fd(fd, nsid, attr_name, NULL, 0); + if (flags & XATTR_REPLACE && get_ret == -1) { + errno = ENOATTR; + return -1; + } else if (flags & XATTR_CREATE && get_ret >= 0) { + errno = EEXIST; + return -1; + } + } + + if (extattr_set_fd(fd, nsid, attr_name, value, size) != -1) + return 0; + return -1; +} + +static int +removexattr(const char *path, const char *name) +{ + int nsid = 0; + int err = 0; + char *attr_name = NULL; + + if ((err = xattrnamespace(&nsid, name))) { + errno = err; + return -1; + } + + attr_name = strchr(name, '.') + 1; + + return extattr_delete_file(path, nsid, attr_name); +} + +static int +lremovexattr(const char *path, const char *name) +{ + int nsid = 0; + int err = 0; + char *attr_name = NULL; + + if ((err = xattrnamespace(&nsid, name))) { + errno = err; + return -1; + } + + attr_name = strchr(name, '.') + 1; + + return extattr_delete_link(path, nsid, attr_name); +} + +static int +fremovexattr(int fd, const char *name) +{ + int nsid = 0; + int err = 0; + char *attr_name = NULL; + + if ((err = xattrnamespace(&nsid, name))) { + errno = err; + return -1; + } + + attr_name = strchr(name, '.') + 1; + + return extattr_delete_fd(fd, nsid, attr_name); +} + +static ssize_t +listxattr(const char *path, char *list, size_t size) +{ + ssize_t ret = 0, retadder = 0; + + memset(list, 0, size); + retadder = extattr_list_file(path, EXTATTR_NAMESPACE_SYSTEM, list, size); + if (retadder == -1 && errno != EPERM) { + return retadder; + } else if (retadder > 0) { + retadder = normalize_xattr_list(EXTATTR_NAMESPACE_SYSTEM, list, size, retadder); + if (retadder < 0) + return retadder; + ret += retadder; + } + + retadder = extattr_list_file(path, EXTATTR_NAMESPACE_USER, list + ret, size - ret); + if (retadder == -1) { + return retadder; + } else { + retadder = normalize_xattr_list(EXTATTR_NAMESPACE_USER, list + ret, size - ret, retadder); + if (retadder < 0) + return retadder; + ret += retadder; + } + + // empty + if (ret < 2) { + return 0; + } + + return ret; +} + +static ssize_t +llistxattr(const char *path, char *list, size_t size) +{ + ssize_t ret = 0, retadder = 0; + + memset(list, 0, size); + retadder = extattr_list_link(path, EXTATTR_NAMESPACE_SYSTEM, list, size); + if (retadder == -1 && errno != EPERM) { + return retadder; + } else if (retadder > 0) { + retadder = normalize_xattr_list(EXTATTR_NAMESPACE_SYSTEM, list, size, retadder); + if (retadder < 0) + return retadder; + ret += retadder; + } + + retadder = extattr_list_link(path, EXTATTR_NAMESPACE_USER, list + ret, size - retadder); + if (retadder == -1) { + return retadder; + } else { + retadder = normalize_xattr_list(EXTATTR_NAMESPACE_USER, list + ret, size - ret, retadder); + if (retadder < 0) + return retadder; + ret += retadder; + } + + // empty + if (ret < 2) { + return 0; + } + + return ret; +} + +static ssize_t +flistxattr(int fd, char *list, size_t size) +{ + ssize_t ret = 0, retadder = 0; + + memset(list, 0, size); + retadder = extattr_list_fd(fd, EXTATTR_NAMESPACE_SYSTEM, list, size); + if (retadder == -1 && errno != EPERM) { + return retadder; + } else if (retadder > 0) { + retadder = normalize_xattr_list(EXTATTR_NAMESPACE_SYSTEM, list, size, retadder); + if (retadder < 0) + return retadder; + ret += retadder; + } + + retadder = extattr_list_fd(fd, EXTATTR_NAMESPACE_USER, list + ret, size - retadder); + if (retadder == -1) { + return retadder; + } else { + retadder = normalize_xattr_list(EXTATTR_NAMESPACE_USER, list + ret, size - ret, retadder); + if (retadder < 0) + return retadder; + ret += retadder; + } + + // empty + if (ret < 2) { + return 0; + } + + return ret; +} + #endif #if defined(__FreeBSD__) || defined(__DragonFly__) || defined(__APPLE__) @@ -11181,23 +11591,20 @@ os_getxattr_impl(PyObject *module, path_t *path, path_t *attribute, { Py_ssize_t i; PyObject *buffer = NULL; + Py_ssize_t buffer_size = 0; + void *ptr = NULL; if (fd_and_follow_symlinks_invalid("getxattr", path->fd, follow_symlinks)) return NULL; - for (i = 0; ; i++) { - void *ptr; + for (i = 0; i < 2; i++) { ssize_t result; - static const Py_ssize_t buffer_sizes[] = {128, XATTR_SIZE_MAX, 0}; - Py_ssize_t buffer_size = buffer_sizes[i]; - if (!buffer_size) { - path_error(path); - return NULL; + if (i) { + buffer = PyBytes_FromStringAndSize(NULL, buffer_size); + if (!buffer) + return NULL; + ptr = PyBytes_AS_STRING(buffer); } - buffer = PyBytes_FromStringAndSize(NULL, buffer_size); - if (!buffer) - return NULL; - ptr = PyBytes_AS_STRING(buffer); Py_BEGIN_ALLOW_THREADS; if (path->fd >= 0) @@ -11209,18 +11616,16 @@ os_getxattr_impl(PyObject *module, path_t *path, path_t *attribute, Py_END_ALLOW_THREADS; if (result < 0) { - Py_DECREF(buffer); + if (buffer) + Py_DECREF(buffer); if (errno == ERANGE) continue; path_error(path); return NULL; } - if (result != buffer_size) { - /* Can only shrink. */ - _PyBytes_Resize(&buffer, result); - } - break; + if (! buffer_size) + buffer_size = result; } return buffer; @@ -11344,27 +11749,20 @@ os_listxattr_impl(PyObject *module, path_t *path, int follow_symlinks) Py_ssize_t i; PyObject *result = NULL; const char *name; - char *buffer = NULL; + char *buffer = NULL, *start, *end, *trace; + size_t buffer_size = 0; + ssize_t length = 0; if (fd_and_follow_symlinks_invalid("listxattr", path->fd, follow_symlinks)) goto exit; name = path->narrow ? path->narrow : "."; - for (i = 0; ; i++) { - const char *start, *trace, *end; - ssize_t length; - static const Py_ssize_t buffer_sizes[] = { 256, XATTR_LIST_MAX, 0 }; - Py_ssize_t buffer_size = buffer_sizes[i]; - if (!buffer_size) { - /* ERANGE */ - path_error(path); - break; - } - buffer = PyMem_MALLOC(buffer_size); - if (!buffer) { - PyErr_NoMemory(); - break; + for (i = 0; i < 2; i++) { + if (i) { + buffer = PyMem_Malloc(buffer_size); + if (!buffer) + return NULL; } Py_BEGIN_ALLOW_THREADS; @@ -11377,42 +11775,39 @@ os_listxattr_impl(PyObject *module, path_t *path, int follow_symlinks) Py_END_ALLOW_THREADS; if (length < 0) { - if (errno == ERANGE) { - PyMem_FREE(buffer); - buffer = NULL; - continue; - } path_error(path); - break; - } - - result = PyList_New(0); - if (!result) { goto exit; } - end = buffer + length; - for (trace = start = buffer; trace != end; trace++) { - if (!*trace) { - int error; - PyObject *attribute = PyUnicode_DecodeFSDefaultAndSize(start, - trace - start); - if (!attribute) { - Py_DECREF(result); - result = NULL; - goto exit; - } - error = PyList_Append(result, attribute); - Py_DECREF(attribute); - if (error) { - Py_DECREF(result); - result = NULL; - goto exit; - } - start = trace + 1; + if (! i) + buffer_size = length; + } + + result = PyList_New(0); + if (!result) { + goto exit; + } + + end = buffer + length; + for (trace = start = buffer; trace != end; trace++) { + if (!*trace) { + int error; + PyObject *attribute = PyUnicode_DecodeFSDefaultAndSize(start, + trace - start); + if (!attribute) { + Py_DECREF(result); + result = NULL; + goto exit; + } + error = PyList_Append(result, attribute); + Py_DECREF(attribute); + if (error) { + Py_DECREF(result); + result = NULL; + goto exit; } + start = trace + 1; } - break; } exit: if (buffer) diff --git a/configure b/configure index f9eee2c028660f..f56b1fb71fc804 100755 --- a/configure +++ b/configure @@ -7710,7 +7710,7 @@ sys/stat.h sys/syscall.h sys/sys_domain.h sys/termio.h sys/time.h \ sys/times.h sys/types.h sys/uio.h sys/un.h sys/utsname.h sys/wait.h pty.h \ libutil.h sys/resource.h netpacket/packet.h sysexits.h bluetooth.h \ linux/tipc.h linux/random.h spawn.h util.h alloca.h endian.h \ -sys/endian.h sys/sysmacros.h +sys/endian.h sys/sysmacros.h sys/extattr.h do : as_ac_Header=`$as_echo "ac_cv_header_$ac_header" | $as_tr_sh` ac_fn_c_check_header_mongrel "$LINENO" "$ac_header" "$as_ac_Header" "$ac_includes_default" diff --git a/configure.ac b/configure.ac index b83abee18cb720..3955e594109a28 100644 --- a/configure.ac +++ b/configure.ac @@ -2062,7 +2062,7 @@ sys/stat.h sys/syscall.h sys/sys_domain.h sys/termio.h sys/time.h \ sys/times.h sys/types.h sys/uio.h sys/un.h sys/utsname.h sys/wait.h pty.h \ libutil.h sys/resource.h netpacket/packet.h sysexits.h bluetooth.h \ linux/tipc.h linux/random.h spawn.h util.h alloca.h endian.h \ -sys/endian.h sys/sysmacros.h) +sys/endian.h sys/sysmacros.h sys/extattr.h) AC_HEADER_DIRENT AC_HEADER_MAJOR diff --git a/pyconfig.h.in b/pyconfig.h.in index a0efff9777d27d..3c8be4ddde1dec 100644 --- a/pyconfig.h.in +++ b/pyconfig.h.in @@ -1045,6 +1045,9 @@ /* Define to 1 if you have the header file. */ #undef HAVE_SYS_EVENT_H +/* Define to 1 if you have the header file. */ +#undef HAVE_SYS_EXTATTR_H + /* Define to 1 if you have the header file. */ #undef HAVE_SYS_FILE_H From 6011a5e54cd445ed603f896359499109e0db393c Mon Sep 17 00:00:00 2001 From: Davide Rizzo Date: Fri, 28 Apr 2023 21:54:16 +0200 Subject: [PATCH 02/10] os.*xattr on darwin --- Lib/test/test_os.py | 2 +- Lib/test/test_shutil.py | 2 +- Modules/posixmodule.c | 59 ++++++++++++++++++++++++++++++++++------- 3 files changed, 52 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py index 9cbf1540c5e7ee..bb19e939fb8a08 100644 --- a/Lib/test/test_os.py +++ b/Lib/test/test_os.py @@ -3675,7 +3675,7 @@ class ExtendedAttributeTests(unittest.TestCase): def _check_xattrs_str(self, s, getxattr, setxattr, removexattr, listxattr, **kwargs): fn = os_helper.TESTFN - if sys.platform.startswith("freebsd"): + if sys.platform.startswith("freebsd") or sys.platform == "darwin": xattr_errno = errno.ENOATTR else: xattr_errno = errno.ENODATA diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py index 36f0b8a31a3715..55fe3266fae02d 100644 --- a/Lib/test/test_shutil.py +++ b/Lib/test/test_shutil.py @@ -1092,7 +1092,7 @@ def test_copystat_symlinks(self): os.symlink(src, src_link) os.symlink(dst, dst_link) if hasattr(os, 'lchmod'): - os.lchmod(src_link, stat.S_IRWXO) + os.lchmod(src_link, stat.S_IRUSR) if hasattr(os, 'lchflags') and hasattr(stat, 'UF_NODUMP'): os.lchflags(src_link, stat.UF_NODUMP) src_link_stat = os.lstat(src_link) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index f9f2485208db0a..38f1314804bd51 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -278,17 +278,17 @@ corresponding Unix manual entries for more information on calls."); # undef HAVE_SCHED_SETAFFINITY #endif -#if defined(HAVE_SYS_XATTR_H) && defined(__linux__) && !defined(__FreeBSD_kernel__) && !defined(__GNU__) +#if defined(HAVE_SYS_XATTR_H) && defined(__linux__) +# include # define USE_XATTRS # include // Needed for XATTR_SIZE_MAX on musl libc. -#elif defined(HAVE_SYS_EXTATTR_H) && defined(__FreeBSD__) -# define USE_XATTRS -#endif - -#if defined(USE_XATTRS) && defined(HAVE_SYS_XATTR_H) +#elif defined(HAVE_SYS_XATTR_H) && defined(__APPLE__) # include -#elif defined(USE_XATTRS) && defined(HAVE_SYS_EXTATTR_H) +# define USE_XATTRS +# define XATTR_SIZE_MAX 4096 +#elif defined(HAVE_SYS_EXTATTR_H) && defined(__FreeBSD__) # include +# define USE_XATTRS # define XATTR_SIZE_MAX 4096 # define XATTR_LIST_MAX 4096 # define XATTR_CREATE 1 @@ -14069,7 +14069,6 @@ os_getxattr_impl(PyObject *module, path_t *path, path_t *attribute, Py_ssize_t i; PyObject *buffer = NULL; Py_ssize_t buffer_size = 0; - void *ptr = NULL; if (fd_and_follow_symlinks_invalid("getxattr", path->fd, follow_symlinks)) return NULL; @@ -14079,7 +14078,7 @@ os_getxattr_impl(PyObject *module, path_t *path, path_t *attribute, } for (i = 0; i < 2; i++) { - void *ptr; + void *ptr = NULL; ssize_t result; if (i) { buffer = PyBytes_FromStringAndSize(NULL, buffer_size); @@ -14089,12 +14088,24 @@ os_getxattr_impl(PyObject *module, path_t *path, path_t *attribute, } Py_BEGIN_ALLOW_THREADS; +#ifdef __APPLE__ + if (path->fd >= 0) + result = fgetxattr(path->fd, attribute->narrow, ptr, buffer_size, + 0, 0); + else if (follow_symlinks) + result = getxattr(path->narrow, attribute->narrow, ptr, + buffer_size, 0, 0); + else + result = getxattr(path->narrow, attribute->narrow, ptr, + buffer_size, 0, XATTR_NOFOLLOW); +#else if (path->fd >= 0) result = fgetxattr(path->fd, attribute->narrow, ptr, buffer_size); else if (follow_symlinks) result = getxattr(path->narrow, attribute->narrow, ptr, buffer_size); else result = lgetxattr(path->narrow, attribute->narrow, ptr, buffer_size); +#endif Py_END_ALLOW_THREADS; if (result < 0) { @@ -14149,6 +14160,17 @@ os_setxattr_impl(PyObject *module, path_t *path, path_t *attribute, } Py_BEGIN_ALLOW_THREADS; +#ifdef __APPLE__ + if (path->fd > -1) + result = fsetxattr(path->fd, attribute->narrow, + value->buf, value->len, 0, flags); + else if (follow_symlinks) + result = setxattr(path->narrow, attribute->narrow, + value->buf, value->len, 0, flags); + else + result = setxattr(path->narrow, attribute->narrow, + value->buf, value->len, 0, flags | XATTR_NOFOLLOW); +#else if (path->fd > -1) result = fsetxattr(path->fd, attribute->narrow, value->buf, value->len, flags); @@ -14158,6 +14180,7 @@ os_setxattr_impl(PyObject *module, path_t *path, path_t *attribute, else result = lsetxattr(path->narrow, attribute->narrow, value->buf, value->len, flags); +#endif Py_END_ALLOW_THREADS; if (result) { @@ -14201,12 +14224,21 @@ os_removexattr_impl(PyObject *module, path_t *path, path_t *attribute, } Py_BEGIN_ALLOW_THREADS; +#ifdef __APPLE__ + if (path->fd > -1) + result = fremovexattr(path->fd, attribute->narrow, 0); + else if (follow_symlinks) + result = removexattr(path->narrow, attribute->narrow, 0); + else + result = removexattr(path->narrow, attribute->narrow, XATTR_NOFOLLOW); +#else if (path->fd > -1) result = fremovexattr(path->fd, attribute->narrow); else if (follow_symlinks) result = removexattr(path->narrow, attribute->narrow); else result = lremovexattr(path->narrow, attribute->narrow); +#endif Py_END_ALLOW_THREADS; if (result) { @@ -14262,12 +14294,21 @@ os_listxattr_impl(PyObject *module, path_t *path, int follow_symlinks) } Py_BEGIN_ALLOW_THREADS; +#ifdef __APPLE__ + if (path->fd > -1) + length = flistxattr(path->fd, buffer, buffer_size, 0); + else if (follow_symlinks) + length = listxattr(name, buffer, buffer_size, 0); + else + length = listxattr(name, buffer, buffer_size, XATTR_NOFOLLOW); +#else if (path->fd > -1) length = flistxattr(path->fd, buffer, buffer_size); else if (follow_symlinks) length = listxattr(name, buffer, buffer_size); else length = llistxattr(name, buffer, buffer_size); +#endif Py_END_ALLOW_THREADS; if (length < 0) { From 4f4ef09d79778897e2ac0fba315ab24416fae30d Mon Sep 17 00:00:00 2001 From: Davide Rizzo Date: Sat, 29 Apr 2023 19:27:40 +0200 Subject: [PATCH 03/10] xattr-related constants --- Lib/shutil.py | 10 +++++++--- Modules/posixmodule.c | 11 +++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/Lib/shutil.py b/Lib/shutil.py index 7d1a3d00011f37..8db0b63e64bdc5 100644 --- a/Lib/shutil.py +++ b/Lib/shutil.py @@ -320,10 +320,14 @@ def _copyxattr(src, dst, *, follow_symlinks=True): """ + if sys.platform.startswith("freebsd") or sys.platform == "darwin": + missing_xattr_errno = errno.ENOATTR + else: + missing_xattr_errno = errno.ENODATA try: names = os.listxattr(src, follow_symlinks=follow_symlinks) except OSError as e: - if e.errno not in (errno.ENOTSUP, errno.ENODATA, errno.EINVAL): + if e.errno not in (errno.ENOTSUP, errno.EINVAL): raise return for name in names: @@ -331,8 +335,8 @@ def _copyxattr(src, dst, *, follow_symlinks=True): value = os.getxattr(src, name, follow_symlinks=follow_symlinks) os.setxattr(dst, name, value, follow_symlinks=follow_symlinks) except OSError as e: - if e.errno not in (errno.EPERM, errno.ENOTSUP, errno.ENODATA, - errno.EINVAL, errno.EACCES): + if e.errno not in (errno.EPERM, errno.ENOTSUP, errno.EINVAL, + errno.EACCES, missing_xattr_errno): raise else: def _copyxattr(*args, **kwargs): diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 38f1314804bd51..65872c9f8f7fff 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -16728,8 +16728,19 @@ all_ins(PyObject *m) #ifdef USE_XATTRS if (PyModule_AddIntMacro(m, XATTR_CREATE)) return -1; if (PyModule_AddIntMacro(m, XATTR_REPLACE)) return -1; +#endif +#ifdef XATTR_SIZE_MAX // Linux if (PyModule_AddIntMacro(m, XATTR_SIZE_MAX)) return -1; #endif +#ifdef XATTR_NAME_MAX // Linux + if (PyModule_AddIntMacro(m, XATTR_NAME_MAX)) return -1; +#endif +#ifdef XATTR_MAXNAMELEN // Darwin + if (PyModule_AddIntMacro(m, XATTR_MAXNAMELEN)) return -1; +#endif +#ifdef EXTATTR_MAXNAMELEN // FreeBSD + if (PyModule_AddIntMacro(m, EXTATTR_MAXNAMELEN)) return -1; +#endif #if HAVE_DECL_RTLD_LAZY if (PyModule_AddIntMacro(m, RTLD_LAZY)) return -1; From 985482bf374563ebfd13ae64d626c01cd92a7962 Mon Sep 17 00:00:00 2001 From: Davide Rizzo Date: Sat, 29 Apr 2023 19:28:03 +0200 Subject: [PATCH 04/10] xattrs doc update --- Doc/library/os.rst | 58 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/Doc/library/os.rst b/Doc/library/os.rst index 50e951c631fa88..dd1646aa249213 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -3727,12 +3727,13 @@ features: .. versionadded:: 3.10 -Linux extended attributes -~~~~~~~~~~~~~~~~~~~~~~~~~ +Extended attributes +~~~~~~~~~~~~~~~~~~~ .. versionadded:: 3.3 -These functions are all available on Linux only. +.. versionchanged:: 3.12 + Added support for macOS and FreeBSD. .. function:: getxattr(path, attribute, *, follow_symlinks=True) @@ -3746,9 +3747,13 @@ These functions are all available on Linux only. .. audit-event:: os.getxattr path,attribute os.getxattr + .. availability:: Linux, macOS, FreeBSD. + .. versionchanged:: 3.6 Accepts a :term:`path-like object` for *path* and *attribute*. + .. versionchanged:: 3.12 + Added support for macOS and FreeBSD. .. function:: listxattr(path=None, *, follow_symlinks=True) @@ -3762,9 +3767,15 @@ These functions are all available on Linux only. .. audit-event:: os.listxattr path os.listxattr + .. availability:: Linux, macOS, FreeBSD. + .. versionchanged:: 3.6 Accepts a :term:`path-like object`. + .. availability:: Linux, macOS, FreeBSD. + + .. versionchanged:: 3.12 + Added support for macOS and FreeBSD. .. function:: removexattr(path, attribute, *, follow_symlinks=True) @@ -3778,9 +3789,13 @@ These functions are all available on Linux only. .. audit-event:: os.removexattr path,attribute os.removexattr + .. availability:: Linux, macOS, FreeBSD. + .. versionchanged:: 3.6 Accepts a :term:`path-like object` for *path* and *attribute*. + .. versionchanged:: 3.12 + Added support for macOS and FreeBSD. .. function:: setxattr(path, attribute, value, flags=0, *, follow_symlinks=True) @@ -3803,21 +3818,54 @@ These functions are all available on Linux only. .. audit-event:: os.setxattr path,attribute,value,flags os.setxattr + .. availability:: Linux, macOS, FreeBSD. + .. versionchanged:: 3.6 Accepts a :term:`path-like object` for *path* and *attribute*. + .. versionchanged:: 3.12 + Added support for macOS and FreeBSD. .. data:: XATTR_SIZE_MAX The maximum size the value of an extended attribute can be. Currently, this - is 64 KiB on Linux. + is 64 KiB. + + .. availability:: Linux. +.. data:: XATTR_NAME_MAX + + The maximum length the name of an extended attribute (including the + namespace qualifier) can be. Currently, this is 255 bytes. + + .. availability:: Linux. + + .. versionadded:: 3.12 + +.. data:: XATTR_MAXNAMELEN + + The maximum length the name of an extended attribute can be. Currently, this + is 127 UTF-8 bytes. + + .. availability:: macOS. + + .. versionadded:: 3.12 + +.. data:: EXTATTR_MAXNAMELEN + + The maximum length the name of an extended attribute (excluding the + namespace qualifier) can be. Currently, this is 255 bytes. + + .. availability:: FreeBSD. + + .. versionadded:: 3.12 .. data:: XATTR_CREATE This is a possible value for the flags argument in :func:`setxattr`. It indicates the operation must create an attribute. + .. availability:: Linux, macOS, FreeBSD. .. data:: XATTR_REPLACE @@ -3825,6 +3873,8 @@ These functions are all available on Linux only. indicates the operation must replace an existing attribute. + .. availability:: Linux, macOS, FreeBSD. + .. _os-process: Process Management From bdcd611b52b5452c6e59771a336cc95e2fd567a1 Mon Sep 17 00:00:00 2001 From: Davide Rizzo Date: Mon, 1 May 2023 15:31:18 +0200 Subject: [PATCH 05/10] xattrs doc update --- Doc/library/os.rst | 34 +++++++++++++++++++++++++++------- 1 file changed, 27 insertions(+), 7 deletions(-) diff --git a/Doc/library/os.rst b/Doc/library/os.rst index dd1646aa249213..079e180a37a731 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -3731,10 +3731,26 @@ Extended attributes ~~~~~~~~~~~~~~~~~~~ .. versionadded:: 3.3 + Added suppport for Linux. -.. versionchanged:: 3.12 +.. versionadded:: 3.12 Added support for macOS and FreeBSD. +These functions provide access to file system extended attributes on the Unix +platforms that support them. + +.. note:: + + On some platforms, extended attribute names have a *namespace qualifier* + prefix such as ``'user.'`` or ``'system.'``. + + On FreeBSD, Python requires that the caller of functions that expect an + attribute name specify the namespace qualifier, which is interpreted + according to ``extattr_string_to_namespace()``. The rest of the string is + interpreted as the attribute name. E.g. Python will interpret the + ``'user.mime_type'`` string as the attribute name ``'mime_type'`` in the + ``EXTATTR_NAMESPACE_USER`` namespace. + .. function:: getxattr(path, attribute, *, follow_symlinks=True) Return the value of the extended filesystem attribute *attribute* for @@ -3752,7 +3768,7 @@ Extended attributes .. versionchanged:: 3.6 Accepts a :term:`path-like object` for *path* and *attribute*. - .. versionchanged:: 3.12 + .. versionadded:: 3.12 Added support for macOS and FreeBSD. .. function:: listxattr(path=None, *, follow_symlinks=True) @@ -3765,6 +3781,12 @@ Extended attributes This function can support :ref:`specifying a file descriptor ` and :ref:`not following symlinks `. + .. note:: + + On FreeBSD, this function will only list attributes residing in the + ``user`` and ``system`` (if permitted) namespaces, prefixed with a + namespace qualifier. + .. audit-event:: os.listxattr path os.listxattr .. availability:: Linux, macOS, FreeBSD. @@ -3772,9 +3794,7 @@ Extended attributes .. versionchanged:: 3.6 Accepts a :term:`path-like object`. - .. availability:: Linux, macOS, FreeBSD. - - .. versionchanged:: 3.12 + .. versionadded:: 3.12 Added support for macOS and FreeBSD. .. function:: removexattr(path, attribute, *, follow_symlinks=True) @@ -3794,7 +3814,7 @@ Extended attributes .. versionchanged:: 3.6 Accepts a :term:`path-like object` for *path* and *attribute*. - .. versionchanged:: 3.12 + .. versionadded:: 3.12 Added support for macOS and FreeBSD. .. function:: setxattr(path, attribute, value, flags=0, *, follow_symlinks=True) @@ -3823,7 +3843,7 @@ Extended attributes .. versionchanged:: 3.6 Accepts a :term:`path-like object` for *path* and *attribute*. - .. versionchanged:: 3.12 + .. versionadded:: 3.12 Added support for macOS and FreeBSD. .. data:: XATTR_SIZE_MAX From 6f3bde31c63ab0abc9fe225459aa8c8ab0f75ce2 Mon Sep 17 00:00:00 2001 From: Davide Rizzo Date: Tue, 16 May 2023 18:29:33 +0200 Subject: [PATCH 06/10] wip --- Modules/posixmodule.c | 141 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 125 insertions(+), 16 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 65872c9f8f7fff..3bc679c743677a 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -316,18 +316,20 @@ xattrnamespace(int *namespace, const char *fullattr) if (sep < fullattr) return ENOATTR; - nslen = sep - fullattr + 1; + nslen = sep - fullattr; - if (strncmp(fullattr, "user", (4 < nslen ? 4 : nslen)) == 0) { + // We could use extattr_string_to_namespace, but we have a prefix, not a + // null-terminated string, at this point. extattr_string_to_namespace is + // currently documented to only support 'user' and 'system' namespaces. + if (nslen == 4 && memcmp(fullattr, "user", 4) == 0) { *namespace = EXTATTR_NAMESPACE_USER; - } else if (strncmp(fullattr, "system", (6 < nslen ? 6 : nslen)) == 0) { + } else if (nslen == 6 && memcmp(fullattr, "system", 6) == 0) { *namespace = EXTATTR_NAMESPACE_SYSTEM; + } else { + return ENOATTR; } - if (*namespace == -1) - ret = ENOATTR; - - return ret; + return 0; } /* xattr lists come back from FreeBSD without the leading namespace. @@ -361,7 +363,7 @@ normalize_xattr_list(int namespace, char *attrs, size_t size, ssize_t extattr_re if (size - SSIZE_MAX < SSIZE_MAX) return -ERANGE; - if (! (scratch = calloc(1, size))) + if (! (scratch = PyMem_RawCalloc(1, size))) return -ENOMEM; scratch_ptr = scratch; @@ -393,7 +395,7 @@ normalize_xattr_list(int namespace, char *attrs, size_t size, ssize_t extattr_re ret = size - left; bad: - free(scratch); + PyMem_RawFree(scratch); return ret; } @@ -14248,6 +14250,95 @@ os_removexattr_impl(PyObject *module, path_t *path, path_t *attribute, Py_RETURN_NONE; } +#ifdef __FreeBSD__ + +static PyObject * +freebsd_listxattr_impl(PyObject* list, path_t *path, int follow_symlinks) { + PyObject *result = list; + int namespaces[] = {EXTATTR_NAMESPACE_SYSTEM, EXTATTR_NAMESPACE_USER}; + char *nsprefix[] = {"system.", "user."}; + int privileged_namespaces = 1; + + char *name = path->narrow ? path->narrow : "."; + + Py_BEGIN_ALLOW_THREADS; + for (int i = 0; i < sizeof(namespaces) / sizeof(int); i++) { + ssize_t length; + int namespace = namespaces[i]; + if (path->fd > -1) + length = extattr_list_fd(path->fd, namespace, NULL, 0); + else if (follow_symlinks) + length = extattr_list_file(name, namespace, NULL, 0); + else + length = extattr_list_link(name, namespace, NULL, 0); + + if (length < 0) { + if (i < privileged_namespaces && errno == EPERM) { + continue; + } else { + path_error(path); + result = NULL; + break; + } + } + + char* buffer = PyMem_RawMalloc(length); + if (path->fd > -1) + length = extattr_list_fd(path->fd, namespace, buffer, length); + else if (follow_symlinks) + length = extattr_list_file(name, namespace, buffer, length); + else + length = extattr_list_link(name, namespace, buffer, length); + + if (length < 0) { + path_error(path); + result = NULL; + break; + } + + // buffer to build "{namespace}.{attr}" strings, must include prefix: + char attr_name[EXTATTR_MAXNAMELEN + 8]; + int attr_name_size = sizeof(attr_name); + + char *next = buffer; + char *end = buffer + length; + char *attr_suffix = stpcpy(attr_name, nsprefix[i]); + ssize_t attr_buf_size = attr_name + attr_name_size - attr_suffix; + Py_BLOCK_THREADS; + while (next < end) { + char attr_len = *next; + if (attr_len >= attr_buf_size) { + result = NULL; + // TODO handle memory error + break; + } + char *attr_name_end = mempcpy(attr_suffix, next + 1, attr_len); + *attr_name_end = '\0'; + int error; + PyObject *attribute = PyUnicode_DecodeFSDefaultAndSize(attr_name, + attr_name_end - attr_name); + if (!attribute) { + result = NULL; + break; + } + error = PyList_Append(list, attribute); + Py_DECREF(attribute); + if (error) { + result = NULL; + break; + } + next += attr_len + 1; + } + Py_UNBLOCK_THREADS; + if (result == NULL) { + break; + } + } + Py_END_ALLOW_THREADS; + + return result; +} +#endif /*[clinic input] os.listxattr @@ -14269,13 +14360,6 @@ static PyObject * os_listxattr_impl(PyObject *module, path_t *path, int follow_symlinks) /*[clinic end generated code: output=bebdb4e2ad0ce435 input=9826edf9fdb90869]*/ { - Py_ssize_t i; - PyObject *result = NULL; - const char *name; - char *buffer = NULL, *start, *end, *trace; - size_t buffer_size = 0; - ssize_t length = 0; - if (fd_and_follow_symlinks_invalid("listxattr", path->fd, follow_symlinks)) goto exit; @@ -14284,6 +14368,14 @@ os_listxattr_impl(PyObject *module, path_t *path, int follow_symlinks) return NULL; } +#if defined(HAVE_SYS_XATTR_H) + Py_ssize_t i; + PyObject *result = NULL; + const char *name; + char *buffer = NULL, *start, *end, *trace; + size_t buffer_size = 0; + ssize_t length = 0; + name = path->narrow ? path->narrow : "."; for (i = 0; i < 2; i++) { @@ -14349,6 +14441,23 @@ os_listxattr_impl(PyObject *module, path_t *path, int follow_symlinks) if (buffer) PyMem_Free(buffer); return result; + +#else if defined(HAVE_SYS_EXTATTR_H) || defined(__FreeBSD__) + + PyObject *list = PyList_New(0); + if (!list) { + goto exit; + } + PyObject *result = freebsd_listxattr_impl(list, path, follow_symlinks); + if (!result) { + Py_DECREF(list); + return NULL; + } +exit: + return result; + +#endif + } #endif /* USE_XATTRS */ From 3a78e5c8c84926c8ceb2d116d82f970b0ecfa1c3 Mon Sep 17 00:00:00 2001 From: Davide Rizzo Date: Tue, 16 May 2023 18:29:33 +0200 Subject: [PATCH 07/10] wip --- Modules/posixmodule.c | 142 +++++++++++++++++++++++++++++++++++++----- 1 file changed, 125 insertions(+), 17 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 65872c9f8f7fff..0a7621985a5fe8 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -306,7 +306,6 @@ xattrnamespace(int *namespace, const char *fullattr) { char *sep = strchr(fullattr, '.'); size_t nslen = 0; - int ret = 0; *namespace = -1; // return ENOATTR on unqualified names @@ -316,18 +315,20 @@ xattrnamespace(int *namespace, const char *fullattr) if (sep < fullattr) return ENOATTR; - nslen = sep - fullattr + 1; + nslen = sep - fullattr; - if (strncmp(fullattr, "user", (4 < nslen ? 4 : nslen)) == 0) { + // We could use extattr_string_to_namespace, but we have a prefix, not a + // null-terminated string, at this point. extattr_string_to_namespace is + // currently documented to only support 'user' and 'system' namespaces. + if (nslen == 4 && memcmp(fullattr, "user", 4) == 0) { *namespace = EXTATTR_NAMESPACE_USER; - } else if (strncmp(fullattr, "system", (6 < nslen ? 6 : nslen)) == 0) { + } else if (nslen == 6 && memcmp(fullattr, "system", 6) == 0) { *namespace = EXTATTR_NAMESPACE_SYSTEM; + } else { + return ENOATTR; } - if (*namespace == -1) - ret = ENOATTR; - - return ret; + return 0; } /* xattr lists come back from FreeBSD without the leading namespace. @@ -361,7 +362,7 @@ normalize_xattr_list(int namespace, char *attrs, size_t size, ssize_t extattr_re if (size - SSIZE_MAX < SSIZE_MAX) return -ERANGE; - if (! (scratch = calloc(1, size))) + if (! (scratch = PyMem_RawCalloc(1, size))) return -ENOMEM; scratch_ptr = scratch; @@ -393,7 +394,7 @@ normalize_xattr_list(int namespace, char *attrs, size_t size, ssize_t extattr_re ret = size - left; bad: - free(scratch); + PyMem_RawFree(scratch); return ret; } @@ -14248,6 +14249,95 @@ os_removexattr_impl(PyObject *module, path_t *path, path_t *attribute, Py_RETURN_NONE; } +#ifdef __FreeBSD__ + +static PyObject * +freebsd_listxattr_impl(PyObject* list, path_t *path, int follow_symlinks) { + PyObject *result = list; + int namespaces[] = {EXTATTR_NAMESPACE_SYSTEM, EXTATTR_NAMESPACE_USER}; + char *nsprefix[] = {"system.", "user."}; + int privileged_namespaces = 1; + + char *name = path->narrow ? path->narrow : "."; + + Py_BEGIN_ALLOW_THREADS; + for (int i = 0; i < sizeof(namespaces) / sizeof(int); i++) { + ssize_t length; + int namespace = namespaces[i]; + if (path->fd > -1) + length = extattr_list_fd(path->fd, namespace, NULL, 0); + else if (follow_symlinks) + length = extattr_list_file(name, namespace, NULL, 0); + else + length = extattr_list_link(name, namespace, NULL, 0); + + if (length < 0) { + if (i < privileged_namespaces && errno == EPERM) { + continue; + } else { + path_error(path); + result = NULL; + break; + } + } + + char* buffer = PyMem_RawMalloc(length); + if (path->fd > -1) + length = extattr_list_fd(path->fd, namespace, buffer, length); + else if (follow_symlinks) + length = extattr_list_file(name, namespace, buffer, length); + else + length = extattr_list_link(name, namespace, buffer, length); + + if (length < 0) { + path_error(path); + result = NULL; + break; + } + + // buffer to build "{namespace}.{attr}" strings, must include prefix: + char attr_name[EXTATTR_MAXNAMELEN + 8]; + int attr_name_size = sizeof(attr_name); + + char *next = buffer; + char *end = buffer + length; + char *attr_suffix = stpcpy(attr_name, nsprefix[i]); + ssize_t attr_buf_size = attr_name + attr_name_size - attr_suffix; + Py_BLOCK_THREADS; + while (next < end) { + char attr_len = *next; + if (attr_len >= attr_buf_size) { + result = NULL; + // TODO handle memory error + break; + } + char *attr_name_end = mempcpy(attr_suffix, next + 1, attr_len); + *attr_name_end = '\0'; + int error; + PyObject *attribute = PyUnicode_DecodeFSDefaultAndSize(attr_name, + attr_name_end - attr_name); + if (!attribute) { + result = NULL; + break; + } + error = PyList_Append(list, attribute); + Py_DECREF(attribute); + if (error) { + result = NULL; + break; + } + next += attr_len + 1; + } + Py_UNBLOCK_THREADS; + if (result == NULL) { + break; + } + } + Py_END_ALLOW_THREADS; + + return result; +} +#endif /*[clinic input] os.listxattr @@ -14269,13 +14359,6 @@ static PyObject * os_listxattr_impl(PyObject *module, path_t *path, int follow_symlinks) /*[clinic end generated code: output=bebdb4e2ad0ce435 input=9826edf9fdb90869]*/ { - Py_ssize_t i; - PyObject *result = NULL; - const char *name; - char *buffer = NULL, *start, *end, *trace; - size_t buffer_size = 0; - ssize_t length = 0; - if (fd_and_follow_symlinks_invalid("listxattr", path->fd, follow_symlinks)) goto exit; @@ -14284,6 +14367,14 @@ os_listxattr_impl(PyObject *module, path_t *path, int follow_symlinks) return NULL; } +#if defined(HAVE_SYS_XATTR_H) + Py_ssize_t i; + PyObject *result = NULL; + const char *name; + char *buffer = NULL, *start, *end, *trace; + size_t buffer_size = 0; + ssize_t length = 0; + name = path->narrow ? path->narrow : "."; for (i = 0; i < 2; i++) { @@ -14349,6 +14440,23 @@ os_listxattr_impl(PyObject *module, path_t *path, int follow_symlinks) if (buffer) PyMem_Free(buffer); return result; + +#else if defined(HAVE_SYS_EXTATTR_H) || defined(__FreeBSD__) + + PyObject *list = PyList_New(0); + if (!list) { + goto exit; + } + PyObject *result = freebsd_listxattr_impl(list, path, follow_symlinks); + if (!result) { + Py_DECREF(list); + return NULL; + } +exit: + return result; + +#endif + } #endif /* USE_XATTRS */ From dc07c31ace0a46ebcf3edca5f40ab6e8fd830a98 Mon Sep 17 00:00:00 2001 From: Davide Rizzo Date: Tue, 5 Sep 2023 14:46:34 +0200 Subject: [PATCH 08/10] wip --- Modules/posixmodule.c | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 0a7621985a5fe8..c3462785ca81c5 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -14260,17 +14260,17 @@ freebsd_listxattr_impl(PyObject* list, path_t *path, int follow_symlinks) { char *name = path->narrow ? path->narrow : "."; - Py_BEGIN_ALLOW_THREADS; for (int i = 0; i < sizeof(namespaces) / sizeof(int); i++) { ssize_t length; int namespace = namespaces[i]; + Py_BEGIN_ALLOW_THREADS; if (path->fd > -1) length = extattr_list_fd(path->fd, namespace, NULL, 0); else if (follow_symlinks) length = extattr_list_file(name, namespace, NULL, 0); else length = extattr_list_link(name, namespace, NULL, 0); - + Py_END_ALLOW_THREADS; if (length < 0) { if (i < privileged_namespaces && errno == EPERM) { continue; @@ -14281,18 +14281,23 @@ freebsd_listxattr_impl(PyObject* list, path_t *path, int follow_symlinks) { } } - char* buffer = PyMem_RawMalloc(length); + char* buffer = PyMem_Malloc(length); + if (buffer == NULL) { + result = NULL; + break; + } + Py_BEGIN_ALLOW_THREADS; if (path->fd > -1) length = extattr_list_fd(path->fd, namespace, buffer, length); else if (follow_symlinks) length = extattr_list_file(name, namespace, buffer, length); else length = extattr_list_link(name, namespace, buffer, length); + Py_END_ALLOW_THREADS; if (length < 0) { path_error(path); - result = NULL; - break; + goto bad; } // buffer to build "{namespace}.{attr}" strings, must include prefix: @@ -14303,7 +14308,6 @@ freebsd_listxattr_impl(PyObject* list, path_t *path, int follow_symlinks) { char *end = buffer + length; char *attr_suffix = stpcpy(attr_name, nsprefix[i]); ssize_t attr_buf_size = attr_name + attr_name_size - attr_suffix; - Py_BLOCK_THREADS; while (next < end) { char attr_len = *next; if (attr_len >= attr_buf_size) { @@ -14328,12 +14332,14 @@ freebsd_listxattr_impl(PyObject* list, path_t *path, int follow_symlinks) { } next += attr_len + 1; } - Py_UNBLOCK_THREADS; - if (result == NULL) { - break; - } + if (result == NULL) + goto bad; + continue; + bad: + PyMem_Free(buffer); + result = NULL; + break; } - Py_END_ALLOW_THREADS; return result; } From e65ac6791c8f50546a0be3e5d61a10823f5afe14 Mon Sep 17 00:00:00 2001 From: Davide Rizzo Date: Tue, 5 Sep 2023 15:53:08 +0200 Subject: [PATCH 09/10] autoreconf --- configure | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/configure b/configure index d73b4b271ac719..2fa3d2716d33b5 100755 --- a/configure +++ b/configure @@ -10529,6 +10529,12 @@ if test "x$ac_cv_header_sys_eventfd_h" = xyes then : printf "%s\n" "#define HAVE_SYS_EVENTFD_H 1" >>confdefs.h +fi +ac_fn_c_check_header_compile "$LINENO" "sys/extattr.h" "ac_cv_header_sys_extattr_h" "$ac_includes_default" +if test "x$ac_cv_header_sys_extattr_h" = xyes +then : + printf "%s\n" "#define HAVE_SYS_EXTATTR_H 1" >>confdefs.h + fi ac_fn_c_check_header_compile "$LINENO" "sys/file.h" "ac_cv_header_sys_file_h" "$ac_includes_default" if test "x$ac_cv_header_sys_file_h" = xyes From 466ca9c18ca8f2267502f991e278b029efa0f13a Mon Sep 17 00:00:00 2001 From: Davide Rizzo Date: Wed, 13 Sep 2023 15:01:52 +0000 Subject: [PATCH 10/10] wip --- Modules/posixmodule.c | 498 ++++++++++++------------------------------ 1 file changed, 138 insertions(+), 360 deletions(-) diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 572bce5e9b70ba..e001e186311c6d 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -316,26 +316,23 @@ corresponding Unix manual entries for more information on calls."); #if defined(USE_XATTRS) && defined(__FreeBSD__) -/* Turns a qualified string into a namespace id on FreeBSD. - * This ensures that that the namespace is qualified - * ((user|system).) and that is non-null. - */ -static int -xattrnamespace(int *namespace, const char *fullattr) +// Split a Linux-style qualified xattr name into its components for FreeBSD. +// +// This ensures that that the namespace is qualified ((user|system).) and +// that is non-empty. +// +// Sets errno. +static void +xattr_split_namespace(const char *fullattr, int *namespace, const char **attr_name) { char *sep = strchr(fullattr, '.'); - size_t nslen = 0; - - *namespace = -1; - // return ENOATTR on unqualified names - if (sep == NULL || *(sep + 1) == '\0') - return ENOATTR; - - if (sep < fullattr) - return ENOATTR; - - nslen = sep - fullattr; - + // fail with ENOATTR on unqualified or empty names + *attr_name = NULL; + if (sep == NULL || *(sep + 1) == '\0' || sep < fullattr) { + errno = ENOATTR; + return; + } + size_t nslen = sep - fullattr; // We could use extattr_string_to_namespace, but we have a prefix, not a // null-terminated string, at this point. extattr_string_to_namespace is // currently documented to only support 'user' and 'system' namespaces. @@ -344,146 +341,58 @@ xattrnamespace(int *namespace, const char *fullattr) } else if (nslen == 6 && memcmp(fullattr, "system", 6) == 0) { *namespace = EXTATTR_NAMESPACE_SYSTEM; } else { - return ENOATTR; - } - - return 0; -} - -/* xattr lists come back from FreeBSD without the leading namespace. - * We need to translate this into a linux-like format - */ -static ssize_t -normalize_xattr_list(int namespace, char *attrs, size_t size, ssize_t extattr_ret) -{ - char *scratch = NULL, *scratch_ptr; - ssize_t ret = 0; - ssize_t i = 0, left = size, attr_size; - - /* FreeBSD only returns the size of the attribute names, not including the - * namespaces. In order to guess a buffer size, we multiply the size - * returned by the longest possible prefix - "system.". This accounts for - * the case where each attribute is 1 character in length, and is a system - * attribute. - * - * This does unfortunately give us longer buffers than we need, but since - * the extattr(2) interface gives us no way to detect overflow, this is - * the best we can do to guess the size. - */ - if (! attrs && extattr_ret) { - if (SSIZE_MAX / extattr_ret <= strlen("system.")) { - return -ERANGE; - } - return strlen("system.") * extattr_ret; - } - - /* overflow */ - if (size - SSIZE_MAX < SSIZE_MAX) - return -ERANGE; - - if (! (scratch = PyMem_RawCalloc(1, size))) - return -ENOMEM; - - scratch_ptr = scratch; - - for (i = 0; i < size && attrs[i] != 0;) { - attr_size = attrs[i]; - if (namespace == EXTATTR_NAMESPACE_USER) { - strncat(scratch_ptr, "user.", left); - scratch_ptr += strlen("user."); - left -= strlen("user."); - } else if (namespace == EXTATTR_NAMESPACE_SYSTEM) { - strncat(scratch_ptr, "system.", left); - scratch_ptr += strlen("system."); - left -= strlen("system."); - } - - if (attr_size > left) { - ret = -ENOMEM; - goto bad; - } - - memcpy(scratch_ptr, attrs + i + 1, attr_size); - scratch_ptr += attr_size + 1; - i += attr_size + 1; - left -= attr_size + 1; + errno = ENOATTR; + return; } - - memcpy(attrs, scratch, size); - ret = size - left; - -bad: - PyMem_RawFree(scratch); - - return ret; + *attr_name = fullattr + nslen + 1; } +/* FreeBSD wrapper that mimics Linux getxattr */ static ssize_t getxattr(const char *path, const char *name, void *value, size_t size) { int nsid = 0; - int err = 0; - char *attr_name = NULL; - - if ((err = xattrnamespace(&nsid, name))) { - errno = err; + const char *attr_name = NULL; + xattr_split_namespace(name, &nsid, &attr_name); + if (attr_name == NULL) return -1; - } - - attr_name = strchr(name, '.') + 1; - return extattr_get_file(path, nsid, attr_name, value, size); } +/* FreeBSD wrapper that mimics Linux lgetxattr */ static ssize_t lgetxattr(const char *path, const char *name, void *value, size_t size) { int nsid = 0; - int err = 0; - char *attr_name = NULL; - - if ((err = xattrnamespace(&nsid, name))) { - errno = err; + const char *attr_name = NULL; + xattr_split_namespace(name, &nsid, &attr_name); + if (attr_name == NULL) return -1; - } - - attr_name = strchr(name, '.') + 1; - return extattr_get_link(path, nsid, attr_name, value, size); } +/* FreeBSD wrapper that mimics Linux fgetxattr */ static ssize_t fgetxattr(int fd, const char *name, void *value, size_t size) { int nsid = 0; - int err = 0; - char *attr_name = NULL; - - if ((err = xattrnamespace(&nsid, name))) { - errno = err; + const char *attr_name = NULL; + xattr_split_namespace(name, &nsid, &attr_name); + if (attr_name == NULL) return -1; - } - - attr_name = strchr(name, '.') + 1; - return extattr_get_fd(fd, nsid, attr_name, value, size); } +/* FreeBSD wrapper that mimics Linux setxattr */ static int setxattr(const char *path, const char *name, const void *value, size_t size, int flags) { int nsid = 0; - int err = 0; - char *attr_name = NULL; - ssize_t get_ret = 0; - - if ((err = xattrnamespace(&nsid, name))) { - errno = err; + const char *attr_name = NULL; + xattr_split_namespace(name, &nsid, &attr_name); + if (attr_name == NULL) return -1; - } - - attr_name = strchr(name, '.') + 1; - + ssize_t get_ret = 0; if (flags) { get_ret = extattr_get_file(path, nsid, attr_name, NULL, 0); if (flags & XATTR_REPLACE && get_ret == -1) { @@ -500,21 +409,16 @@ setxattr(const char *path, const char *name, const void *value, size_t size, int return -1; } +/* FreeBSD wrapper that mimics Linux lsetxattr */ static int lsetxattr(const char *path, const char *name, const void *value, size_t size, int flags) { int nsid = 0; - int err = 0; - ssize_t get_ret = 0; - char *attr_name = NULL; - - if ((err = xattrnamespace(&nsid, name))) { - errno = err; + const char *attr_name = NULL; + xattr_split_namespace(name, &nsid, &attr_name); + if (attr_name == NULL) return -1; - } - - attr_name = strchr(name, '.') + 1; - + ssize_t get_ret = 0; if (flags) { get_ret = extattr_get_link(path, nsid, attr_name, NULL, 0); if (flags & XATTR_REPLACE && get_ret == -1) { @@ -531,21 +435,16 @@ lsetxattr(const char *path, const char *name, const void *value, size_t size, in return -1; } +/* FreeBSD wrapper that mimics Linux fsetxattr */ static int fsetxattr(int fd, const char *name, const void *value, size_t size, int flags) { int nsid = 0; - int err = 0; - ssize_t get_ret = 0; - char *attr_name = NULL; - - if ((err = xattrnamespace(&nsid, name))) { - errno = err; + const char *attr_name = NULL; + xattr_split_namespace(name, &nsid, &attr_name); + if (attr_name == NULL) return -1; - } - - attr_name = strchr(name, '.') + 1; - + ssize_t get_ret = 0; if (flags) { get_ret = extattr_get_fd(fd, nsid, attr_name, NULL, 0); if (flags & XATTR_REPLACE && get_ret == -1) { @@ -562,159 +461,42 @@ fsetxattr(int fd, const char *name, const void *value, size_t size, int flags) return -1; } +/* FreeBSD wrapper that mimics Linux removexattr */ static int removexattr(const char *path, const char *name) { int nsid = 0; - int err = 0; - char *attr_name = NULL; - - if ((err = xattrnamespace(&nsid, name))) { - errno = err; + const char *attr_name = NULL; + xattr_split_namespace(name, &nsid, &attr_name); + if (attr_name == NULL) return -1; - } - - attr_name = strchr(name, '.') + 1; - return extattr_delete_file(path, nsid, attr_name); } +/* FreeBSD wrapper that mimics Linux lremovexattr */ static int lremovexattr(const char *path, const char *name) { int nsid = 0; - int err = 0; - char *attr_name = NULL; - - if ((err = xattrnamespace(&nsid, name))) { - errno = err; + const char *attr_name = NULL; + xattr_split_namespace(name, &nsid, &attr_name); + if (attr_name == NULL) return -1; - } - - attr_name = strchr(name, '.') + 1; - return extattr_delete_link(path, nsid, attr_name); } +/* FreeBSD wrapper that mimics Linux fremovexattr */ static int fremovexattr(int fd, const char *name) { int nsid = 0; - int err = 0; - char *attr_name = NULL; - - if ((err = xattrnamespace(&nsid, name))) { - errno = err; + const char *attr_name = NULL; + xattr_split_namespace(name, &nsid, &attr_name); + if (attr_name == NULL) return -1; - } - - attr_name = strchr(name, '.') + 1; - return extattr_delete_fd(fd, nsid, attr_name); } -static ssize_t -listxattr(const char *path, char *list, size_t size) -{ - ssize_t ret = 0, retadder = 0; - - memset(list, 0, size); - retadder = extattr_list_file(path, EXTATTR_NAMESPACE_SYSTEM, list, size); - if (retadder == -1 && errno != EPERM) { - return retadder; - } else if (retadder > 0) { - retadder = normalize_xattr_list(EXTATTR_NAMESPACE_SYSTEM, list, size, retadder); - if (retadder < 0) - return retadder; - ret += retadder; - } - - retadder = extattr_list_file(path, EXTATTR_NAMESPACE_USER, list + ret, size - ret); - if (retadder == -1) { - return retadder; - } else { - retadder = normalize_xattr_list(EXTATTR_NAMESPACE_USER, list + ret, size - ret, retadder); - if (retadder < 0) - return retadder; - ret += retadder; - } - - // empty - if (ret < 2) { - return 0; - } - - return ret; -} - -static ssize_t -llistxattr(const char *path, char *list, size_t size) -{ - ssize_t ret = 0, retadder = 0; - - memset(list, 0, size); - retadder = extattr_list_link(path, EXTATTR_NAMESPACE_SYSTEM, list, size); - if (retadder == -1 && errno != EPERM) { - return retadder; - } else if (retadder > 0) { - retadder = normalize_xattr_list(EXTATTR_NAMESPACE_SYSTEM, list, size, retadder); - if (retadder < 0) - return retadder; - ret += retadder; - } - - retadder = extattr_list_link(path, EXTATTR_NAMESPACE_USER, list + ret, size - retadder); - if (retadder == -1) { - return retadder; - } else { - retadder = normalize_xattr_list(EXTATTR_NAMESPACE_USER, list + ret, size - ret, retadder); - if (retadder < 0) - return retadder; - ret += retadder; - } - - // empty - if (ret < 2) { - return 0; - } - - return ret; -} - -static ssize_t -flistxattr(int fd, char *list, size_t size) -{ - ssize_t ret = 0, retadder = 0; - - memset(list, 0, size); - retadder = extattr_list_fd(fd, EXTATTR_NAMESPACE_SYSTEM, list, size); - if (retadder == -1 && errno != EPERM) { - return retadder; - } else if (retadder > 0) { - retadder = normalize_xattr_list(EXTATTR_NAMESPACE_SYSTEM, list, size, retadder); - if (retadder < 0) - return retadder; - ret += retadder; - } - - retadder = extattr_list_fd(fd, EXTATTR_NAMESPACE_USER, list + ret, size - retadder); - if (retadder == -1) { - return retadder; - } else { - retadder = normalize_xattr_list(EXTATTR_NAMESPACE_USER, list + ret, size - ret, retadder); - if (retadder < 0) - return retadder; - ret += retadder; - } - - // empty - if (ret < 2) { - return 0; - } - - return ret; -} - #endif #if defined(__FreeBSD__) || defined(__DragonFly__) || defined(__APPLE__) @@ -14242,10 +14024,6 @@ os_getxattr_impl(PyObject *module, path_t *path, path_t *attribute, int follow_symlinks) /*[clinic end generated code: output=5f2f44200a43cff2 input=025789491708f7eb]*/ { - Py_ssize_t i; - PyObject *buffer = NULL; - Py_ssize_t buffer_size = 0; - if (fd_and_follow_symlinks_invalid("getxattr", path->fd, follow_symlinks)) return NULL; @@ -14253,51 +14031,62 @@ os_getxattr_impl(PyObject *module, path_t *path, path_t *attribute, return NULL; } - for (i = 0; i < 2; i++) { - void *ptr = NULL; - ssize_t result; - if (i) { - buffer = PyBytes_FromStringAndSize(NULL, buffer_size); - if (!buffer) - return NULL; - ptr = PyBytes_AS_STRING(buffer); - } - - Py_BEGIN_ALLOW_THREADS; + ssize_t result; + // First, query buffer size + Py_BEGIN_ALLOW_THREADS; #ifdef __APPLE__ - if (path->fd >= 0) - result = fgetxattr(path->fd, attribute->narrow, ptr, buffer_size, - 0, 0); - else if (follow_symlinks) - result = getxattr(path->narrow, attribute->narrow, ptr, - buffer_size, 0, 0); - else - result = getxattr(path->narrow, attribute->narrow, ptr, - buffer_size, 0, XATTR_NOFOLLOW); + if (path->fd >= 0) + result = fgetxattr(path->fd, attribute->narrow, NULL, 0, 0, 0); + else if (follow_symlinks) + result = getxattr(path->narrow, attribute->narrow, NULL, 0, 0, 0); + else + result = getxattr(path->narrow, attribute->narrow, NULL, 0, 0, + XATTR_NOFOLLOW); #else - if (path->fd >= 0) - result = fgetxattr(path->fd, attribute->narrow, ptr, buffer_size); - else if (follow_symlinks) - result = getxattr(path->narrow, attribute->narrow, ptr, buffer_size); - else - result = lgetxattr(path->narrow, attribute->narrow, ptr, buffer_size); + if (path->fd >= 0) + result = fgetxattr(path->fd, attribute->narrow, NULL, 0); + else if (follow_symlinks) + result = getxattr(path->narrow, attribute->narrow, NULL, 0); + else + result = lgetxattr(path->narrow, attribute->narrow, NULL, 0); #endif - Py_END_ALLOW_THREADS; - - if (result < 0) { - if (errno == ERANGE) { - Py_DECREF(buffer); - continue; - } - path_error(path); - Py_DECREF(buffer); - return NULL; - } - - if (! buffer_size) - buffer_size = result; + Py_END_ALLOW_THREADS; + if (result < 0) { + path_error(path); + return NULL; } + // Next, allocate and query the actual value + Py_ssize_t buffer_size = result; + PyObject *buffer = PyBytes_FromStringAndSize(NULL, buffer_size); + if (!buffer) + return NULL; + void *ptr = PyBytes_AS_STRING(buffer); + Py_BEGIN_ALLOW_THREADS; +#ifdef __APPLE__ + if (path->fd >= 0) + result = fgetxattr(path->fd, attribute->narrow, ptr, buffer_size, + 0, 0); + else if (follow_symlinks) + result = getxattr(path->narrow, attribute->narrow, ptr, + buffer_size, 0, 0); + else + result = getxattr(path->narrow, attribute->narrow, ptr, + buffer_size, 0, XATTR_NOFOLLOW); +#else + if (path->fd >= 0) + result = fgetxattr(path->fd, attribute->narrow, ptr, buffer_size); + else if (follow_symlinks) + result = getxattr(path->narrow, attribute->narrow, ptr, buffer_size); + else + result = lgetxattr(path->narrow, attribute->narrow, ptr, buffer_size); +#endif + Py_END_ALLOW_THREADS; + if (result < 0) { + path_error(path); + Py_DECREF(buffer); + return NULL; + } return buffer; } @@ -14430,15 +14219,16 @@ os_removexattr_impl(PyObject *module, path_t *path, path_t *attribute, static PyObject * freebsd_listxattr_impl(PyObject* list, path_t *path, int follow_symlinks) { PyObject *result = list; - int namespaces[] = {EXTATTR_NAMESPACE_SYSTEM, EXTATTR_NAMESPACE_USER}; - char *nsprefix[] = {"system.", "user."}; - int privileged_namespaces = 1; + const int namespaces[] = {EXTATTR_NAMESPACE_SYSTEM, EXTATTR_NAMESPACE_USER}; + const char *nsprefix[] = {"system.", "user."}; + const size_t max_nsprefix_len = 7; + const int privileged_namespaces = 1; - char *name = path->narrow ? path->narrow : "."; + const char *name = path->narrow ? path->narrow : "."; for (int i = 0; i < sizeof(namespaces) / sizeof(int); i++) { ssize_t length; - int namespace = namespaces[i]; + const int namespace = namespaces[i]; Py_BEGIN_ALLOW_THREADS; if (path->fd > -1) length = extattr_list_fd(path->fd, namespace, NULL, 0); @@ -14456,7 +14246,6 @@ freebsd_listxattr_impl(PyObject* list, path_t *path, int follow_symlinks) { break; } } - char* buffer = PyMem_Malloc(length); if (buffer == NULL) { result = NULL; @@ -14470,51 +14259,41 @@ freebsd_listxattr_impl(PyObject* list, path_t *path, int follow_symlinks) { else length = extattr_list_link(name, namespace, buffer, length); Py_END_ALLOW_THREADS; - if (length < 0) { + PyMem_Free(buffer); path_error(path); - goto bad; + result = NULL; + break; } - - // buffer to build "{namespace}.{attr}" strings, must include prefix: - char attr_name[EXTATTR_MAXNAMELEN + 8]; - int attr_name_size = sizeof(attr_name); - - char *next = buffer; - char *end = buffer + length; - char *attr_suffix = stpcpy(attr_name, nsprefix[i]); - ssize_t attr_buf_size = attr_name + attr_name_size - attr_suffix; - while (next < end) { - char attr_len = *next; - if (attr_len >= attr_buf_size) { - result = NULL; - // TODO handle memory error - break; - } - char *attr_name_end = mempcpy(attr_suffix, next + 1, attr_len); - *attr_name_end = '\0'; - int error; - PyObject *attribute = PyUnicode_DecodeFSDefaultAndSize(attr_name, - attr_name_end - attr_name); + static_assert(EXTATTR_MAXNAMELEN <= UCHAR_MAX, + "extattr length can exceed UCHAR_MAX"); + // buffer to build qualified "{namespace}.{attr}" strings, including + // prefix and separator: + char qual_name[UCHAR_MAX + 8]; + assert(sizeof(qual_name) >= EXTATTR_MAXNAMELEN + max_nsprefix_len + 1); + char *attr_suffix = stpcpy(qual_name, nsprefix[i]); + unsigned char attr_len; + for (size_t pos = 0; pos < length; pos += attr_len + 1) { + attr_len = (unsigned char)buffer[pos]; + const char *attr = buffer + pos + 1; + char *qual_name_end = mempcpy(attr_suffix, attr, attr_len); + const size_t qual_name_len = qual_name_end - qual_name; + PyObject *attribute = PyUnicode_DecodeFSDefaultAndSize(qual_name, qual_name_len); if (!attribute) { result = NULL; break; } - error = PyList_Append(list, attribute); + int error = PyList_Append(list, attribute); Py_DECREF(attribute); if (error) { result = NULL; break; } - next += attr_len + 1; } - if (result == NULL) - goto bad; - continue; - bad: PyMem_Free(buffer); - result = NULL; - break; + if (result == NULL) { + break; + } } return result; @@ -14542,7 +14321,7 @@ os_listxattr_impl(PyObject *module, path_t *path, int follow_symlinks) /*[clinic end generated code: output=bebdb4e2ad0ce435 input=9826edf9fdb90869]*/ { if (fd_and_follow_symlinks_invalid("listxattr", path->fd, follow_symlinks)) - goto exit; + return NULL; if (PySys_Audit("os.listxattr", "(O)", path->object ? path->object : Py_None) < 0) { @@ -14623,18 +14402,17 @@ os_listxattr_impl(PyObject *module, path_t *path, int follow_symlinks) PyMem_Free(buffer); return result; -#else if defined(HAVE_SYS_EXTATTR_H) || defined(__FreeBSD__) +#elif defined(HAVE_SYS_EXTATTR_H) || defined(__FreeBSD__) PyObject *list = PyList_New(0); if (!list) { - goto exit; + return NULL; } PyObject *result = freebsd_listxattr_impl(list, path, follow_symlinks); if (!result) { Py_DECREF(list); return NULL; } -exit: return result; #endif