Make sure that `statvfs` is used correctly

[sobolevn]

Bug report

While working on #116542 @serhiy-storchaka noticed that

Also, on Linux some of fields are defined as unsigned long instead of long. Did not check other Posix systems. There may be a problem, but this is another issue.

Here's how it is documented:

struct statvfs {
               unsigned long  f_bsize;    /* Filesystem block size */
               unsigned long  f_frsize;   /* Fragment size */
               fsblkcnt_t     f_blocks;   /* Size of fs in f_frsize units */
               fsblkcnt_t     f_bfree;    /* Number of free blocks */
               fsblkcnt_t     f_bavail;   /* Number of free blocks for
                                             unprivileged users */
               fsfilcnt_t     f_files;    /* Number of inodes */
               fsfilcnt_t     f_ffree;    /* Number of free inodes */
               fsfilcnt_t     f_favail;   /* Number of free inodes for
                                             unprivileged users */
               unsigned long  f_fsid;     /* Filesystem ID */
               unsigned long  f_flag;     /* Mount flags */
               unsigned long  f_namemax;  /* Maximum filename length */
           };

Here's how it is used:

cpython/Modules/posixmodule.c

Lines 12983 to 13011 in d308d33

	#if !defined(HAVE_LARGEFILE_SUPPORT)
	PyStructSequence_SET_ITEM(v, 0, PyLong_FromLong((long) st.f_bsize));
	PyStructSequence_SET_ITEM(v, 1, PyLong_FromLong((long) st.f_frsize));
	PyStructSequence_SET_ITEM(v, 2, PyLong_FromLong((long) st.f_blocks));
	PyStructSequence_SET_ITEM(v, 3, PyLong_FromLong((long) st.f_bfree));
	PyStructSequence_SET_ITEM(v, 4, PyLong_FromLong((long) st.f_bavail));
	PyStructSequence_SET_ITEM(v, 5, PyLong_FromLong((long) st.f_files));
	PyStructSequence_SET_ITEM(v, 6, PyLong_FromLong((long) st.f_ffree));
	PyStructSequence_SET_ITEM(v, 7, PyLong_FromLong((long) st.f_favail));
	PyStructSequence_SET_ITEM(v, 8, PyLong_FromLong((long) st.f_flag));
	PyStructSequence_SET_ITEM(v, 9, PyLong_FromLong((long) st.f_namemax));
	#else
	PyStructSequence_SET_ITEM(v, 0, PyLong_FromLong((long) st.f_bsize));
	PyStructSequence_SET_ITEM(v, 1, PyLong_FromLong((long) st.f_frsize));
	PyStructSequence_SET_ITEM(v, 2,
	PyLong_FromLongLong((long long) st.f_blocks));
	PyStructSequence_SET_ITEM(v, 3,
	PyLong_FromLongLong((long long) st.f_bfree));
	PyStructSequence_SET_ITEM(v, 4,
	PyLong_FromLongLong((long long) st.f_bavail));
	PyStructSequence_SET_ITEM(v, 5,
	PyLong_FromLongLong((long long) st.f_files));
	PyStructSequence_SET_ITEM(v, 6,
	PyLong_FromLongLong((long long) st.f_ffree));
	PyStructSequence_SET_ITEM(v, 7,
	PyLong_FromLongLong((long long) st.f_favail));
	PyStructSequence_SET_ITEM(v, 8, PyLong_FromLong((long) st.f_flag));
	PyStructSequence_SET_ITEM(v, 9, PyLong_FromLong((long) st.f_namemax));
	#endif

Notice that f_bsize is documented to be unsigned long, but it treated as long:

cpython/Modules/posixmodule.c

Line 12995 in d308d33

PyStructSequence_SET_ITEM(v, 0, PyLong_FromLong((long) st.f_bsize));

This might be a problem.

Here' git blame for these lines:

