[PATCH v3 0/2] tools/nolibc: add pipe(), pipe2() and their testcase

[PATCH v3 0/2] tools/nolibc: add pipe(), pipe2() and their testcase

[Yuan Tan]

Hi Willy and Thomas,

In v3, I have fixed all the problems you mentioned.

Welcome any other suggestion.

---
Changes in v3:
- Fix the missing return 
- Fix __NR_pipe to __NR_pipe2
- Fix the missing static
- Test case works in one process
- Link to v2:
  https://lore.kernel.org/all/cover.1690733545.git.tanyuan@tinylab.org

Changes in v2:
- Use sys_pipe2 to implement the pipe()
- Use memcmp() instead of strcmp()
- Link to v1:
  https://lore.kernel.org/all/cover.1690307717.git.tanyuan@tinylab.org

---
Yuan Tan (2):
  tools/nolibc: add pipe() and pipe2() support
  selftests/nolibc: add testcase for pipe

 tools/include/nolibc/sys.h                   | 24 ++++++++++++++++++++
 tools/testing/selftests/nolibc/nolibc-test.c | 22 ++++++++++++++++++
 2 files changed, 46 insertions(+)

-- 
2.34.1

[PATCH v3 1/2] tools/nolibc: add pipe() and pipe2() support

[Yuan Tan]

According to manual page [1], posix spec [2] and source code like
arch/mips/kernel/syscall.c, for historic reasons, the sys_pipe() syscall
on some architectures has an unusual calling convention.  It returns
results in two registers which means there is no need for it to do
verify the validity of a userspace pointer argument.  Historically that
used to be expensive in Linux.  These days the performance advantage is
negligible.

Nolibc doesn't support the unusual calling convention above, luckily
Linux provides a generic sys_pipe2() with an additional flags argument
from 2.6.27. If flags is 0, then pipe2() is the same as pipe(). So here
we use sys_pipe2() to implement the pipe().

pipe2() is also provided to allow users to use flags argument on demand.

[1]: https://man7.org/linux/man-pages/man2/pipe.2.html
[2]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/pipe.html

Suggested-by: Zhangjin Wu <falcon@tinylab.org>
Link: https://lore.kernel.org/all/20230729100401.GA4577@1wt.eu/
Signed-off-by: Yuan Tan <tanyuan@tinylab.org>
---
 tools/include/nolibc/sys.h | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index 8bfe7db20b80..56f63eb48a1b 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -752,6 +752,30 @@ int open(const char *path, int flags, ...)
 }
 
 
