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

[Shuah Khan <skhan@linuxfoundation.org>]

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@linuxfoundation.org>
>> ---
>>   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