» git blame -L 12983,+30 -- Modules/posixmodule.c
Blaming lines: 100% (30/30), done.
98bf58f1c61a (Guido van Rossum  2001-10-18 20:34:25 +0000 12983) #if !defined(HAVE_LARGEFILE_SUPPORT)
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 12984)     PyStructSequence_SET_ITEM(v, 0, PyLong_FromLong((long) st.f_bsize));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 12985)     PyStructSequence_SET_ITEM(v, 1, PyLong_FromLong((long) st.f_frsize));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 12986)     PyStructSequence_SET_ITEM(v, 2, PyLong_FromLong((long) st.f_blocks));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 12987)     PyStructSequence_SET_ITEM(v, 3, PyLong_FromLong((long) st.f_bfree));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 12988)     PyStructSequence_SET_ITEM(v, 4, PyLong_FromLong((long) st.f_bavail));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 12989)     PyStructSequence_SET_ITEM(v, 5, PyLong_FromLong((long) st.f_files));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 12990)     PyStructSequence_SET_ITEM(v, 6, PyLong_FromLong((long) st.f_ffree));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 12991)     PyStructSequence_SET_ITEM(v, 7, PyLong_FromLong((long) st.f_favail));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 12992)     PyStructSequence_SET_ITEM(v, 8, PyLong_FromLong((long) st.f_flag));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 12993)     PyStructSequence_SET_ITEM(v, 9, PyLong_FromLong((long) st.f_namemax));
98bf58f1c61a (Guido van Rossum  2001-10-18 20:34:25 +0000 12994) #else
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 12995)     PyStructSequence_SET_ITEM(v, 0, PyLong_FromLong((long) st.f_bsize));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 12996)     PyStructSequence_SET_ITEM(v, 1, PyLong_FromLong((long) st.f_frsize));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 12997)     PyStructSequence_SET_I
TEM(v, 2,
af580dff4af3 (Benjamin Peterson 2016-09-06 10:46:49 -0700 12998)                           
    PyLong_FromLongLong((long long) st.f_blocks));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 12999)     PyStructSequence_SET_I
TEM(v, 3,
af580dff4af3 (Benjamin Peterson 2016-09-06 10:46:49 -0700 13000)                           
    PyLong_FromLongLong((long long) st.f_bfree));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 13001)     PyStructSequence_SET_I
TEM(v, 4,
af580dff4af3 (Benjamin Peterson 2016-09-06 10:46:49 -0700 13002)                           
    PyLong_FromLongLong((long long) st.f_bavail));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 13003)     PyStructSequence_SET_I
TEM(v, 5,
af580dff4af3 (Benjamin Peterson 2016-09-06 10:46:49 -0700 13004)                           
    PyLong_FromLongLong((long long) st.f_files));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 13005)     PyStructSequence_SET_I
TEM(v, 6,
af580dff4af3 (Benjamin Peterson 2016-09-06 10:46:49 -0700 13006)                           
    PyLong_FromLongLong((long long) st.f_ffree));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 13007)     PyStructSequence_SET_I
TEM(v, 7,
af580dff4af3 (Benjamin Peterson 2016-09-06 10:46:49 -0700 13008)                           
    PyLong_FromLongLong((long long) st.f_favail));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 13009)     PyStructSequence_SET_I
TEM(v, 8, PyLong_FromLong((long) st.f_flag));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 13010)     PyStructSequence_SET_I
TEM(v, 9, PyLong_FromLong((long) st.f_namemax));
8c62be88e6d5 (Victor Stinner    2010-05-06 00:08:46 +0000 13011) #endif
502d551c6d78 (Michael Felt      2018-01-05 13:01:58 +0100 13012) /* The _ALL_SOURCE feature
 test macro defines f_fsid as a structure

On the other hand, seems like this code was there for a very long time, with seemingly no issues. Maybe there are some platform differences that we need to remember about.

[serhiy-storchaka]

Perhaps no real system (at least on which Python was tested) has values > LONG_MAX.

We should look how these fields are defined in Posix and on other concrete Posix systems: macOS, FreeBSD, NetBSD, OpenBSD, AIX, Solaris, other commercial Unixes.

[serhiy-storchaka]

In Posix, the statvfs structure includes at least the following members:

unsigned long f_bsize    File system block size. 
unsigned long f_frsize   Fundamental file system block size. 
fsblkcnt_t    f_blocks   Total number of blocks on file system in units of f_frsize. 
fsblkcnt_t    f_bfree    Total number of free blocks. 
fsblkcnt_t    f_bavail   Number of free blocks available to 
                         non-privileged process. 
fsfilcnt_t    f_files    Total number of file serial numbers. 
fsfilcnt_t    f_ffree    Total number of free file serial numbers. 
fsfilcnt_t    f_favail   Number of file serial numbers available to 
                         non-privileged process. 
unsigned long f_fsid     File system ID. 
unsigned long f_flag     Bit mask of f_flag values. 
unsigned long f_namemax  Maximum filename length. 

On Solaris it includes the following members:

u_long      f_bsize;             /* preferred file system block size */
u_long      f_frsize;            /* fundamental filesystem block
                                    (size if supported) */