+/*
+ * int pipe2(int pipefd[2], int flags);
+ * int pipe(int pipefd[2]);
+ */
+
+static __attribute__((unused))
+int sys_pipe2(int pipefd[2], int flags)
+{
+	return my_syscall2(__NR_pipe2, pipefd, flags);
+}
+
+static __attribute__((unused))
+int pipe2(int pipefd[2], int flags)
+{
+	return __sysret(sys_pipe2(pipefd, flags));
+}
+
+static __attribute__((unused))
+int pipe(int pipefd[2])
+{
+	return pipe2(pipefd, 0);
+}
+
+
 /*
  * int prctl(int option, unsigned long arg2, unsigned long arg3,
  *                       unsigned long arg4, unsigned long arg5);
-- 
2.34.1

[PATCH v3 2/2] selftests/nolibc: add testcase for pipe

[Yuan Tan]

Add a test case of pipe that sends and receives message in a single
process.

Suggested-by: Thomas Weißschuh <thomas@t-8ch.de>
Suggested-by: Willy Tarreau <w@1wt.eu>
Link: https://lore.kernel.org/all/c5de2d13-3752-4e1b-90d9-f58cca99c702@t-8ch.de/
Signed-off-by: Yuan Tan <tanyuan@tinylab.org>
---
 tools/testing/selftests/nolibc/nolibc-test.c | 22 ++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 03b1d30f5507..e5667fa3cf0a 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -767,6 +767,27 @@ int test_mmap_munmap(void)
 	return ret;
 }
 
+static int test_pipe(void)
+{
+	const char *const msg = "hello, nolibc";
+	int pipefd[2];
+	char buf[32];
+	ssize_t len;
+
+	if (pipe(pipefd) == -1)
+		return 1;
+
+	write(pipefd[1], msg, strlen(msg));
+	close(pipefd[1]);
+	len = read(pipefd[0], buf, sizeof(buf));
+	close(pipefd[0]);
+
+	if (len != strlen(msg))
+		return 1;
+
+	return !!memcmp(buf, msg, len);
+}
+
 
 /* Run syscall tests between IDs <min> and <max>.
  * Return 0 on success, non-zero on failure.
@@ -851,6 +872,7 @@ int run_syscall(int min, int max)
 		CASE_TEST(mmap_munmap_good);  EXPECT_SYSZR(1, test_mmap_munmap()); break;
 		CASE_TEST(open_tty);          EXPECT_SYSNE(1, tmp = open("/dev/null", 0), -1); if (tmp != -1) close(tmp); break;
 		CASE_TEST(open_blah);         EXPECT_SYSER(1, tmp = open("/proc/self/blah", 0), -1, ENOENT); if (tmp != -1) close(tmp); break;
+		CASE_TEST(pipe);              EXPECT_SYSZR(1, test_pipe()); break;
 		CASE_TEST(poll_null);         EXPECT_SYSZR(1, poll(NULL, 0, 0)); break;
 		CASE_TEST(poll_stdout);       EXPECT_SYSNE(1, ({ struct pollfd fds = { 1, POLLOUT, 0}; poll(&fds, 1, 0); }), -1); break;
 		CASE_TEST(poll_fault);        EXPECT_SYSER(1, poll((void *)1, 1, 0), -1, EFAULT); break;
-- 
2.34.1

Re: [PATCH v3 0/2] tools/nolibc: add pipe(), pipe2() and their testcase

[Thomas Weißschuh]

Aug 1, 2023 17:40:01 Yuan Tan <tanyuan@tinylab.org>:

> Hi Willy and Thomas,
>
> In v3, I have fixed all the problems you mentioned.
>
> Welcome any other suggestion.
>
> ---
> Changes in v3:
> - Fix the missing return
> - Fix __NR_pipe to __NR_pipe2
> - Fix the missing static
> - Test case works in one process
> - Link to v2:
>   https://lore.kernel.org/all/cover.1690733545.git.tanyuan@tinylab.org
>
> Changes in v2:
> - Use sys_pipe2 to implement the pipe()
> - Use memcmp() instead of strcmp()
> - Link to v1:
>   https://lore.kernel.org/all/cover.1690307717.git.tanyuan@tinylab.org
>
> ---
> Yuan Tan (2):
>   tools/nolibc: add pipe() and pipe2() support
>   selftests/nolibc: add testcase for pipe
>
> tools/include/nolibc/sys.h                   | 24 ++++++++++++++++++++
> tools/testing/selftests/nolibc/nolibc-test.c | 22 ++++++++++++++++++
> 2 files changed, 46 insertions(+)

For the full series:

Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>

Thanks!

Re: [PATCH v3 0/2] tools/nolibc: add pipe(), pipe2() and their testcase

[Willy Tarreau]

Hi,

On Tue, Aug 01, 2023 at 05:42:57PM +0200, Thomas Weißschuh  wrote:
> 
> Aug 1, 2023 17:40:01 Yuan Tan <tanyuan@tinylab.org>:
> 
> > Hi Willy and Thomas,
> >
> > In v3, I have fixed all the problems you mentioned.
> >
> > Welcome any other suggestion.
> >
> > ---
> > Changes in v3:
> > - Fix the missing return
> > - Fix __NR_pipe to __NR_pipe2
> > - Fix the missing static
> > - Test case works in one process
> > - Link to v2:
> >   https://lore.kernel.org/all/cover.1690733545.git.tanyuan@tinylab.org
> >
> > Changes in v2:
> > - Use sys_pipe2 to implement the pipe()
> > - Use memcmp() instead of strcmp()
> > - Link to v1:
> >   https://lore.kernel.org/all/cover.1690307717.git.tanyuan@tinylab.org
> >
> > ---
> > Yuan Tan (2):
> >   tools/nolibc: add pipe() and pipe2() support
> >   selftests/nolibc: add testcase for pipe
> >
> > tools/include/nolibc/sys.h                   | 24 ++++++++++++++++++++
> > tools/testing/selftests/nolibc/nolibc-test.c | 22 ++++++++++++++++++
> > 2 files changed, 46 insertions(+)
> 
> For the full series:
> 
> Reviewed-by: Thomas Weißschuh <linux@weissschuh.net>

Thank you both, now queued on top of the rest in 20230801-nolibc-next-1.
Thomas I'll try to review your last series tomorrow, at first glance it
looks OK.

Thanks,
Willy