Re: getmntinfo compatibility question

[Havard Eidnes <he%NetBSD.org@localhost>]

> On Fri, Feb 14, 2025 at 09:22:54AM +0100, Thomas Klausner wrote:
>> Perhaps we need to add code like FreeBSD has (to rust libc):
>> https://github.com/rust-lang/libc/blob/2258bf0fb96767bcffbe3ed09b29a31ee54b549b/src/unix/bsd/freebsdlike/freebsd/mod.rs#L5340
>
> Your rust binary calls __getvfsstat90 (according to ktrace) [...]

That's strange.  I'd like to understand why.  I was living under
the assumption that the rust code didn't use the sys/statvfs.h
header file, so would not be exposed to or use the effect this
__RENAME:

> int     getvfsstat(struct statvfs *, size_t, int)
>     __RENAME(__getvfsstat90);

which would cause __getvfsstat90 to be called.

Inside the rust source code, the file
vendor/libc-0.2.164/src/unix/bsd/netbsdlike/netbsd/mod.rs contains

    pub struct statvfs {
        pub f_flag: ::c_ulong,
        pub f_bsize: ::c_ulong,
        pub f_frsize: ::c_ulong,
        pub f_iosize: ::c_ulong,

        pub f_blocks: ::fsblkcnt_t,
        pub f_bfree: ::fsblkcnt_t,
        pub f_bavail: ::fsblkcnt_t,
        pub f_bresvd: ::fsblkcnt_t,

        pub f_files: ::fsfilcnt_t,
        pub f_ffree: ::fsfilcnt_t,
        pub f_favail: ::fsfilcnt_t,
        pub f_fresvd: ::fsfilcnt_t,

        pub f_syncreads: u64,
        pub f_syncwrites: u64,

        pub f_asyncreads: u64,
        pub f_asyncwrites: u64,

        pub f_fsidx: ::fsid_t,
        pub f_fsid: ::c_ulong,
        pub f_namemax: ::c_ulong,
        pub f_owner: ::uid_t,

        pub f_spare: [u32; 4],

        pub f_fstypename: [::c_char; 32],
        pub f_mntonname: [::c_char; 1024],
        pub f_mntfromname: [::c_char; 1024],
    }

and

        extern {
            // these functions use statvfs:
            pub fn getmntinfo(mntbufp: *mut *mut ::statvfs, flags: ::c_int) -> ::c_int;
            pub fn getvfsstat(buf: *mut statvfs, bufsize: ::size_t, flags: ::c_int) -> ::c_int;
        }

How that ends up calling __getvfsstat90() I must admit that I do not
(at the time I wrote this sentence ... more below) understand.

Then there is the comprehension of what happens in the C source
code when a "system call rename" happens.  The starting process
for that is usually that a given "non-renamed" syscall needs a
new ABI.  It was my understanding that the original-ABI syscall
continues to have the non-renamed name, and would be called via
the above function declaration of getmntinfo().  However, it is
this old-ABI which gets the specially-named arguments?  And our
header files do not define e.g. struct statvfs90, since it's only
referred to via "struct statvfs90*", so "a pointer", so doesn't
really need the actual struct layout defined?  So if you need
that struct layout, you need to do "time travel" via CVS?

Now, getmntinfo() is a library function and not a syscall, so
that may make a difference.  I would expect this one to be
called:

$ nm -op /usr/lib/libc.a | grep getmntinfo
...
/usr/lib/libc.a:compat_getmntinfo.o:0000000000000000 T getmntinfo
...

which corresponds with lib/libc/compat/gen/compat_getmntinfo.c

and it in turn calls __compat_getfsstat(), and is aliased into
getmntinfo() via

__strong_alias(getmntinfo, __compat_getmntinfo)

So that on the surface of it looks sort of sane?

__compat_getfsstat() is implemented in
lib/libc/compat/sys/compat_statfs.c and ... calls __getvfsstat90(),
and then "converts" the results.  But if __getvfsstat90() expects
the caller to allocate more space than what the old ABI dictated
(the buffer appears to be caller-supplied), I don't quite see how
that will work well (and I've not looked at the
statvfs_to_statfs12() function).


Now, backing off that for a bit, and returning to the original
question, the "90" suffix used with statvfs*() appears to me to
indicate that this rename was done in the era leading up to the
NetBSD 9.0 release.  Since we do not support any older release than
9.0 (and barely that...) with rust, we should perhaps take steps to
adopt the new ABI instead.  This means the struct above needs to be
amended/extended and we need to ensure that we call the right
version of the libc function.

And ... since the rust compiler distribution has multiple versions
of the libc crate embedded, we need to figure out how many of them
need to be fixed, and then we need to get the rust folks to adopt
the modification, so that we eventually down the road can stop
maintaining that as a diff in our package.


Regards,

- Håvard