fsblkcnt_t  f_blocks;            /* total # of blocks on file system
                                    in units of f_frsize */
fsblkcnt_t  f_bfree;             /* total # of free blocks */
fsblkcnt_t  f_bavail;            /* # of free blocks avail to
                                    non-privileged user */
fsfilcnt_t  f_files;             /* total # of file nodes (inodes) */
fsfilcnt_t  f_ffree;             /* total # of free file nodes */
fsfilcnt_t  f_favail;            /* # of inodes avail to
                                    non-privileged user*/
u_long      f_fsid;              /* file system id (dev for now) */
char        f_basetype[FSTYPSZ]; /* target fs type name, 
                                    null-terminated */
u_long      f_flag;              /* bit mask of flags */
u_long      f_namemax;           /* maximum file name length */
char        f_fstr[32];          /* file system specific string */
u_long      f_filler[16];        /* reserved for future expansion */

On OpenBSD it is defined as follows:

struct statvfs {
        unsigned long f_bsize;    /* file system block size */
        unsigned long f_frsize;   /* fundamental file system block size */
        fsblkcnt_t    f_blocks;   /* number of blocks (unit f_frsize) */
        fsblkcnt_t    f_bfree;    /* free blocks in file system */
        fsblkcnt_t    f_bavail;   /* free blocks for non-root */
        fsfilcnt_t    f_files;    /* total file inodes */
        fsfilcnt_t    f_ffree;    /* free file inodes */
        fsfilcnt_t    f_favail;   /* free file inodes for non-root */
        unsigned long f_fsid;     /* file system id */
        unsigned long f_flag;     /* bit mask of f_flag values */
        unsigned long f_namemax;  /* maximum filename length */
};

On macOS, FreeBSD and NetBSD they also should conform Posix (there are no tables in the man pages).

fsblkcnt_t and fsfilcnt_t are unsigned 64-bit integers on Linux. I think we should use unsigned integer converters for all fields.

[serhiy-storchaka]

On macOS, Python's implementation for os.statvfs() uses statfs() instead of statvfs(). It has different fields, mostly signed integers:

struct statfs {
         short   f_otype;    /* type of file system (reserved: zero) */
         short   f_oflags;   /* copy of mount flags (reserved: zero) */
         long    f_bsize;    /* fundamental file system block size */
         long    f_iosize;   /* optimal transfer block size */
         long    f_blocks;   /* total data blocks in file system */
         long    f_bfree;    /* free blocks in fs */
         long    f_bavail;   /* free blocks avail to non-superuser */
         long    f_files;    /* total file nodes in file system */
         long    f_ffree;    /* free file nodes in fs */
         fsid_t  f_fsid;     /* file system id */
         uid_t   f_owner;    /* user that mounted the file system */
         short   f_reserved1;        /* reserved for future use */
         short   f_type;     /* type of file system (reserved) */
         long    f_flags;    /* copy of mount flags (reserved) */
         long    f_reserved2[2];     /* reserved for future use */
         char    f_fstypename[MFSNAMELEN]; /* fs type name */
         char    f_mntonname[MNAMELEN];    /* directory on which mounted */
         char    f_mntfromname[MNAMELEN];  /* mounted file system */
         char    f_reserved3;        /* reserved for future use */
         long    f_reserved4[4];     /* reserved for future use */
     };

Furthermore, fields that are undefined for a particular file system are set to -1, so it is important to use signed integer conversion. If such convention is also used on other platforms (I have not found this in the documentation), using an unsigned converter can break Python code that tests for -1 but get 4294967295 or 18446744073709551615.