[PATCH] selftests/landlock:Fix two build issues

[PATCH] selftests/landlock:Fix two build issues

[Hu Yadi]

From: "Hu.Yadi" <hu.yadi@h3c.com>

Two issues comes up  while building selftest/landlock:

the first one is as to gettid

net_test.c: In function ‘set_service’:
net_test.c:91:45: warning: implicit declaration of function ‘gettid’; did you mean ‘getgid’? [-Wimplicit-function-declaration]
    "_selftests-landlock-net-tid%d-index%d", gettid(),
                                             ^~~~~~
                                             getgid
net_test.c:(.text+0x4e0): undefined reference to `gettid'

the second is compiler error
gcc -Wall -O2 -isystem   fs_test.c -lcap -o /home/linux/tools/testing/selftests/landlock/fs_test
fs_test.c:4575:9: error: initializer element is not constant
  .mnt = mnt_tmp,
         ^~~~~~~

this patch is to fix them

Signed-off-by: Hu.Yadi <hu.yadi@h3c.com>
Suggested-by: Jiao <jiaoxupo@h3c.com>
Reviewed-by:Berlin <berlin@h3c.com>
---
 tools/testing/selftests/landlock/fs_test.c  | 5 ++++-
 tools/testing/selftests/landlock/net_test.c | 3 +--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 18e1f86a6234..93eb40a09073 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -4572,7 +4572,10 @@ FIXTURE_VARIANT(layout3_fs)
 /* clang-format off */
 FIXTURE_VARIANT_ADD(layout3_fs, tmpfs) {
 	/* clang-format on */
-	.mnt = mnt_tmp,
+	.mnt = {
+		.type = "tmpfs",
+        	.data = "size=4m,mode=700",
+	},
 	.file_path = file1_s1d1,
 };
 
diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
index 929e21c4db05..8fb357de8c55 100644
--- a/tools/testing/selftests/landlock/net_test.c
+++ b/tools/testing/selftests/landlock/net_test.c
@@ -18,7 +18,6 @@
 #include <sys/prctl.h>
 #include <sys/socket.h>
 #include <sys/un.h>
-
 #include "common.h"
 
 const short sock_port_start = (1 << 10);
@@ -88,7 +87,7 @@ static int set_service(struct service_fixture *const srv,
 	case AF_UNIX:
 		srv->unix_addr.sun_family = prot.domain;
 		sprintf(srv->unix_addr.sun_path,
-			"_selftests-landlock-net-tid%d-index%d", gettid(),
+			"_selftests-landlock-net-tid%ld-index%d", syscall(SYS_gettid),
 			index);
 		srv->unix_addr_len = SUN_LEN(&srv->unix_addr);
 		srv->unix_addr.sun_path[0] = '\0';
-- 
2.23.0

Re: [PATCH] selftests/landlock:Fix two build issues

[Mickaël Salaün]

On Wed, Jan 10, 2024 at 03:08:54PM +0800, Hu Yadi wrote:
> From: "Hu.Yadi" <hu.yadi@h3c.com>
> 
> Two issues comes up  while building selftest/landlock:
> 
> the first one is as to gettid
> 
> net_test.c: In function ‘set_service’:
> net_test.c:91:45: warning: implicit declaration of function ‘gettid’; did you mean ‘getgid’? [-Wimplicit-function-declaration]
>     "_selftests-landlock-net-tid%d-index%d", gettid(),
>                                              ^~~~~~
>                                              getgid
> net_test.c:(.text+0x4e0): undefined reference to `gettid'
> 
> the second is compiler error
> gcc -Wall -O2 -isystem   fs_test.c -lcap -o /home/linux/tools/testing/selftests/landlock/fs_test
> fs_test.c:4575:9: error: initializer element is not constant
>   .mnt = mnt_tmp,
>          ^~~~~~~

What is the version of GCC (and headers) and on which system (and
version) are you building these tests?

