2024-01-12 07:16:29

by Huyadi

[permalink] [raw]
Subject: [PATCH v3] selftests/landlock:Fix two build issues

From: "Hu.Yadi" <[email protected]>

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,
^~~~~~~

this patch is to fix them

Signed-off-by: Hu.Yadi <[email protected]>
Suggested-by: Jiao <[email protected]>
Reviewed-by: Berlin <[email protected]>
---
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 | 9 +++++++++
2 files changed, 13 insertions(+), 1 deletion(-)

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..12a6744568e2 100644
--- a/tools/testing/selftests/landlock/net_test.c
+++ b/tools/testing/selftests/landlock/net_test.c
@@ -21,6 +21,15 @@

#include "common.h"

+#ifndef gettid
+static pid_t gettid(void)
+{
+ return syscall(__NR_gettid);
+}
+
+#endif
+
+
const short sock_port_start = (1 << 10);

static const char loopback_ipv4[] = "127.0.0.1";
--
2.23.0



2024-01-12 10:46:14

by Mickaël Salaün

[permalink] [raw]
Subject: Re: [PATCH v3] selftests/landlock:Fix two build issues

On Fri, Jan 12, 2024 at 03:12:45PM +0800, Hu Yadi wrote:
> From: "Hu.Yadi" <[email protected]>
>
> 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,
> ^~~~~~~
>
> this patch is to fix them
>
> Signed-off-by: Hu.Yadi <[email protected]>
> Suggested-by: Jiao <[email protected]>
> Reviewed-by: Berlin <[email protected]>
> ---
> 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 | 9 +++++++++
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> 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",

OK, C doesn't play nice with constants. A better approach would be to
define MNT_TMP_DATA, and use it with this fixture variant and the
mnt_tmp variable. No need for a MNT_TMP_TYPE though. While you're at
it, you can also make the mnt_tmp variable static const.

You can create two patches from this one. This first hunk with:

Fixes: 04f9070e99a4 ("selftests/landlock: Add tests for pseudo filesystems")

> + },
> .file_path = file1_s1d1,
> };
>
> diff --git a/tools/testing/selftests/landlock/net_test.c b/tools/testing/selftests/landlock/net_test.c
> index 929e21c4db05..12a6744568e2 100644
> --- a/tools/testing/selftests/landlock/net_test.c
> +++ b/tools/testing/selftests/landlock/net_test.c
> @@ -21,6 +21,15 @@
>
> #include "common.h"

..and this second hunk for another patch with:

Fixes: a549d055a22e ("selftests/landlock: Add network tests")
Cc: Konstantin Meskhidze <[email protected]>

>
> +#ifndef gettid
> +static pid_t gettid(void)
> +{
> + return syscall(__NR_gettid);
> +}
> +

Please remove this newline.

> +#endif

You can move this gettid hunk to common.h, under the
landlock_restrict_self block.

> +
> +
> const short sock_port_start = (1 << 10);
>
> static const char loopback_ipv4[] = "127.0.0.1";
> --
> 2.23.0
>

2024-01-15 03:47:54

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v3] selftests/landlock:Fix two build issues

Hi Hu,

kernel test robot noticed the following build errors:

[auto build test ERROR on shuah-kselftest/next]
[also build test ERROR on shuah-kselftest/fixes linus/master v6.7 next-20240112]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Hu-Yadi/selftests-landlock-Fix-two-build-issues/20240112-151805
base: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next
patch link: https://lore.kernel.org/r/20240112071245.669-1-hu.yadi%40h3c.com
patch subject: [PATCH v3] selftests/landlock:Fix two build issues
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240115/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All errors (new ones prefixed by >>):

>> net_test.c:25:14: error: static declaration of 'gettid' follows non-static declaration
25 | static pid_t gettid(void)
| ^~~~~~
In file included from /usr/include/unistd.h:1218,
from /usr/include/x86_64-linux-gnu/bits/sigstksz.h:24,
from /usr/include/signal.h:328,
from /usr/include/x86_64-linux-gnu/sys/wait.h:36,
from common.h:16,
from net_test.c:22:
/usr/include/x86_64-linux-gnu/bits/unistd_ext.h:34:16: note: previous declaration of 'gettid' with type '__pid_t(void)' {aka 'int(void)'}
34 | extern __pid_t gettid (void) __THROW;
| ^~~~~~

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

2024-01-15 10:44:38

by Huyadi

[permalink] [raw]
Subject: 回复: [PATCH v3] selftests/landlock:Fix two b uild issues


>Hi Hu,
>
>kernel test robot noticed the following build errors:
>
> [auto build test ERROR on shuah-kselftest/next] [also build test ERROR on shuah-kselftest/fixes linus/master v6.7 next-20240112] [If your patch is applied to the wrong git tree, kindly drop us a note.
>And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]

>url: https://github.com/intel-lab-lkp/linux/commits/Hu-Yadi/selftests-landlock-Fix-two-build-issues/20240112-151805
>base: https://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git next
>patch link: https://lore.kernel.org/r/20240112071245.669-1-hu.yadi%40h3c.com
>patch subject: [PATCH v3] selftests/landlock:Fix two build issues
>compiler: gcc-12 (Debian 12.2.0-14) 12.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240115/[email protected]/reproduce)
>
>If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
>| Reported-by: kernel test robot <[email protected]>
>| Closes:
>| https://lore.kernel.org/oe-kbuild-all/202401151147.T1s11iHJ-lkp@intel.
>| com/
>
>All errors (new ones prefixed by >>):
>

Fix it, and send patch V4 soon.

>>> net_test.c:25:14: error: static declaration of 'gettid' follows
>>> non-static declaration
> 25 | static pid_t gettid(void)
> | ^~~~~~
> In file included from /usr/include/unistd.h:1218,
> from /usr/include/x86_64-linux-gnu/bits/sigstksz.h:24,
> from /usr/include/signal.h:328,
> from /usr/include/x86_64-linux-gnu/sys/wait.h:36,
> from common.h:16,
> from net_test.c:22:
> /usr/include/x86_64-linux-gnu/bits/unistd_ext.h:34:16: note: previous declaration of 'gettid' with type '__pid_t(void)' {aka 'int(void)'}
> 34 | extern __pid_t gettid (void) __THROW;
> | ^~~~~~
>
>--
>0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki