gh-83714: Set os.statx().stx_mode to None if missing from stx_mask (#140484)

* Set stx_mode to None if STATX_TYPE|STATX_MODE is missing from
  stx_mask.
* Enhance os.statx() tests.
* statx_result structure: remove atime_sec, btime_sec, ctime_sec and
  mtime_sec members. Compute them on demand when stx_atime,
  stx_btime, stx_ctime and stx_mtime are read.
* Doc: fix statx members sorting.

Doc/library/os.rst

@@ -3413,11 +3413,6 @@ features:
 
    :class:`!statx_result` has the following attributes:
 
-   .. attribute:: stx_mask
-
-      Bitmask of :const:`STATX_* <STATX_TYPE>` constants specifying the
-      information retrieved, which may differ from what was requested.
-
    .. attribute:: stx_atime
 
       Time of most recent access expressed in seconds.
@@ -3442,16 +3437,6 @@ features:
       .. availability:: Linux >= 4.11 with glibc >= 2.28 and build-time kernel
          userspace API headers >= 6.11.
 
-   .. attribute:: stx_atomic_write_unit_min
-
-      Minimum size for direct I/O with torn-write protection.
-
-      Equal to ``None`` if :data:`STATX_WRITE_ATOMIC` is missing from
-      :attr:`~statx_result.stx_mask`.
-
-      .. availability:: Linux >= 4.11 with glibc >= 2.28 and build-time kernel
-         userspace API headers >= 6.11.
-
    .. attribute:: stx_atomic_write_unit_max
 
       Maximum size for direct I/O with torn-write protection.
@@ -3472,6 +3457,16 @@ features:
       .. availability:: Linux >= 4.11 with glibc >= 2.28 and build-time kernel
          userspace API headers >= 6.16.
 
+   .. attribute:: stx_atomic_write_unit_min
+
+      Minimum size for direct I/O with torn-write protection.
+
+      Equal to ``None`` if :data:`STATX_WRITE_ATOMIC` is missing from
+      :attr:`~statx_result.stx_mask`.
+
+      .. availability:: Linux >= 4.11 with glibc >= 2.28 and build-time kernel
+         userspace API headers >= 6.11.
+
    .. attribute:: stx_attributes
 
       Bitmask of :const:`STATX_ATTR_* <stat.STATX_ATTR_COMPRESSED>` constants
@@ -3536,9 +3531,9 @@ features:
 
       Minor number of the device on which this file resides.
 
-   .. attribute:: stx_dio_offset_align
+   .. attribute:: stx_dio_mem_align
 
-      Direct I/O file offset alignment requirement.
+      Direct I/O memory buffer alignment requirement.
 
       Equal to ``None`` if :data:`STATX_DIOALIGN` is missing from
       :attr:`~statx_result.stx_mask`.
@@ -3546,9 +3541,9 @@ features:
       .. availability:: Linux >= 4.11 with glibc >= 2.28 and build-time kernel
          userspace API headers >= 6.1.
 
-   .. attribute:: stx_dio_mem_align
+   .. attribute:: stx_dio_offset_align
 
-      Direct I/O memory buffer alignment requirement.
+      Direct I/O file offset alignment requirement.
 
       Equal to ``None`` if :data:`STATX_DIOALIGN` is missing from
       :attr:`~statx_result.stx_mask`.
@@ -3580,6 +3575,11 @@ features:
       Equal to ``None`` if :data:`STATX_INO` is missing from
       :attr:`~statx_result.stx_mask`.
 
+   .. attribute:: stx_mask
+
+      Bitmask of :const:`STATX_* <STATX_TYPE>` constants specifying the
+      information retrieved, which may differ from what was requested.
+
    .. attribute:: stx_mnt_id
 
       Mount identifier.
@@ -3594,6 +3594,9 @@ features:
 
       File mode: file type and file mode bits (permissions).
 
+      Equal to ``None`` if :data:`STATX_TYPE | STATX_MODE <STATX_TYPE>`
+      is missing from :attr:`~statx_result.stx_mask`.
+
    .. attribute:: stx_mtime
 
       Time of most recent content modification expressed in seconds.

Lib/test/test_os/test_os.py

@@ -748,7 +748,7 @@ def check_statx_attributes(self, filename):
             if name.startswith('STATX_'):
                 maximal_mask |= getattr(os, name)
         result = os.statx(filename, maximal_mask)
-        basic_result = os.stat(filename)
+        stat_result = os.stat(filename)
 
         time_attributes = ('stx_atime', 'stx_btime', 'stx_ctime', 'stx_mtime')
         # gh-83714: stx_btime can be None on tmpfs even if STATX_BTIME mask
@@ -757,62 +757,108 @@ def check_statx_attributes(self, filename):
                            if getattr(result, name) is not None]
         self.check_timestamp_agreement(result, time_attributes)
 