> 
> this patch is to fix them
> 
> Signed-off-by: Hu.Yadi <hu.yadi@h3c.com>
> Suggested-by: Jiao <jiaoxupo@h3c.com>
> Reviewed-by:Berlin <berlin@h3c.com>
> ---
>  tools/testing/selftests/landlock/fs_test.c  | 5 ++++-
>  tools/testing/selftests/landlock/net_test.c | 3 +--
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 18e1f86a6234..93eb40a09073 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -4572,7 +4572,10 @@ FIXTURE_VARIANT(layout3_fs)
>  /* clang-format off */
>  FIXTURE_VARIANT_ADD(layout3_fs, tmpfs) {
>  	/* clang-format on */
> -	.mnt = mnt_tmp,
> +	.mnt = {
> +		.type = "tmpfs",
> +        	.data = "size=4m,mode=700",

When applying this patch we get: "space before tab in indent"

> +	},
>  	.file_path = file1_s1d1,
>  };
>  
> diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
> index 929e21c4db05..8fb357de8c55 100644
> --- a/tools/testing/selftests/landlock/net_test.c
> +++ b/tools/testing/selftests/landlock/net_test.c
> @@ -18,7 +18,6 @@
>  #include <sys/prctl.h>
>  #include <sys/socket.h>
>  #include <sys/un.h>
> -
>  #include "common.h"
>  
>  const short sock_port_start = (1 << 10);
> @@ -88,7 +87,7 @@ static int set_service(struct service_fixture *const srv,
>  	case AF_UNIX:
>  		srv->unix_addr.sun_family = prot.domain;
>  		sprintf(srv->unix_addr.sun_path,
> -			"_selftests-landlock-net-tid%d-index%d", gettid(),
> +			"_selftests-landlock-net-tid%ld-index%d", syscall(SYS_gettid),

You sent another patch that "replace SYS_<syscall> with __NR_<syscall>".
Why not here?

Please follow the same approach:
https://git.kernel.org/stable/c/87129ef13603ae46c82bcd09eed948acf0506dbb

>  			index);
>  		srv->unix_addr_len = SUN_LEN(&srv->unix_addr);
>  		srv->unix_addr.sun_path[0] = '\0';
> -- 
> 2.23.0
> 
> 

回复: [PATCH] selftests/landlock:Fix two build issues

[Huyadi]

->On Wed, Jan 10, 2024 at 03:08:54PM +0800, Hu Yadi wrote:
>> From: "Hu.Yadi" <hu.yadi@h3c.com>
>> 
>> Two issues comes up  while building selftest/landlock:
>> 
>> the first one is as to gettid
>> 
>> net_test.c: In function ‘set_service’:
>> net_test.c:91:45: warning: implicit declaration of function ‘gettid’; did you mean ‘getgid’? [-Wimplicit-function-declaration]
>>     "_selftests-landlock-net-tid%d-index%d", gettid(),
>>                                              ^~~~~~
>>                                              getgid
>> net_test.c:(.text+0x4e0): undefined reference to `gettid'
>> 
>> the second is compiler error
>> gcc -Wall -O2 -isystem   fs_test.c -lcap -o /home/linux/tools/testing/selftests/landlock/fs_test
>> fs_test.c:4575:9: error: initializer element is not constant
>>   .mnt = mnt_tmp,
>>          ^~~~~~~
>
>What is the version of GCC (and headers) and on which system (and
>version) are you building these tests?

gcc 7.3 / glibc-2.28/ kernel 4.19/ OpenEulor20.03

>> 
>> this patch is to fix them
>> 
>> Signed-off-by: Hu.Yadi <hu.yadi@h3c.com>
>> Suggested-by: Jiao <jiaoxupo@h3c.com>
>> Reviewed-by:Berlin <berlin@h3c.com>
>> ---
>>  tools/testing/selftests/landlock/fs_test.c  | 5 ++++-  
>> tools/testing/selftests/landlock/net_test.c | 3 +--
>>  2 files changed, 5 insertions(+), 3 deletions(-)
>> 
>> diff --git a/tools/testing/selftests/landlock/fs_test.c 
>> b/tools/testing/selftests/landlock/fs_test.c
>> index 18e1f86a6234..93eb40a09073 100644
>> --- a/tools/testing/selftests/landlock/fs_test.c
>> +++ b/tools/testing/selftests/landlock/fs_test.c
>> @@ -4572,7 +4572,10 @@ FIXTURE_VARIANT(layout3_fs)
>>  /* clang-format off */
>>  FIXTURE_VARIANT_ADD(layout3_fs, tmpfs) {
>>  	/* clang-format on */
>> -	.mnt = mnt_tmp,
>> +	.mnt = {
>> +		.type = "tmpfs",
>> +        	.data = "size=4m,mode=700",
>
>When applying this patch we get: "space before tab in indent"

Sorry for inconvenient, I'll resend it v2 after checkpatch.pl shows no error.

>> +	},
>>  	.file_path = file1_s1d1,
>>  };
>>  
>> diff --git a/tools/testing/selftests/landlock/net_test.c 
>> b/tools/testing/selftests/landlock/net_test.c
>> index 929e21c4db05..8fb357de8c55 100644
>> --- a/tools/testing/selftests/landlock/net_test.c
>> +++ b/tools/testing/selftests/landlock/net_test.c
>> @@ -18,7 +18,6 @@
>>  #include <sys/prctl.h>
>>  #include <sys/socket.h>
>>  #include <sys/un.h>
>> -
>>  #include "common.h"
>>  
>>  const short sock_port_start = (1 << 10); @@ -88,7 +87,7 @@ static int 
>> set_service(struct service_fixture *const srv,
>>  	case AF_UNIX:
>>  		srv->unix_addr.sun_family = prot.domain;
>>  		sprintf(srv->unix_addr.sun_path,
>> -			"_selftests-landlock-net-tid%d-index%d", gettid(),
>> +			"_selftests-landlock-net-tid%ld-index%d", syscall(SYS_gettid),
>
>You sent another patch that "replace SYS_<syscall> with __NR_<syscall>".
>Why not here?
>
>Please follow the same approach:
>https://git.kernel.org/stable/c/87129ef13603ae46c82bcd09eed948acf0506dbb

Got it, I'll resend it v2 including the fix 

>>  			index);
>>  		srv->unix_addr_len = SUN_LEN(&srv->unix_addr);
>>  		srv->unix_addr.sun_path[0] = '\0';
>> --
>> 2.23.0
>> 
>> 

 

Re: 回复: [PATCH] selftests/landlock:Fix two build issues

[Mickaël Salaün]

On Thu, Jan 11, 2024 at 02:34:08AM +0000, Huyadi wrote:
> 
> ->On Wed, Jan 10, 2024 at 03:08:54PM +0800, Hu Yadi wrote:
> >> From: "Hu.Yadi" <hu.yadi@h3c.com>
> >> 
> >> Two issues comes up  while building selftest/landlock:
> >> 
> >> the first one is as to gettid
> >> 
> >> net_test.c: In function ‘set_service’:
> >> net_test.c:91:45: warning: implicit declaration of function ‘gettid’; did you mean ‘getgid’? [-Wimplicit-function-declaration]
> >>     "_selftests-landlock-net-tid%d-index%d", gettid(),
> >>                                              ^~~~~~
> >>                                              getgid
> >> net_test.c:(.text+0x4e0): undefined reference to `gettid'
> >> 
> >> the second is compiler error
> >> gcc -Wall -O2 -isystem   fs_test.c -lcap -o /home/linux/tools/testing/selftests/landlock/fs_test
> >> fs_test.c:4575:9: error: initializer element is not constant
> >>   .mnt = mnt_tmp,
> >>          ^~~~~~~
> >
> >What is the version of GCC (and headers) and on which system (and
> >version) are you building these tests?
> 
> gcc 7.3 / glibc-2.28/ kernel 4.19/ OpenEulor20.03

These are old versions. You should mention in the commit message which
version of glibc added gettid().

> 
> >> 
> >> this patch is to fix them
> >> 
> >> Signed-off-by: Hu.Yadi <hu.yadi@h3c.com>
> >> Suggested-by: Jiao <jiaoxupo@h3c.com>
> >> Reviewed-by:Berlin <berlin@h3c.com>
> >> ---
> >>  tools/testing/selftests/landlock/fs_test.c  | 5 ++++-  
> >> tools/testing/selftests/landlock/net_test.c | 3 +--
> >>  2 files changed, 5 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/tools/testing/selftests/landlock/fs_test.c 
> >> b/tools/testing/selftests/landlock/fs_test.c
> >> index 18e1f86a6234..93eb40a09073 100644
> >> --- a/tools/testing/selftests/landlock/fs_test.c
> >> +++ b/tools/testing/selftests/landlock/fs_test.c
> >> @@ -4572,7 +4572,10 @@ FIXTURE_VARIANT(layout3_fs)
> >>  /* clang-format off */
> >>  FIXTURE_VARIANT_ADD(layout3_fs, tmpfs) {
> >>  	/* clang-format on */
> >> -	.mnt = mnt_tmp,

Can you just cast mnt_tmp? It exists to avoid such duplicate code.

> >> +	.mnt = {
> >> +		.type = "tmpfs",
> >> +        	.data = "size=4m,mode=700",
> >
> >When applying this patch we get: "space before tab in indent"
> 
> Sorry for inconvenient, I'll resend it v2 after checkpatch.pl shows no error.
> 
> >> +	},
> >>  	.file_path = file1_s1d1,
> >>  };
> >>  
> >> diff --git a/tools/testing/selftests/landlock/net_test.c 
> >> b/tools/testing/selftests/landlock/net_test.c
> >> index 929e21c4db05..8fb357de8c55 100644
> >> --- a/tools/testing/selftests/landlock/net_test.c
> >> +++ b/tools/testing/selftests/landlock/net_test.c
> >> @@ -18,7 +18,6 @@
> >>  #include <sys/prctl.h>
> >>  #include <sys/socket.h>
> >>  #include <sys/un.h>
> >> -
> >>  #include "common.h"
> >>  
> >>  const short sock_port_start = (1 << 10); @@ -88,7 +87,7 @@ static int 
> >> set_service(struct service_fixture *const srv,
> >>  	case AF_UNIX:
> >>  		srv->unix_addr.sun_family = prot.domain;
> >>  		sprintf(srv->unix_addr.sun_path,
> >> -			"_selftests-landlock-net-tid%d-index%d", gettid(),
> >> +			"_selftests-landlock-net-tid%ld-index%d", syscall(SYS_gettid),
> >
> >You sent another patch that "replace SYS_<syscall> with __NR_<syscall>".
> >Why not here?
> >
> >Please follow the same approach:
> >https://git.kernel.org/stable/c/87129ef13603ae46c82bcd09eed948acf0506dbb

Can you please add a getpid() helper like for renameat2() in this
commit?

Also, all Landlock-related code is formatted with clang-format. You can
do it with clang-format -i tools/testing/selftests/landlock/*.[ch]

> 
> Got it, I'll resend it v2 including the fix 
> 
> >>  			index);
> >>  		srv->unix_addr_len = SUN_LEN(&srv->unix_addr);
> >>  		srv->unix_addr.sun_path[0] = '\0';
> >> --
> >> 2.23.0
> >> 
> >> 
> 
>  

回复: 回复: [PATCH] selftests/landlock:Fix two build issues

[Huyadi]

>On Thu, Jan 11, 2024 at 02:34:08AM +0000, Huyadi wrote:
>> 
>> ->On Wed, Jan 10, 2024 at 03:08:54PM +0800, Hu Yadi wrote:
>> >> From: "Hu.Yadi" <hu.yadi@h3c.com>
>> >> 
>> >> Two issues comes up  while building selftest/landlock:
>> >> 
>> >> the first one is as to gettid
>> >> 
>> >> net_test.c: In function ‘set_service’:
>> >> net_test.c:91:45: warning: implicit declaration of function ‘gettid’; did you mean ‘getgid’? [-Wimplicit-function-declaration]
>> >>     "_selftests-landlock-net-tid%d-index%d", gettid(),
>> >>                                              ^~~~~~
>> >>                                              getgid
>> >> net_test.c:(.text+0x4e0): undefined reference to `gettid'
>> >> 
>> >> the second is compiler error
>> >> gcc -Wall -O2 -isystem   fs_test.c -lcap -o /home/linux/tools/testing/selftests/landlock/fs_test
>> >> fs_test.c:4575:9: error: initializer element is not constant
>> >>   .mnt = mnt_tmp,
>> >>          ^~~~~~~
>> >
>> >What is the version of GCC (and headers) and on which system (and
>> >version) are you building these tests?
>> 
>> gcc 7.3 / glibc-2.28/ kernel 4.19/ OpenEulor20.03
>
>These are old versions. You should mention in the commit message which version of glibc added gettid().>

Ok, I'll add it .

>> 
>> >> 
>> >> this patch is to fix them
>> >> 
>> >> Signed-off-by: Hu.Yadi <hu.yadi@h3c.com>
>> >> Suggested-by: Jiao <jiaoxupo@h3c.com> Reviewed-by:Berlin 
>> >> <berlin@h3c.com>
>> >> ---
>> >>  tools/testing/selftests/landlock/fs_test.c  | 5 ++++- 
>> >> tools/testing/selftests/landlock/net_test.c | 3 +--
>> >>  2 files changed, 5 insertions(+), 3 deletions(-)
>> >> 
>> >> diff --git a/tools/testing/selftests/landlock/fs_test.c
>> >> b/tools/testing/selftests/landlock/fs_test.c
>> >> index 18e1f86a6234..93eb40a09073 100644
>> >> --- a/tools/testing/selftests/landlock/fs_test.c
>> >> +++ b/tools/testing/selftests/landlock/fs_test.c
>> >> @@ -4572,7 +4572,10 @@ FIXTURE_VARIANT(layout3_fs)
>> >>  /* clang-format off */
>> >>  FIXTURE_VARIANT_ADD(layout3_fs, tmpfs) {
>> >>  	/* clang-format on */
>> >> -	.mnt = mnt_tmp,
>
>Can you just cast mnt_tmp? It exists to avoid such duplicate code.

I tried to add cast, but it is not effective.  what's more, all FIXTURE_VARIANT_ADD declaration under tools/testing are assigned with constant,
So, current solution looks fine to me, what's your option?
>
>> >> +	.mnt = {
>> >> +		.type = "tmpfs",
>> >> +        	.data = "size=4m,mode=700",
>> >
>> >When applying this patch we get: "space before tab in indent"
>> 
>> Sorry for inconvenient, I'll resend it v2 after checkpatch.pl shows no error.
>> 
>> >> +	},
>> >>  	.file_path = file1_s1d1,
>> >>  };
>> >>  
>> >> diff --git a/tools/testing/selftests/landlock/net_test.c
>> >> b/tools/testing/selftests/landlock/net_test.c
>> >> index 929e21c4db05..8fb357de8c55 100644
>> >> --- a/tools/testing/selftests/landlock/net_test.c
>> >> +++ b/tools/testing/selftests/landlock/net_test.c
>> >> @@ -18,7 +18,6 @@
>> >>  #include <sys/prctl.h>
>> >>  #include <sys/socket.h>
>> >>  #include <sys/un.h>
>> >> -
>> >>  #include "common.h"
>> >>  
>> >>  const short sock_port_start = (1 << 10); @@ -88,7 +87,7 @@ static 
>> >> int set_service(struct service_fixture *const srv,
>> >>  	case AF_UNIX:
>> >>  		srv->unix_addr.sun_family = prot.domain;
>> >>  		sprintf(srv->unix_addr.sun_path,
>> >> -			"_selftests-landlock-net-tid%d-index%d", gettid(),
>> >> +			"_selftests-landlock-net-tid%ld-index%d", syscall(SYS_gettid),
>> >
>> >You sent another patch that "replace SYS_<syscall> with __NR_<syscall>".
>> >Why not here?
>> >
>> >Please follow the same approach:
>> >https://git.kernel.org/stable/c/87129ef13603ae46c82bcd09eed948acf0506
>> >dbb
>
>Can you please add a getpid() helper like for renameat2() in this commit?>

Thanks your warm instruction, I'll do it and send patch soon.

>Also, all Landlock-related code is formatted with clang-format. You can do it with clang-format -i tools/testing/selftests/landlock/*.[ch]
>
>> 
>> Got it, I'll resend it v2 including the fix
>> 
>> >>  			index);
>> >>  		srv->unix_addr_len = SUN_LEN(&srv->unix_addr);
>> >>  		srv->unix_addr.sun_path[0] = '\0';
>> >> --
>> >> 2.23.0
>> >> 
>> >> 
>> 
>>