From: "Hu.Yadi" <[email protected]>
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 <[email protected]>
Suggested-by: Jiao <[email protected]>
Reviewed-by:Berlin <[email protected]>
---
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
On Wed, Jan 10, 2024 at 03:08:54PM +0800, Hu Yadi wrote:
> From: "Hu.Yadi" <[email protected]>
>
> 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 <[email protected]>
> Suggested-by: Jiao <[email protected]>
> Reviewed-by:Berlin <[email protected]>
> ---
> 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
>
>
->On Wed, Jan 10, 2024 at 03:08:54PM +0800, Hu Yadi wrote:
>> From: "Hu.Yadi" <[email protected]>
>>
>> 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 <[email protected]>
>> Suggested-by: Jiao <[email protected]>
>> Reviewed-by:Berlin <[email protected]>
>> ---
>> 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
>>
>>
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" <[email protected]>
> >>
> >> 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 <[email protected]>
> >> Suggested-by: Jiao <[email protected]>
> >> Reviewed-by:Berlin <[email protected]>
> >> ---
> >> 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
> >>
> >>
>
>
>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" <[email protected]>
>> >>
>> >> 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 <[email protected]>
>> >> Suggested-by: Jiao <[email protected]> Reviewed-by:Berlin
>> >> <[email protected]>
>> >> ---
>> >> 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
>> >>
>> >>
>>
>>