Re: [PATCH] selftests/mount_setattr: fix redefine struct mount_attr build error

[Shuah Khan]

On 2/13/23 16:50, Seth Forshee wrote:
> On Mon, Feb 13, 2023 at 11:31:49AM -0700, Shuah Khan wrote:
> > Fix the following build error due to redefining struct mount_attr by
> > removing duplicate define from mount_setattr_test.c
> >
> > gcc -g -isystem .../tools/testing/selftests/../../../usr/include -Wall -O2 -pthread     mount_setattr_test.c  -o .../tools/testing/selftests/mount_setattr/mount_setattr_test
> > mount_setattr_test.c:107:8: error: redefinition of ‘struct mount_attr’
> >    107 | struct mount_attr {
> >        |        ^~~~~~~~~~
> > In file included from /usr/include/x86_64-linux-gnu/sys/mount.h:32,
> >                   from mount_setattr_test.c:10:
> > .../usr/include/linux/mount.h:129:8: note: originally defined here
> >    129 | struct mount_attr {
> >        |        ^~~~~~~~~~
> > make: *** [../lib.mk:145: .../tools/testing/selftests/mount_setattr/mount_setattr_test] Error 1
> >
> > Signed-off-by: Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx>
> > ---
> >   tools/testing/selftests/mount_setattr/mount_setattr_test.c | 7 -------
> >   1 file changed, 7 deletions(-)
> >
> > diff --git a/tools/testing/selftests/mount_setattr/mount_setattr_test.c b/tools/testing/selftests/mount_setattr/mount_setattr_test.c
> > index 8c5fea68ae67..582669ca38e9 100644
> > --- a/tools/testing/selftests/mount_setattr/mount_setattr_test.c
> > +++ b/tools/testing/selftests/mount_setattr/mount_setattr_test.c
> > @@ -103,13 +103,6 @@
> >   	#else
> >   		#define __NR_mount_setattr 442
> >   	#endif
> > -
> > -struct mount_attr {
> > -	__u64 attr_set;
> > -	__u64 attr_clr;
> > -	__u64 propagation;
> > -	__u64 userns_fd;
> > -};
> >   #endif
>
> The difficulty with this is that whether or not you need this definition
> depends on your system headers. My laptop doesn't have definitions for
> either __NR_mount_setattr or struct mount_attr, so for me the build
> works without this patch but fails with it.

The header search looks up system headers followed by installed headers in
the repo (both in-tree and out-of-tree builds). kselftest builds do depend
on headers_install. Did you building after running headers_install?

> I suppose we could fix this universally by using a different name for
> the struct in the test, e.g.:

This is not a good way to for a couple of reasons. This masks any problems
due to incompatibility between these defines.

thanks,
-- Shuah