| Message ID | 20240115102409.19799-1-hu.yadi@h3c.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [v4] selftests/landlock:Fix two build issues | expand |
On Mon, Jan 15, 2024 at 06:24:09PM +0800, Hu Yadi wrote: > From: "Hu.Yadi" <hu.yadi@h3c.com> > > Two issues comes up while building selftest/landlock on my side > (gcc 7.3/glibc-2.28/kernel-4.19) > > 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’; [-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 selftests/landlock/fs_test > fs_test.c:4575:9: error: initializer element is not constant > .mnt = mnt_tmp, > ^~~~~~~ > > Fixes: 04f9070e99a4 ("selftests/landlock: Add tests for pseudo filesystems") > Fixes: a549d055a22e ("selftests/landlock: Add network tests") Could you please create two patches as requested for v3, one per fix? This is useful because it enables to backport these fixes when appropriate. > > 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> > --- > Changes v4 -> v3: > fix gettid error from kernel test robot > https://lore.kernel.org/oe-kbuild-all/202401151147.T1s11iHJ-lkp@intel.com/ > Changes v3 -> v2: > - add helper of gettid instead of __NR_gettid > - add gcc/glibc version info in comments > Changes v1 -> v2: > - fix whitespace error > - replace SYS_gettid with _NR_gettid > > tools/testing/selftests/landlock/fs_test.c | 5 ++++- > tools/testing/selftests/landlock/net_test.c | 7 ++++++- > 2 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c > index 18e1f86a6234..a992cf7c0ad1 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", > + }, I requested some changes here. > .file_path = file1_s1d1, > }; > > diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c > index 929e21c4db05..d50f2920ed82 100644 > --- a/tools/testing/selftests/landlock/net_test.c > +++ b/tools/testing/selftests/landlock/net_test.c > @@ -21,6 +21,11 @@ We should include sys/syscall.h > > #include "common.h" > > +static pid_t landlock_gettid(void) Please rename to sys_gettid(). > +{ > + return syscall(__NR_gettid); > +} > + > const short sock_port_start = (1 << 10); > > static const char loopback_ipv4[] = "127.0.0.1"; > @@ -88,7 +93,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%d-index%d", landlock_gettid(), > index); > srv->unix_addr_len = SUN_LEN(&srv->unix_addr); > srv->unix_addr.sun_path[0] = '\0'; > -- > 2.23.0 >
> On Mon, Jan 15, 2024 at 06:24:09PM +0800, Hu Yadi wrote: >> From: "Hu.Yadi" <hu.yadi@h3c.com> >> >> Two issues comes up while building selftest/landlock on my side (gcc >> 7.3/glibc-2.28/kernel-4.19) >> >> 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’; [-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 selftests/landlock/fs_test >> fs_test.c:4575:9: error: initializer element is not constant >> .mnt = mnt_tmp, >> ^~~~~~~ >> >> Fixes: 04f9070e99a4 ("selftests/landlock: Add tests for pseudo >> filesystems") >> Fixes: a549d055a22e ("selftests/landlock: Add network tests") > >Could you please create two patches as requested for v3, one per fix? >This is useful because it enables to backport these fixes when appropriate. OK, I'll resend it by two patches with your warmly instruction. Thanks again. >> >> 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> >> --- >> Changes v4 -> v3: >> fix gettid error from kernel test robot >> >> https://lore.kernel.org/oe-kbuild-all/202401151147.T1s11iHJ-lkp@intel. >> com/ >> Changes v3 -> v2: >> - add helper of gettid instead of __NR_gettid >> - add gcc/glibc version info in comments Changes v1 -> v2: >> - fix whitespace error >> - replace SYS_gettid with _NR_gettid >> >> tools/testing/selftests/landlock/fs_test.c | 5 ++++- >> tools/testing/selftests/landlock/net_test.c | 7 ++++++- >> 2 files changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/tools/testing/selftests/landlock/fs_test.c >> b/tools/testing/selftests/landlock/fs_test.c >> index 18e1f86a6234..a992cf7c0ad1 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", >> + }, > >I requested some changes here. > >> .file_path = file1_s1d1, >> }; >> >> diff --git a/tools/testing/selftests/landlock/net_test.c >> b/tools/testing/selftests/landlock/net_test.c >> index 929e21c4db05..d50f2920ed82 100644 >> --- a/tools/testing/selftests/landlock/net_test.c >> +++ b/tools/testing/selftests/landlock/net_test.c > @@ -21,6 +21,11 @@ > >We should include sys/syscall.h > >> >> #include "common.h" >> >> +static pid_t landlock_gettid(void) > >Please rename to sys_gettid(). > >> +{ >> + return syscall(__NR_gettid); >> +} >> + >> const short sock_port_start = (1 << 10); >> >> static const char loopback_ipv4[] = "127.0.0.1"; @@ -88,7 +93,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%d-index%d", landlock_gettid(), >> index); >> srv->unix_addr_len = SUN_LEN(&srv->unix_addr); >> srv->unix_addr.sun_path[0] = '\0'; >> -- >> 2.23.0 >>
>> Changes v3 -> v2: >> - add helper of gettid instead of __NR_gettid >> - add gcc/glibc version info in comments Changes v1 -> v2: >> - fix whitespace error >> - replace SYS_gettid with _NR_gettid >> >> tools/testing/selftests/landlock/fs_test.c | 5 ++++- >> tools/testing/selftests/landlock/net_test.c | 7 ++++++- >> 2 files changed, 10 insertions(+), 2 deletions(-) >> >> diff --git a/tools/testing/selftests/landlock/fs_test.c >> b/tools/testing/selftests/landlock/fs_test.c >> index 18e1f86a6234..a992cf7c0ad1 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", >> + }, > >I requested some changes here. > Could you give me some inspiration how to fix it? it looks fine to me to assign value as above, which consistent with other pseudo FS tests. Thanks in advance. >> .file_path = file1_s1d1, >> }; >>
On Tue, Jan 23, 2024 at 12:04:17PM +0000, Huyadi wrote: > > >> Changes v3 -> v2: > >> - add helper of gettid instead of __NR_gettid > >> - add gcc/glibc version info in comments Changes v1 -> v2: > >> - fix whitespace error > >> - replace SYS_gettid with _NR_gettid > >> > >> tools/testing/selftests/landlock/fs_test.c | 5 ++++- > >> tools/testing/selftests/landlock/net_test.c | 7 ++++++- > >> 2 files changed, 10 insertions(+), 2 deletions(-) > >> > >> diff --git a/tools/testing/selftests/landlock/fs_test.c > >> b/tools/testing/selftests/landlock/fs_test.c > >> index 18e1f86a6234..a992cf7c0ad1 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", > >> + }, > > > >I requested some changes here. > > > > Could you give me some inspiration how to fix it? > it looks fine to me to assign value as above, which consistent with other pseudo FS tests. > Thanks in advance. Just add and use this for the two tmpfs data: #define MNT_TMP_DATA "size=4m,mode=700" You can also make the mnt_tmp variable static const. > > >> .file_path = file1_s1d1, > >> }; > >> >
>On Tue, Jan 23, 2024 at 12:04:17PM +0000, Huyadi wrote: >> >> >> Changes v3 -> v2: >> >> - add helper of gettid instead of __NR_gettid >> >> - add gcc/glibc version info in comments Changes v1 -> v2: >> >> - fix whitespace error >> >> - replace SYS_gettid with _NR_gettid >> >> >> >> tools/testing/selftests/landlock/fs_test.c | 5 ++++- >> >> tools/testing/selftests/landlock/net_test.c | 7 ++++++- >> >> 2 files changed, 10 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/tools/testing/selftests/landlock/fs_test.c >> >> b/tools/testing/selftests/landlock/fs_test.c >> >> index 18e1f86a6234..a992cf7c0ad1 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", >> >> + }, >> > >> >I requested some changes here. >> > >> >> Could you give me some inspiration how to fix it? >> it looks fine to me to assign value as above, which consistent with other pseudo FS tests. >> Thanks in advance. > >Just add and use this for the two tmpfs data: >#define MNT_TMP_DATA "size=4m,mode=700" > >You can also make the mnt_tmp variable static const. static const is not useful for the case, the errot still, and I'll use macro definition to solve it. thanks your warmly instruction,I'll send next version , please help to review it. > > >> .file_path = file1_s1d1, > >> }; > >> >
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 18e1f86a6234..a992cf7c0ad1 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..d50f2920ed82 100644 --- a/tools/testing/selftests/landlock/net_test.c +++ b/tools/testing/selftests/landlock/net_test.c @@ -21,6 +21,11 @@ #include "common.h" +static pid_t landlock_gettid(void) +{ + return syscall(__NR_gettid); +} + const short sock_port_start = (1 << 10); static const char loopback_ipv4[] = "127.0.0.1"; @@ -88,7 +93,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%d-index%d", landlock_gettid(), index); srv->unix_addr_len = SUN_LEN(&srv->unix_addr); srv->unix_addr.sun_path[0] = '\0';