Re: [LTP] [PATCH] configure.ac: Fix redefinition of mount_attr by checking sys/mount.h

[Petr Vorel <pvorel@suse.cz>]

Hi Marcos,

...
> > > +++ b/configure.ac
> > > @@ -71,6 +71,7 @@ AC_CHECK_HEADERS_ONCE([ \
> > >      sys/epoll.h \
> > >      sys/fanotify.h \
> > >      sys/inotify.h \
> > > +    sys/mount.h \

> > Well, we don't add check for this, as sys/mount.h has been here since "ever", so
> > it'd just make configure little bit slower for no gain.

> > But you'd have to use somewhere compiler macro #ifdef HAVE_SYS_MOUNT_H, as this
> > macro is generated by configure script. Adding header to the list does nothing.

> It wasn't needed in my case, let me explain better below.
Interesting, maybe I'm missing something.

> > You obviously wanted to fix something, could you post the problem you're trying
> > to solve + steps you did to ML?

> I was seeing redefinition problems:

> In file included from fsconfig01.c:9:
> ../../../../include/lapi/fsmount.h:55:8: error: redefinition of 'struct mount_attr'
>    55 | struct mount_attr {
>       |        ^~~~~~~~~~
> In file included from /usr/include/sys/mount.h:32,
>                  from ../../../../include/lapi/fsmount.h:14:
> /usr/include/linux/mount.h:129:8: note: originally defined here
>   129 | struct mount_attr {
>       |        ^~~~~~~~~~
> ../../../../include/lapi/fsmount.h:110:19: error: conflicting types for 'mount_setattr'; have 'int(int,  const char *, unsigned int,  struct mount_attr *, size_t)' {aka 'int(int,  const char *, unsigned int,  struct mount_attr *, long unsigned int)'}
>   110 | static inline int mount_setattr(int dirfd, const char *from_pathname, unsigned int flags,
>       |                   ^~~~~~~~~~~~~
> In file included from fsconfig02.c:8:
> ../../../../include/lapi/fsmount.h:55:8: error: redefinition of 'struct mount_attr'
>    55 | struct mount_attr {
>       |        ^~~~~~~~~~
> /usr/include/sys/mount.h:316:12: note: previous declaration of 'mount_setattr' with type 'int(int,  const char *, unsigned int,  struct mount_attr *, size_t)' {aka 'int(int,  const char *, unsigned int,  struct mount_attr *, long unsigned int)'}
>   316 | extern int mount_setattr (int __dfd, const char *__path, unsigned int __flags,
>       |            ^~~~~~~~~~~~~
> In file included from /usr/include/sys/mount.h:32,
>                  from ../../../../include/lapi/fsmount.h:14:
> /usr/include/linux/mount.h:129:8: note: originally defined here
>   129 | struct mount_attr {
>       |        ^~~~~~~~~~
> ../../../../include/lapi/fsmount.h:110:19: error: conflicting types for 'mount_setattr'; have 'int(int,  const char *, unsigned int,  struct mount_attr *, size_t)' {aka 'int(int,  const char *, unsigned int,  struct mount_attr *, long unsigned int)'}
>   110 | static inline int mount_setattr(int dirfd, const char *from_pathname, unsigned int flags,
>       |                   ^~~~~~~~~~~~~
> /usr/include/sys/mount.h:316:12: note: previous declaration of 'mount_setattr' with type 'int(int,  const char *, unsigned int,  struct mount_attr *, size_t)' {aka 'int(int,  const char *, unsigned int,  struct mount_attr *, long unsigned int)'}
>   316 | extern int mount_setattr (int __dfd, const char *__path, unsigned int __flags,
>       |            ^~~~~~~~~~~~~
> make[4]: *** [../../../../include/mk/rules.mk:45: fsconfig01] Error 1
> make[4]: *** Waiting for unfinished jobs....
> make[4]: *** [../../../../include/mk/rules.mk:45: fsconfig02] Error 1
> make[3]: *** [../../../include/mk/generic_trunk_target.inc:108: all] Error 2
> make[2]: *** [../../include/mk/generic_trunk_target.inc:108: all] Error 2
> make[1]: *** [../include/mk/generic_trunk_target.inc:108: all] Error 2
> make[1]: Leaving directory '/home/mpdesouza/git/ltp/testcases'
> make: *** [Makefile:106: testcases-all] Error 2

> In my TW system I found that the struct mount_attr was being defined in
> sys/mount.h. Checking include/lapi/fsmount.h I found some ifdefs:

> ...
> #ifndef HAVE_STRUCT_MOUNT_ATTR
> /*
>  * mount_setattr()
>  */
> struct mount_attr {
>         uint64_t attr_set;
>         uint64_t attr_clr;
>         uint64_t propagation;
>         uint64_t userns_fd;
> };
> #endif
> ...

> #ifndef HAVE_MOUNT_SETATTR
> static inline int mount_setattr(int dirfd, const char *from_pathname, unsigned int flags,
>                                 struct mount_attr *attr, size_t size)
> {
>         return tst_syscall(__NR_mount_setattr, dirfd, from_pathname, flags,
>                            attr, size);
> }
> #endif /* HAVE_MOUNT_SETATTR */

> So, only adding the sys/mount.h entry to the headers list is enough to make
> autoconf to find that sys/mount.h defines the same struct and function, fixing
> the issue.
Hm, I'm likely missing something :).

$ rpm -q glibc
glibc-2.36-5.1.x86_64

$ grep -B2 '^struct mount_attr' /usr/include/sys/mount.h
#ifndef MOUNT_ATTR_SIZE_VER0
/* For mount_setattr.  */
struct mount_attr

=> I'm not sure who/where is supposed to define MOUNT_ATTR_SIZE_VER0.
Is it in your config?
Do you have different config than mine?

grep MOUNT include/config.h
#define HAVE_FSMOUNT 1
#define HAVE_LINUX_MOUNT_H 1
#define HAVE_MOUNT_SETATTR 1
#define HAVE_MOVE_MOUNT 1
#define HAVE_STRUCT_MOUNT_ATTR 1
(i.e. HAVE_STRUCT_MOUNT_ATTR found with code in current master)

I also tested fedora rawhide (glibc 2.36.9000), which we don't have in our CI:
https://github.com/pevik/ltp/actions/runs/3133529596
Works well.

> > BTW I have no problem to build locally on the latest Tumbleweed. Also LTP
> > package build for Tumbleweed is also fine:
> > https://build.opensuse.org/package/binaries/benchmark:ltp:devel/ltp/openSUSE_Factory
> > https://build.opensuse.org/package/binaries/benchmark:ltp:devel/ltp/openSUSE_Tumbleweed

> Interesting... so is there anything wrong with my setup? Am I missing something?

Hard to tell. Could you share your commands?
I suppose it's the usual: use gcc and in the root directory:
make autotools && ./configure && make -j`nproc`

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp