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

[Allan McRae]

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