[pacman-dev,1/5] Fix check for FSSTATSTYPE

Message ID 20210417034524.204755-2-mark.weiman@markzz.com
State New
Headers show
Series Various fixes for FreeBSD (and perhaps others) | expand

Commit Message

Mark Weiman April 17, 2021, 3:45 a.m. UTC
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>
---
 meson.build | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Allan McRae April 19, 2021, 7:01 a.m. UTC | #1
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(" ?

> -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 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.

Allan
Mark Weiman April 19, 2021, 2:25 p.m. UTC | #2
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
Allan McRae April 20, 2021, 2:26 a.m. UTC | #3
On 20/4/21 12:25 am, Mark Weiman wrote:
> 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.
> 

Thanks - it was suprisingly hard to find documentation for this...

>>> -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.
> 

Ah...  f_mntonname is only used with getmntinfo.  So BSD land.  I had
forgotten the pain of implementing this...

So now I am on the right page, lets go back and look at the patch!
Here is the bit I am looking at:


-if conf.has('HAVE_STRUCT_STATVFS_F_FLAG')
+if conf.get('HAVE_STRUCT_STATVFS_F_FLAG', false)
   conf.set('FSSTATSTYPE', 'struct statvfs')
-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


My reading is this is about equivalent to:

if conf.get('HAVE_STRUCT_STATVFS_F_MNTONNAME', false)
  conf.set('FSSTATSTYPE', 'struct statvfs')
elif conf.get('HAVE_STRUCT_STATFS_F_MNTONNAME', false)
  conf.set('FSSTATSTYPE', 'struct statfs')
elif conf.get('HAVE_STRUCT_STATVFS_F_FLAG', false)
  conf.set('FSSTATSTYPE', 'struct statvfs')
elif conf.get('HAVE_STRUCT_STATFS_F_FLAGS', false)
  conf.set('FSSTATSTYPE', 'struct statfs')
endif

Do we just fail on compile if FSSTATSTYPE is undefined?  That seems not
ideal.


This really does not correspond to the checks in the code!

HAVE_GETMNTENT  -> statvfs
HAVE_GETMNTINFO_STATVFS + HAVE_STRUCT_STATVFS_F_FLAG -> statvfs
HAVE_GETMNTINFO_STATFS + HAVE_STRUCT_STATFS_F_FLAGS -> statfs

I think we want to match the code (and fix any of the code that is wrong):

if conf.get('HAVE_GETMNTENT', false) and
conf.get('HAVE_STRUCT_STATVFS_F_FLAG', false)
  conf.set('FSSTATSTYPE', 'struct statvfs')
elif conf.get('HAVE_GETMNTINFO', false) and
conf.get('HAVE_STRUCT_STATVFS_F_MNTONNAME', false)
  conf.set('FSSTATSTYPE', 'struct statvfs')
elif conf.get('HAVE_GETMNTINFO', false) and
conf.get('HAVE_STRUCT_STATFS_F_MNTONNAME', false)
  conf.set('FSSTATSTYPE', 'struct statfs')
endif


Looks like the F_FLAG/F_FLAGS check within the getmntinfo are only to
see if we can detect read-only filesystems, and are not a requirement.
Mark Weiman April 20, 2021, 3:50 a.m. UTC | #4
On Tue, 2021-04-20 at 12:26 +1000, Allan McRae wrote:
> On 20/4/21 12:25 am, Mark Weiman wrote:
> > 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.
> > 
> 
> Thanks - it was suprisingly hard to find documentation for this...
> 
> > > > -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.
> > 
> 
> Ah...  f_mntonname is only used with getmntinfo.  So BSD land.  I had
> forgotten the pain of implementing this...
> 
> So now I am on the right page, lets go back and look at the patch!
> Here is the bit I am looking at:
> 
> 
> -if conf.has('HAVE_STRUCT_STATVFS_F_FLAG')
> +if conf.get('HAVE_STRUCT_STATVFS_F_FLAG', false)
>    conf.set('FSSTATSTYPE', 'struct statvfs')
> -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
> 
> 
> My reading is this is about equivalent to:
> 
> if conf.get('HAVE_STRUCT_STATVFS_F_MNTONNAME', false)
>   conf.set('FSSTATSTYPE', 'struct statvfs')
> elif conf.get('HAVE_STRUCT_STATFS_F_MNTONNAME', false)
>   conf.set('FSSTATSTYPE', 'struct statfs')
> elif conf.get('HAVE_STRUCT_STATVFS_F_FLAG', false)
>   conf.set('FSSTATSTYPE', 'struct statvfs')
> elif conf.get('HAVE_STRUCT_STATFS_F_FLAGS', false)
>   conf.set('FSSTATSTYPE', 'struct statfs')
> endif
> 
> Do we just fail on compile if FSSTATSTYPE is undefined?  That seems
> not
> ideal.
> 

Yes, FSSTATSTYPE is used in alpm_mountpoint_t
(lib/libalpm/diskspace.h). This *could* be remedied if needed.

> 
> This really does not correspond to the checks in the code!
> 
> HAVE_GETMNTENT  -> statvfs
> HAVE_GETMNTINFO_STATVFS + HAVE_STRUCT_STATVFS_F_FLAG -> statvfs
> HAVE_GETMNTINFO_STATFS + HAVE_STRUCT_STATFS_F_FLAGS -> statfs
> 
> I think we want to match the code (and fix any of the code that is
> wrong):
> 
> if conf.get('HAVE_GETMNTENT', false) and
> conf.get('HAVE_STRUCT_STATVFS_F_FLAG', false)
>   conf.set('FSSTATSTYPE', 'struct statvfs')
> elif conf.get('HAVE_GETMNTINFO', false) and
> conf.get('HAVE_STRUCT_STATVFS_F_MNTONNAME', false)
>   conf.set('FSSTATSTYPE', 'struct statvfs')
> elif conf.get('HAVE_GETMNTINFO', false) and
> conf.get('HAVE_STRUCT_STATFS_F_MNTONNAME', false)
>   conf.set('FSSTATSTYPE', 'struct statfs')
> endif
> 

This is likely what would be appropriate, but perhaps with an else or
some preprocessor check for alpm_mountpoint_t to prevent a compilation
error if nothing in that block applies.

> 
> Looks like the F_FLAG/F_FLAGS check within the getmntinfo are only to
> see if we can detect read-only filesystems, and are not a
> requirement.

I can submit a v3 patch for this if you'd like. I wasn't entirely happy
with the patches I provided since they felt "hacky" to me, but they did
work on both FreeBSD and Arch Linux, so I felt it was at least a start.

Mark
Allan McRae April 20, 2021, 4:30 a.m. UTC | #5
On 20/4/21 1:50 pm, Mark Weiman wrote:
> I can submit a v3 patch for this if you'd like. I wasn't entirely happy
> with the patches I provided since they felt "hacky" to me, but they did
> work on both FreeBSD and Arch Linux, so I felt it was at least a start.

That would be great.  I think this one was too hacky to put in as is.

Allan

Patch

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>'''],
   ]
   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')
-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