-        # Check that valid attributes match os.stat.
+        def getmask(name):
+            return getattr(os, name, 0)
+
         requirements = (
-            ('stx_mode', os.STATX_TYPE | os.STATX_MODE),
-            ('stx_nlink', os.STATX_NLINK),
-            ('stx_uid', os.STATX_UID),
-            ('stx_gid', os.STATX_GID),
             ('stx_atime', os.STATX_ATIME),
             ('stx_atime_ns', os.STATX_ATIME),
-            ('stx_mtime', os.STATX_MTIME),
+            ('stx_atomic_write_segments_max', getmask('STATX_WRITE_ATOMIC')),
-            ('stx_mtime_ns', os.STATX_MTIME),
+            ('stx_atomic_write_unit_max', getmask('STATX_WRITE_ATOMIC')),
+            ('stx_atomic_write_unit_max_opt', getmask('STATX_WRITE_ATOMIC')),
+            ('stx_atomic_write_unit_min', getmask('STATX_WRITE_ATOMIC')),
+            ('stx_attributes', 0),
+            ('stx_attributes_mask', 0),
+            ('stx_blksize', 0),
+            ('stx_blocks', os.STATX_BLOCKS),
+            ('stx_btime', os.STATX_BTIME),
+            ('stx_btime_ns', os.STATX_BTIME),
             ('stx_ctime', os.STATX_CTIME),
             ('stx_ctime_ns', os.STATX_CTIME),
-            ('stx_ino', os.STATX_INO),
-            ('stx_size', os.STATX_SIZE),
-            ('stx_blocks', os.STATX_BLOCKS),
-            ('stx_birthtime', os.STATX_BTIME),
-            ('stx_birthtime_ns', os.STATX_BTIME),
-            # unconditionally valid members
-            ('stx_blksize', 0),
-            ('stx_rdev', 0),
             ('stx_dev', 0),
+            ('stx_dev_major', 0),
+            ('stx_dev_minor', 0),
+            ('stx_dio_mem_align', getmask('STATX_DIOALIGN')),
+            ('stx_dio_offset_align', getmask('STATX_DIOALIGN')),
+            ('stx_dio_read_offset_align', getmask('STATX_DIO_READ_ALIGN')),
+            ('stx_gid', os.STATX_GID),
+            ('stx_ino', os.STATX_INO),
+            ('stx_mask', 0),
+            ('stx_mnt_id', getmask('STATX_MNT_ID')),
+            ('stx_mode', os.STATX_TYPE | os.STATX_MODE),
+            ('stx_mtime', os.STATX_MTIME),
+            ('stx_mtime_ns', os.STATX_MTIME),
+            ('stx_nlink', os.STATX_NLINK),
+            ('stx_rdev', 0),
+            ('stx_rdev_major', 0),
+            ('stx_rdev_minor', 0),
+            ('stx_size', os.STATX_SIZE),
+            ('stx_subvol', getmask('STATX_SUBVOL')),
+            ('stx_uid', os.STATX_UID),
         )
