Message-ID: <f536ecc2-1889-6df6-43d3-1a04dc8f1b14@linuxfoundation.org>
Date:   Tue, 14 Feb 2023 10:10:00 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.1
Subject: Re: [PATCH] selftests/mount_setattr: fix redefine struct mount_attr
 build error
To:     Seth Forshee <sforshee@kernel.org>
Cc:     shuah@kernel.org, brauner@kernel.org,
        linux-fsdevel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Shuah Khan <skhan@linuxfoundation.org>
References: <20230213183149.231779-1-skhan@linuxfoundation.org>
 <Y+rMtlvx31w7eWCA@do-x1extreme>
Content-Language: en-US
From:   Shuah Khan <skhan@linuxfoundation.org>
In-Reply-To: <Y+rMtlvx31w7eWCA@do-x1extreme>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk

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