-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
NetBSD: fix getmntinfo for NetBSD #4265
Conversation
This should be pulled up, so |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me. Our statvfs
is actually statvfs90
then? (Based on the call here https://github.com/NetBSD/src/blob/ea69897761f4bc39da7326529c06cc55f32a6d1c/lib/libc/compat/gen/compat___getmntinfo13.c#L52-L58)
Exactly. We could switch to the new statvfs with the one additional field, but it's a bigger change and we'd need to differentiate between NetBSD 9 and 10+, so i'd leave that for another time. |
You should open an issue for changing the struct, that is probably something we could do before 1.0. |
(backport <rust-lang#4265>) (cherry picked from commit e1e9d97)
I've filed a pull request for this, which works on NetBSD 10, but I need some guidance on how to best support NetBSD 9. Thanks! |
Description
This patch fixes libc::getmntinfo on NetBSD.
Sources
The problem is that NetBSD provides binary compatibility in a way that was not considered when initially providing the implementation.
Once a symbol name has been used, it stays the same, so the original
getmntinfo()
in NetBSD, usingstruct statfs
, will always be available asgetmntinfo
. Whengetmntinfo
was changed to providestruct statvfs
(note the 'v') a long time ago, a new symbol was introduced,__getmntinfo13
and the system headers rename access togetmntinfo
to call__getmntinfo13
instead.The rust implementation called the old symbol (providing
struct statfs
) but parsed the returned information asstruct statvfs
, leading to wrong data and segfaults.Checklist
The change has been tested on NetBSD 9, 10, and -current (all supported versions).
No compatibility for NetBSD 8 or older is required, since these versions are not supported any longer; the symbol rename happened before NetBSD 9.