-        for name, bits in requirements:
+        optional_members = {
-            st_name = "st_" + name[4:]
+            'stx_atomic_write_segments_max',
-            if result.stx_mask & bits == bits and hasattr(basic_result, st_name):
+            'stx_atomic_write_unit_max',
-                x = getattr(result, name)
+            'stx_atomic_write_unit_max_opt',
-                b = getattr(basic_result, st_name)
+            'stx_atomic_write_unit_min',
-                self.assertEqual(type(x), type(b))
+            'stx_dio_mem_align',
-                if isinstance(x, float):
+            'stx_dio_offset_align',
-                    self.assertAlmostEqual(x, b, msg=name)
+            'stx_dio_read_offset_align',
+            'stx_mnt_id',
+            'stx_subvol',
+        }
+        float_type = {
+            'stx_atime',
+            'stx_btime',
+            'stx_ctime',
+            'stx_mtime',
+        }
+
+        members = set(name for name in dir(result)
+                      if name.startswith('stx_'))
+        tested = set(name for name, mask in requirements)
+        if members - tested:
+            raise ValueError(f"statx members not tested: {members - tested}")
+
+        for name, mask in requirements:
+            with self.subTest(name=name):
+                try:
+                    x = getattr(result, name)
+                except AttributeError:
+                    if name in optional_members:
+                        continue
+                    else:
+                        raise
+
+                if not(result.stx_mask & mask == mask):
+                    self.assertIsNone(x)
+                    continue
+
+                if name in float_type:
+                    self.assertIsInstance(x, float)
                 else:
-                    self.assertEqual(x, b, msg=name)
+                    self.assertIsInstance(x, int)
+
+                # Compare with stat_result
+                try:
+                    b = getattr(stat_result, "st_" + name[4:])
+                except AttributeError:
+                    pass
+                else:
+                    self.assertEqual(type(x), type(b))
+                    if isinstance(x, float):
+                        self.assertAlmostEqual(x, b)
+                    else:
+                        self.assertEqual(x, b)
 
         self.assertEqual(result.stx_rdev_major, os.major(result.stx_rdev))
         self.assertEqual(result.stx_rdev_minor, os.minor(result.stx_rdev))
         self.assertEqual(result.stx_dev_major, os.major(result.stx_dev))
         self.assertEqual(result.stx_dev_minor, os.minor(result.stx_dev))
 
-        members = [name for name in dir(result)
-                   if name.startswith('stx_')]
-        for name in members:
-            try:
-                setattr(result, name, 1)
-                self.fail("No exception raised")
-            except AttributeError:
-                pass
-
         self.assertEqual(result.stx_attributes & result.stx_attributes_mask,
                          result.stx_attributes)
 
-        # statx_result is not a tuple or tuple-like object.
-        with self.assertRaisesRegex(TypeError, 'not subscriptable'):
-            result[0]
-        with self.assertRaisesRegex(TypeError, 'cannot unpack'):
-            _, _ = result
-
     @unittest.skipUnless(hasattr(os, 'statx'), 'test needs os.statx()')
     def test_statx_attributes(self):
         self.check_statx_attributes(self.fname)
@@ -829,6 +875,27 @@ def test_statx_attributes_bytes(self):
     def test_statx_attributes_pathlike(self):
         self.check_statx_attributes(FakePath(self.fname))
 
+    @unittest.skipUnless(hasattr(os, 'statx'), 'test needs os.statx()')
+    def test_statx_result(self):
+        result = os.statx(self.fname, os.STATX_BASIC_STATS)
+
+        # Check that attributes are read-only
+        members = [name for name in dir(result)
+                   if name.startswith('stx_')]
+        for name in members:
+            try:
+                setattr(result, name, 1)
+            except AttributeError:
+                pass
+            else:
+                self.fail("No exception raised")
+
+        # statx_result is not a tuple or tuple-like object.
+        with self.assertRaisesRegex(TypeError, 'not subscriptable'):
+            result[0]
+        with self.assertRaisesRegex(TypeError, 'cannot unpack'):
+            _, _ = result
+
     @unittest.skipUnless(hasattr(os, 'statvfs'), 'test needs os.statvfs()')
     def test_statvfs_attributes(self):
         result = os.statvfs(self.fname)

Modules/posixmodule.c

@@ -3314,7 +3314,6 @@ os_lstat_impl(PyObject *module, path_t *path, int dir_fd)
 #ifdef HAVE_STATX
 typedef struct {
     PyObject_HEAD
-    double atime_sec, btime_sec, ctime_sec, mtime_sec;
     dev_t rdev, dev;
     struct statx stx;
 } Py_statx_result;
