On Mon, 2021-04-19 at 17:01 +1000, Allan McRae wrote:
On 17/4/21 1:45 pm, Mark Weiman wrote:
On FreeBSD, both `struct statvfs` and `struct statfs` satisfy the conditions where the `f_flag` and `f_flags` fields are present in both respectively.
This patch accomplishes a couple of things:
1. Adds a check for `f_mntonname` in both structs and makes a decision to choose statfs if the field is not present in statvfs, but is in statfs. 2. Corrects a small error where the values of those checks are just checked for their presence and not whether its true or false.
Signed-off-by: Mark Weiman <mark.weiman@markzz.com>
Thanks for the BSD fixes! It is clear that we do not test there very often...
--- meson.build | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/meson.build b/meson.build index 579ff2ed..483c4fbd 100644 --- a/meson.build +++ b/meson.build @@ -154,6 +154,9 @@ foreach member : [ ['struct statvfs', 'f_flag', '''#include <sys/statvfs.h>'''], ['struct statfs', 'f_flags', '''#include <sys/param.h> #include <sys/mount.h>'''], + ['struct statvfs', 'f_mntonname', '''#include <sys/statvfs.h>'''], + ['struct statfs', 'f_mntonname', '''#include <sys/param.h> + #include <sys/mount.h>'''],
OK
] have = cc.has_member(member[0], member[1], prefix : member[2]) conf.set('HAVE_' + '_'.join([member[0], member[1]]).underscorify().to_upper(), have) @@ -174,9 +177,13 @@ foreach type : [ endif endforeach -if conf.has('HAVE_STRUCT_STATVFS_F_FLAG') +if conf.get('HAVE_STRUCT_STATVFS_F_FLAG', false) conf.set('FSSTATSTYPE', 'struct statvfs')
Should this be "if conf.get( ... true)", or "if not conf.get(" ?
No, the singature to conf.get() is the key you're checking first, then the default value you want if it doesn't exist. In this case, we want false to be the default.
-elif conf.has('HAVE_STRUCT_STATFS_F_FLAGS') +elif conf.get('HAVE_STRUCT_STATFS_F_FLAGS', false) + conf.set('FSSTATSTYPE', 'struct statfs') +endif + +if not conf.get('HAVE_STRUCT_STATVFS_F_MNTONNAME', false) and conf.get('HAVE_STRUCT_STATFS_F_MNTONNAME', false) conf.set('FSSTATSTYPE', 'struct statfs') endif
This just seems wrong to me. We require f_mntonname for either statfs or statvfs, so just test for this when making the original assignment to FSSTATSTYPE. It appears the f_flag(s) part is not actually useful in determining which of statfs or statvfs we should use.
(As an aside, is there some statvfs without the f_mntonname field?)
I didn't quite know how to address this. On Arch Linux, it appears that f_mntonname doesn't exist at all, but on FreeBSD, it exists in statfs, but not statvfs. And yes, statvfs exists without f_mntonname. I figured getting it to build and choosing the correct one on both Linux and FreeBSD was what was important here and doing some homework later on what exactly to do. On Linux, its statvfs (I think), but on FreeBSD, that's statfs.
I also found this looking at the mountpoint code. It appears that the f_flag(s) only gives us access to whether the mountpoint is read- only. That also appears broken since the switch to meson (or earlier!):
e.g. lib/libalpm/diskspace.c;
#if defined(HAVE_GETMNTINFO_STATVFS) && defined(HAVE_STRUCT_STATVFS_F_FLAG)
I don't see that first define being made anywhere.
I'll look at it further later.
Allan
Mark