@@ -3332,7 +3331,6 @@ static PyMemberDef pystatx_result_members[] = {
     MM(stx_mask, Py_T_UINT, mask, "member validity mask"),
     MM(stx_blksize, Py_T_UINT, blksize, "blocksize for filesystem I/O"),
     MM(stx_attributes, Py_T_ULONGLONG, attributes, "Linux inode attribute bits"),
-    MM(stx_mode, Py_T_USHORT, mode, "protection bits"),
     MM(stx_attributes_mask, Py_T_ULONGLONG, attributes_mask,
         "Mask of supported bits in stx_attributes"),
     MM(stx_rdev_major, Py_T_UINT, rdev_major, "represented device major number"),
@@ -3381,6 +3379,17 @@ STATX_GET_UINT(stx_atomic_write_unit_max_opt, STATX_WRITE_ATOMIC)
 #endif
 
 
+static PyObject*
+pystatx_result_get_stx_mode(PyObject *op, void *Py_UNUSED(context))
+{
+    Py_statx_result *self = Py_statx_result_CAST(op);
+    if (!(self->stx.stx_mask & (STATX_TYPE | STATX_MODE))) {
+        Py_RETURN_NONE;
+    }
+    return PyLong_FromUnsignedLong(self->stx.stx_mode);
+}
+
+
 #define STATX_GET_ULONGLONG(ATTR, MASK) \
     static PyObject* \
     pystatx_result_get_##ATTR(PyObject *op, void *Py_UNUSED(context)) \
@@ -3404,7 +3413,7 @@ STATX_GET_ULONGLONG(stx_subvol, STATX_SUBVOL)
 #endif
 
 
-#define STATX_GET_DOUBLE(ATTR, MEMBER, MASK) \
+#define STATX_GET_DOUBLE(ATTR, MASK) \
     static PyObject* \
     pystatx_result_get_##ATTR(PyObject *op, void *Py_UNUSED(context)) \
     { \
@@ -3412,14 +3421,15 @@ STATX_GET_ULONGLONG(stx_subvol, STATX_SUBVOL)
         if (!(self->stx.stx_mask & MASK)) { \
             Py_RETURN_NONE; \
         } \
-        double sec = self->MEMBER; \
+        struct statx_timestamp *ts = &self->stx.ATTR; \
+        double sec = ((double)ts->tv_sec + ts->tv_nsec * 1e-9); \
         return PyFloat_FromDouble(sec); \
     }
 
-STATX_GET_DOUBLE(stx_atime, atime_sec, STATX_ATIME)
+STATX_GET_DOUBLE(stx_atime, STATX_ATIME)
-STATX_GET_DOUBLE(stx_btime, btime_sec, STATX_BTIME)
+STATX_GET_DOUBLE(stx_btime, STATX_BTIME)
-STATX_GET_DOUBLE(stx_ctime, ctime_sec, STATX_CTIME)
+STATX_GET_DOUBLE(stx_ctime, STATX_CTIME)
-STATX_GET_DOUBLE(stx_mtime, mtime_sec, STATX_MTIME)
+STATX_GET_DOUBLE(stx_mtime, STATX_MTIME)
 
 #define STATX_GET_NSEC(ATTR, MEMBER, MASK) \
     static PyObject* \
@@ -3444,6 +3454,7 @@ STATX_GET_NSEC(stx_mtime_ns, stx_mtime, STATX_MTIME)
     {#attr, pystatx_result_get_##attr, NULL, PyDoc_STR(doc), NULL}
 
 static PyGetSetDef pystatx_result_getset[] = {
+    G(stx_mode, "protection bits"),
     G(stx_nlink, "number of hard links"),
     G(stx_uid, "user ID of owner"),
     G(stx_gid, "group ID of owner"),
@@ -3670,14 +3681,6 @@ os_statx_impl(PyObject *module, path_t *path, unsigned int mask, int flags,
         return path_error(path);
     }
 
-    v->atime_sec = ((double)v->stx.stx_atime.tv_sec
-                    + 1e-9 * v->stx.stx_atime.tv_nsec);
-    v->btime_sec = ((double)v->stx.stx_btime.tv_sec
-                    + 1e-9 * v->stx.stx_btime.tv_nsec);
-    v->ctime_sec = ((double)v->stx.stx_ctime.tv_sec
-                    + 1e-9 * v->stx.stx_ctime.tv_nsec);
-    v->mtime_sec = ((double)v->stx.stx_mtime.tv_sec
-                    + 1e-9 * v->stx.stx_mtime.tv_nsec);
     v->rdev = makedev(v->stx.stx_rdev_major, v->stx.stx_rdev_minor);
     v->dev = makedev(v->stx.stx_dev_major, v->stx.stx_dev_minor);