All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static
@ 2022-12-12 12:48 ` Ammar Faizi
  0 siblings, 0 replies; 11+ messages in thread
From: Ammar Faizi @ 2022-11-24 16:28 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Ammar Faizi, Dylan Yudaken, Pavel Begunkov, Gilang Fachrezy,
	Muhammad Rizki, Alviro Iskandar Setiawan, VNLX Kernel Department,
	io-uring Mailing List, GNU/Weeb Mailing List

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Hi Jens,

This is a v2 revision.

This series is a -Wmissing-prototypes enforcement. -Wmissing-prototypes
is a clang C compiler flag that warns us if we have functions or
variables that are not used outisde the translation unit, but not marked
as static.

There are 8 patches in this series:

- Patch #1, is just a typo cleanup.
- Patch #2, core library change, mark __io_uring_flush_sq() as static.
- Patch #3 to patch #7 are the cleanup preparation for
  -Wmissing-prototypes enforcement.
- Patch #8 is to enforce -Wmissing-prototypes on the GitHub CI robot.

This series has been build-tested each patch with the GitHub CI robot
and no breakage is found.

## Changelog

v2:
  - Add a new patch #1, fix typo.
  - Don't export __io_uring_flush_sq(). Instead, make a copy of this
    function in test/helpers.c.
  - Massage the commit message.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---

Ammar Faizi (8):
  queue: Fix typo "entererd" -> "entered"
  queue: Mark `__io_uring_flush_sq()` as static
  test/io_uring_setup: Remove unused functions
  ucontext-cp: Remove an unused function
  tests: Mark non-exported functions as static
  ucontext-cp: Mark a non-exported function as static
  test/Makefile: Omit `-Wmissing-prototypes` from the C++ compiler flags
  github: Add `-Wmissing-prototypes` for GitHub CI bot

 .github/workflows/build.yml |  7 ++--
 examples/ucontext-cp.c      | 19 +--------
 src/queue.c                 |  4 +-
 test/Makefile               | 11 ++++-
 test/accept-link.c          |  2 +-
 test/accept-reuse.c         |  9 ++--
 test/ce593a6c480a.c         |  4 +-
 test/defer-taskrun.c        |  2 +-
 test/exit-no-cleanup.c      |  2 +-
 test/hardlink.c             |  2 +-
 test/helpers.c              | 33 +++++++++++++++
 test/helpers.h              |  2 +
 test/io_uring_passthrough.c |  2 -
 test/io_uring_setup.c       | 83 ++-----------------------------------
 test/iopoll.c               |  2 -
 test/link_drain.c           |  2 +-
 test/multicqes_drain.c      | 27 +++++++-----
 test/nvme.h                 |  3 +-
 test/poll-link.c            |  2 +-
 test/poll-mshot-overflow.c  |  2 +-
 test/read-before-exit.c     |  2 +-
 test/ring-leak2.c           |  2 +-
 test/sq-poll-kthread.c      |  2 +-
 test/sqpoll-cancel-hang.c   |  2 +-
 test/symlink.c              |  3 +-
 test/timeout-new.c          | 12 ++++--
 26 files changed, 101 insertions(+), 142 deletions(-)


base-commit: 636b6bdaa8d84f1b5318e27d1e4ffa86361ae66d
-- 
Ammar Faizi


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH liburing v2 1/8] queue: Fix typo "entererd" -> "entered"
  2022-12-12 12:48 ` [PATCH] test/sendmsg_fs_cve: Fix the wrong SPDX-License-Identifier Ammar Faizi
  (?)
@ 2022-11-24 16:28 ` Ammar Faizi
  -1 siblings, 0 replies; 11+ messages in thread
From: Ammar Faizi @ 2022-11-24 16:28 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Ammar Faizi, Dylan Yudaken, Pavel Begunkov, Gilang Fachrezy,
	Muhammad Rizki, Alviro Iskandar Setiawan, VNLX Kernel Department,
	io-uring Mailing List, GNU/Weeb Mailing List

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

  s/entererd/entered/

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 src/queue.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/queue.c b/src/queue.c
index c06bcc3..feea0ad 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -81,7 +81,7 @@ static int _io_uring_get_cqe(struct io_uring *ring,
 		}
 		if (!cqe && !data->wait_nr && !data->submit) {
 			/*
-			 * If we already looped once, we already entererd
+			 * If we already looped once, we already entered
 			 * the kernel. Since there's nothing to submit or
 			 * wait for, don't keep retrying.
 			 */
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH liburing v2 2/8] queue: Mark `__io_uring_flush_sq()` as static
  2022-12-12 12:48 ` [PATCH] test/sendmsg_fs_cve: Fix the wrong SPDX-License-Identifier Ammar Faizi
  (?)
  (?)
@ 2022-11-24 16:28 ` Ammar Faizi
  -1 siblings, 0 replies; 11+ messages in thread
From: Ammar Faizi @ 2022-11-24 16:28 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Ammar Faizi, Dylan Yudaken, Pavel Begunkov, Gilang Fachrezy,
	Muhammad Rizki, Alviro Iskandar Setiawan, VNLX Kernel Department,
	io-uring Mailing List, GNU/Weeb Mailing List

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

This function is not exported, mark it as static. Clang says:

  queue.c:204:10: error: no previous prototype for function \
  '__io_uring_flush_sq' [-Werror,-Wmissing-prototypes] \
  unsigned __io_uring_flush_sq(struct io_uring *ring)
           ^
  queue.c:204:1: note: declare 'static' if the function is not intended \
  to be used outside of this translation unit \
  unsigned __io_uring_flush_sq(struct io_uring *ring)

Side note:
There was an attempt to export this function because it is used by
test/iopoll.c and test/io_uring_passthrough.c. But after a discussion
with Dylan and Jens, it's better to make a copy of this function for
those tests only instead of exporting it. Therefore, also create a
copy of this function in test/helpers.c.

Cc: Dylan Yudaken <dylany@meta.com>
Cc: Jens Axboe <axboe@kernel.dk>
Link: https://lore.kernel.org/io-uring/6491d7b5-0a52-e6d1-0f86-d36ec88bbc15@kernel.dk
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 src/queue.c                 |  2 +-
 test/helpers.c              | 33 +++++++++++++++++++++++++++++++++
 test/helpers.h              |  2 ++
 test/io_uring_passthrough.c |  2 --
 test/iopoll.c               |  2 --
 5 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/src/queue.c b/src/queue.c
index feea0ad..b784b10 100644
--- a/src/queue.c
+++ b/src/queue.c
@@ -201,7 +201,7 @@ again:
  * Sync internal state with kernel ring state on the SQ side. Returns the
  * number of pending items in the SQ ring, for the shared ring.
  */
-unsigned __io_uring_flush_sq(struct io_uring *ring)
+static unsigned __io_uring_flush_sq(struct io_uring *ring)
 {
 	struct io_uring_sq *sq = &ring->sq;
 	unsigned tail = sq->sqe_tail;
diff --git a/test/helpers.c b/test/helpers.c
index 8fb32b8..869e903 100644
--- a/test/helpers.c
+++ b/test/helpers.c
@@ -266,3 +266,36 @@ bool t_probe_defer_taskrun(void)
 	io_uring_queue_exit(&ring);
 	return true;
 }
+
+/*
+ * Sync internal state with kernel ring state on the SQ side. Returns the
+ * number of pending items in the SQ ring, for the shared ring.
+ */
+unsigned __io_uring_flush_sq(struct io_uring *ring)
+{
+	struct io_uring_sq *sq = &ring->sq;
+	unsigned tail = sq->sqe_tail;
+
+	if (sq->sqe_head != tail) {
+		sq->sqe_head = tail;
+		/*
+		 * Ensure kernel sees the SQE updates before the tail update.
+		 */
+		if (!(ring->flags & IORING_SETUP_SQPOLL))
+			IO_URING_WRITE_ONCE(*sq->ktail, tail);
+		else
+			io_uring_smp_store_release(sq->ktail, tail);
+	}
+	/*
+	 * This _may_ look problematic, as we're not supposed to be reading
+	 * SQ->head without acquire semantics. When we're in SQPOLL mode, the
+	 * kernel submitter could be updating this right now. For non-SQPOLL,
+	 * task itself does it, and there's no potential race. But even for
+	 * SQPOLL, the load is going to be potentially out-of-date the very
+	 * instant it's done, regardless or whether or not it's done
+	 * atomically. Worst case, we're going to be over-estimating what
+	 * we can submit. The point is, we need to be able to deal with this
+	 * situation regardless of any perceived atomicity.
+	 */
+	return tail - *sq->khead;
+}
diff --git a/test/helpers.h b/test/helpers.h
index 4375a9e..cb814ca 100644
--- a/test/helpers.h
+++ b/test/helpers.h
@@ -85,6 +85,8 @@ enum t_setup_ret t_register_buffers(struct io_uring *ring,
 
 bool t_probe_defer_taskrun(void);
 
+unsigned __io_uring_flush_sq(struct io_uring *ring);
+
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
 
 #ifdef __cplusplus
diff --git a/test/io_uring_passthrough.c b/test/io_uring_passthrough.c
index ee9ab87..d8468c5 100644
--- a/test/io_uring_passthrough.c
+++ b/test/io_uring_passthrough.c
@@ -268,8 +268,6 @@ static int test_io(const char *file, int tc, int read, int sqthread,
 	return ret;
 }
 
-extern unsigned __io_uring_flush_sq(struct io_uring *ring);
-
 /*
  * Send a passthrough command that nvme will fail during submission.
  * This comes handy for testing error handling.
diff --git a/test/iopoll.c b/test/iopoll.c
index 20f91c7..f8ab1f1 100644
--- a/test/iopoll.c
+++ b/test/iopoll.c
@@ -201,8 +201,6 @@ err:
 	return 1;
 }
 
-extern unsigned __io_uring_flush_sq(struct io_uring *ring);
-
 /*
  * if we are polling io_uring_submit needs to always enter the
  * kernel to fetch events
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH liburing v2 3/8] test/io_uring_setup: Remove unused functions
  2022-12-12 12:48 ` [PATCH] test/sendmsg_fs_cve: Fix the wrong SPDX-License-Identifier Ammar Faizi
                   ` (2 preceding siblings ...)
  (?)
@ 2022-11-24 16:28 ` Ammar Faizi
  -1 siblings, 0 replies; 11+ messages in thread
From: Ammar Faizi @ 2022-11-24 16:28 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Ammar Faizi, Dylan Yudaken, Pavel Begunkov, Gilang Fachrezy,
	Muhammad Rizki, Alviro Iskandar Setiawan, VNLX Kernel Department,
	io-uring Mailing List, GNU/Weeb Mailing List

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

When marking all internal functions as static after an attempt to
integrate `-Wmissing-prototypes` flag. Unused functions are found:

  io_uring_setup.c:22:14: error: unused function 'features_string' [-Werror,-Wunused-function]
  static char *features_string(struct io_uring_params *p)
               ^
  io_uring_setup.c:44:14: error: unused function 'flags_string' [-Werror,-Wunused-function]
  static char *flags_string(struct io_uring_params *p)
               ^
  io_uring_setup.c:83:15: error: unused function 'dump_resv' [-Werror,-Wunused-function]
  static char * dump_resv(struct io_uring_params *p)
                ^
  3 errors generated.
  make[1]: *** [Makefile:215: io_uring_setup.t] Error 1
  make[1]: *** Waiting for unfinished jobs....

Remove them.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/io_uring_setup.c | 82 ++-----------------------------------------
 1 file changed, 2 insertions(+), 80 deletions(-)

diff --git a/test/io_uring_setup.c b/test/io_uring_setup.c
index d945421..9e1a353 100644
--- a/test/io_uring_setup.c
+++ b/test/io_uring_setup.c
@@ -19,86 +19,9 @@
 
 #include "../syscall.h"
 
-char *features_string(struct io_uring_params *p)
-{
-	static char flagstr[64];
-
-	if (!p || !p->features)
-		return "none";
-
-	if (p->features & ~IORING_FEAT_SINGLE_MMAP) {
-		snprintf(flagstr, 64, "0x%.8x", p->features);
-		return flagstr;
-	}
-
-	if (p->features & IORING_FEAT_SINGLE_MMAP)
-		strncat(flagstr, "IORING_FEAT_SINGLE_MMAP", 64 - strlen(flagstr));
-
-	return flagstr;
-}
-
-/*
- * Attempt the call with the given args.  Return 0 when expect matches
- * the return value of the system call, 1 otherwise.
- */
-char *
-flags_string(struct io_uring_params *p)
-{
-	static char flagstr[64];
-	int add_pipe = 0;
-
-	memset(flagstr, 0, sizeof(flagstr));
-
-	if (!p || p->flags == 0)
-		return "none";
-
-	/*
-	 * If unsupported flags are present, just print the bitmask.
-	 */
-	if (p->flags & ~(IORING_SETUP_IOPOLL | IORING_SETUP_SQPOLL |
-			 IORING_SETUP_SQ_AFF)) {
-		snprintf(flagstr, 64, "0x%.8x", p->flags);
-		return flagstr;
-	}
-
-	if (p->flags & IORING_SETUP_IOPOLL) {
-		strncat(flagstr, "IORING_SETUP_IOPOLL", 64 - strlen(flagstr));
-		add_pipe = 1;
-	}
-	if (p->flags & IORING_SETUP_SQPOLL) {
-		if (add_pipe)
-			strncat(flagstr, "|", 64 - strlen(flagstr));
-		else
-			add_pipe = 1;
-		strncat(flagstr, "IORING_SETUP_SQPOLL", 64 - strlen(flagstr));
-	}
-	if (p->flags & IORING_SETUP_SQ_AFF) {
-		if (add_pipe)
-			strncat(flagstr, "|", 64 - strlen(flagstr));
-		strncat(flagstr, "IORING_SETUP_SQ_AFF", 64 - strlen(flagstr));
-	}
-
-	return flagstr;
-}
-
-char *
-dump_resv(struct io_uring_params *p)
-{
-	static char resvstr[4096];
-
-	if (!p)
-		return "";
-
-	sprintf(resvstr, "0x%.8x 0x%.8x 0x%.8x", p->resv[0],
-		p->resv[1], p->resv[2]);
-
-	return resvstr;
-}
-
 /* bogus: setup returns a valid fd on success... expect can't predict the
    fd we'll get, so this really only takes 1 parameter: error */
-int
-try_io_uring_setup(unsigned entries, struct io_uring_params *p, int expect)
+int try_io_uring_setup(unsigned entries, struct io_uring_params *p, int expect)
 {
 	int ret;
 
@@ -123,8 +46,7 @@ try_io_uring_setup(unsigned entries, struct io_uring_params *p, int expect)
 	return 0;
 }
 
-int
-main(int argc, char **argv)
+int main(int argc, char **argv)
 {
 	int fd;
 	unsigned int status = 0;
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH liburing v2 4/8] ucontext-cp: Remove an unused function
  2022-12-12 12:48 ` [PATCH] test/sendmsg_fs_cve: Fix the wrong SPDX-License-Identifier Ammar Faizi
                   ` (3 preceding siblings ...)
  (?)
@ 2022-11-24 16:28 ` Ammar Faizi
  -1 siblings, 0 replies; 11+ messages in thread
From: Ammar Faizi @ 2022-11-24 16:28 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Ammar Faizi, Dylan Yudaken, Pavel Begunkov, Gilang Fachrezy,
	Muhammad Rizki, Alviro Iskandar Setiawan, VNLX Kernel Department,
	io-uring Mailing List, GNU/Weeb Mailing List

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

When marking all internal functions as static after an attempt to
integrate `-Wmissing-prototypes` flag. An unused function is found:

  ucontext-cp.c:71:12: error: ‘await_poll’ defined but not used [-Werror=unused-function]
     71 | static int await_poll(async_context *pctx, int fd, short poll_mask)
        |            ^~~~~~~~~~
  cc1: all warnings being treated as errors
  make[1]: *** [Makefile:36: ucontext-cp] Error 1

Remove it.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 examples/ucontext-cp.c | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/examples/ucontext-cp.c b/examples/ucontext-cp.c
index 281013f..ed3b342 100644
--- a/examples/ucontext-cp.c
+++ b/examples/ucontext-cp.c
@@ -68,23 +68,8 @@ DEFINE_AWAIT_OP(readv)
 DEFINE_AWAIT_OP(writev)
 #undef DEFINE_AWAIT_OP
 
-int await_poll(async_context *pctx, int fd, short poll_mask) {
-	struct io_uring_sqe *sqe = io_uring_get_sqe(pctx->ring);
-	struct io_uring_cqe *cqe;
-	if (!sqe)
-		return -1;
-
-	io_uring_prep_poll_add(sqe, fd, poll_mask);
-	io_uring_sqe_set_data(sqe, pctx);
-	swapcontext(&pctx->ctx_fnew, &pctx->ctx_main);
-	io_uring_peek_cqe(pctx->ring, &cqe);
-	assert(cqe);
-	io_uring_cqe_seen(pctx->ring, cqe);
-
-	return cqe->res;
-}
-
-int await_delay(async_context *pctx, time_t seconds) {
+int await_delay(async_context *pctx, time_t seconds)
+{
 	struct io_uring_sqe *sqe = io_uring_get_sqe(pctx->ring);
 	struct io_uring_cqe *cqe;
 	struct __kernel_timespec ts = {
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH liburing v2 5/8] tests: Mark non-exported functions as static
  2022-12-12 12:48 ` [PATCH] test/sendmsg_fs_cve: Fix the wrong SPDX-License-Identifier Ammar Faizi
                   ` (4 preceding siblings ...)
  (?)
@ 2022-11-24 16:28 ` Ammar Faizi
  -1 siblings, 0 replies; 11+ messages in thread
From: Ammar Faizi @ 2022-11-24 16:28 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Ammar Faizi, Dylan Yudaken, Pavel Begunkov, Gilang Fachrezy,
	Muhammad Rizki, Alviro Iskandar Setiawan, VNLX Kernel Department,
	io-uring Mailing List, GNU/Weeb Mailing List

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Functions that are not used outside the translation unit should be
static.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/accept-link.c         |  2 +-
 test/accept-reuse.c        |  9 ++++-----
 test/ce593a6c480a.c        |  4 ++--
 test/defer-taskrun.c       |  2 +-
 test/exit-no-cleanup.c     |  2 +-
 test/hardlink.c            |  2 +-
 test/io_uring_setup.c      |  3 ++-
 test/link_drain.c          |  2 +-
 test/multicqes_drain.c     | 27 ++++++++++++++++-----------
 test/nvme.h                |  3 ++-
 test/poll-link.c           |  2 +-
 test/poll-mshot-overflow.c |  2 +-
 test/read-before-exit.c    |  2 +-
 test/ring-leak2.c          |  2 +-
 test/sq-poll-kthread.c     |  2 +-
 test/sqpoll-cancel-hang.c  |  2 +-
 test/symlink.c             |  3 ++-
 test/timeout-new.c         | 12 ++++++++----
 18 files changed, 47 insertions(+), 36 deletions(-)

diff --git a/test/accept-link.c b/test/accept-link.c
index d0dc73b..4d42f15 100644
--- a/test/accept-link.c
+++ b/test/accept-link.c
@@ -77,7 +77,7 @@ static void *send_thread(void *arg)
 	return NULL;
 }
 
-void *recv_thread(void *arg)
+static void *recv_thread(void *arg)
 {
 	struct data *data = arg;
 	struct io_uring ring;
diff --git a/test/accept-reuse.c b/test/accept-reuse.c
index 66eded2..7df5e56 100644
--- a/test/accept-reuse.c
+++ b/test/accept-reuse.c
@@ -13,15 +13,14 @@
 
 struct io_uring io_uring;
 
-int sys_io_uring_enter(const int fd,
-		       const unsigned to_submit,
-		       const unsigned min_complete,
-		       const unsigned flags, sigset_t * const sig)
+static int sys_io_uring_enter(const int fd, const unsigned to_submit,
+			      const unsigned min_complete,
+			      const unsigned flags, sigset_t * const sig)
 {
 	return __sys_io_uring_enter(fd, to_submit, min_complete, flags, sig);
 }
 
-int submit_sqe(void)
+static int submit_sqe(void)
 {
 	struct io_uring_sq *sq = &io_uring.sq;
 	const unsigned tail = *sq->ktail;
diff --git a/test/ce593a6c480a.c b/test/ce593a6c480a.c
index 4c71fbc..9fa74a9 100644
--- a/test/ce593a6c480a.c
+++ b/test/ce593a6c480a.c
@@ -15,7 +15,7 @@
 
 static int use_sqpoll = 0;
 
-void notify_fd(int fd)
+static void notify_fd(int fd)
 {
 	char buf[8] = {0, 0, 0, 0, 0, 0, 1};
 	int ret;
@@ -25,7 +25,7 @@ void notify_fd(int fd)
 		perror("write");
 }
 
-void *delay_set_fd_from_thread(void *data)
+static void *delay_set_fd_from_thread(void *data)
 {
 	int fd = (intptr_t) data;
 
diff --git a/test/defer-taskrun.c b/test/defer-taskrun.c
index 9283f28..624285e 100644
--- a/test/defer-taskrun.c
+++ b/test/defer-taskrun.c
@@ -119,7 +119,7 @@ struct thread_data {
 	char buff[8];
 };
 
-void *thread(void *t)
+static void *thread(void *t)
 {
 	struct thread_data *td = t;
 
diff --git a/test/exit-no-cleanup.c b/test/exit-no-cleanup.c
index 4b8574d..aadef8c 100644
--- a/test/exit-no-cleanup.c
+++ b/test/exit-no-cleanup.c
@@ -26,7 +26,7 @@ static pthread_barrier_t init_barrier;
 static int sleep_fd, notify_fd;
 static sem_t sem;
 
-void *thread_func(void *arg)
+static void *thread_func(void *arg)
 {
 	struct io_uring ring;
 	int res;
diff --git a/test/hardlink.c b/test/hardlink.c
index aeb9ac6..2b5140e 100644
--- a/test/hardlink.c
+++ b/test/hardlink.c
@@ -44,7 +44,7 @@ err:
 	return 1;
 }
 
-int files_linked_ok(const char* fn1, const char *fn2)
+static int files_linked_ok(const char* fn1, const char *fn2)
 {
 	struct stat s1, s2;
 
diff --git a/test/io_uring_setup.c b/test/io_uring_setup.c
index 9e1a353..2e95418 100644
--- a/test/io_uring_setup.c
+++ b/test/io_uring_setup.c
@@ -21,7 +21,8 @@
 
 /* bogus: setup returns a valid fd on success... expect can't predict the
    fd we'll get, so this really only takes 1 parameter: error */
-int try_io_uring_setup(unsigned entries, struct io_uring_params *p, int expect)
+static int try_io_uring_setup(unsigned entries, struct io_uring_params *p,
+			      int expect)
 {
 	int ret;
 
diff --git a/test/link_drain.c b/test/link_drain.c
index b95168d..1a50c10 100644
--- a/test/link_drain.c
+++ b/test/link_drain.c
@@ -96,7 +96,7 @@ err:
 	return 1;
 }
 
-int test_link_drain_multi(struct io_uring *ring)
+static int test_link_drain_multi(struct io_uring *ring)
 {
 	struct io_uring_cqe *cqe;
 	struct io_uring_sqe *sqe[9];
diff --git a/test/multicqes_drain.c b/test/multicqes_drain.c
index f95c438..3755bee 100644
--- a/test/multicqes_drain.c
+++ b/test/multicqes_drain.c
@@ -42,12 +42,16 @@ struct sqe_info {
  * up an entry in multi_sqes when form a cancellation sqe.
  * multi_cap: limitation of number of multishot sqes
  */
-const unsigned sqe_flags[4] = {0, IOSQE_IO_LINK, IOSQE_IO_DRAIN,
-	IOSQE_IO_LINK | IOSQE_IO_DRAIN};
-int multi_sqes[max_entry], cnt = 0;
-int multi_cap = max_entry / 5;
+static const unsigned sqe_flags[4] = {
+	0,
+	IOSQE_IO_LINK,
+	IOSQE_IO_DRAIN,
+	IOSQE_IO_LINK | IOSQE_IO_DRAIN
+};
+static int multi_sqes[max_entry], cnt = 0;
+static int multi_cap = max_entry / 5;
 
-int write_pipe(int pipe, char *str)
+static int write_pipe(int pipe, char *str)
 {
 	int ret;
 	do {
@@ -57,7 +61,7 @@ int write_pipe(int pipe, char *str)
 	return ret;
 }
 
-void read_pipe(int pipe)
+static void read_pipe(int pipe)
 {
 	char str[4] = {0};
 	int ret;
@@ -67,7 +71,7 @@ void read_pipe(int pipe)
 		perror("read");
 }
 
-int trigger_event(int p[])
+static int trigger_event(int p[])
 {
 	int ret;
 	if ((ret = write_pipe(p[1], "foo")) != 3) {
@@ -78,7 +82,8 @@ int trigger_event(int p[])
 	return 0;
 }
 
-void io_uring_sqe_prep(int op, struct io_uring_sqe *sqe, unsigned sqe_flags, int arg)
+static void io_uring_sqe_prep(int op, struct io_uring_sqe *sqe,
+			      unsigned sqe_flags, int arg)
 {
 	switch (op) {
 		case multi:
@@ -98,7 +103,7 @@ void io_uring_sqe_prep(int op, struct io_uring_sqe *sqe, unsigned sqe_flags, int
 	sqe->flags = sqe_flags;
 }
 
-__u8 generate_flags(int sqe_op)
+static __u8 generate_flags(int sqe_op)
 {
 	__u8 flags = 0;
 	/*
@@ -136,7 +141,7 @@ __u8 generate_flags(int sqe_op)
  * - ensure number of multishot sqes doesn't exceed multi_cap
  * - don't generate multishot sqes after high watermark
  */
-int generate_opcode(int i, int pre_flags)
+static int generate_opcode(int i, int pre_flags)
 {
 	int sqe_op;
 	int high_watermark = max_entry - max_entry / 5;
@@ -163,7 +168,7 @@ static inline void add_multishot_sqe(int index)
 	multi_sqes[cnt++] = index;
 }
 
-int remove_multishot_sqe()
+static int remove_multishot_sqe(void)
 {
 	int ret;
 
diff --git a/test/nvme.h b/test/nvme.h
index 14dc338..52f4172 100644
--- a/test/nvme.h
+++ b/test/nvme.h
@@ -120,7 +120,8 @@ static inline int ilog2(uint32_t i)
 	return log;
 }
 
-int nvme_get_info(const char *file)
+__attribute__((__unused__))
+static int nvme_get_info(const char *file)
 {
 	struct nvme_id_ns ns;
 	int fd, err;
diff --git a/test/poll-link.c b/test/poll-link.c
index 39b48f5..b94f954 100644
--- a/test/poll-link.c
+++ b/test/poll-link.c
@@ -71,7 +71,7 @@ static void *send_thread(void *arg)
 	return 0;
 }
 
-void *recv_thread(void *arg)
+static void *recv_thread(void *arg)
 {
 	struct sockaddr_in addr = { };
 	struct data *data = arg;
diff --git a/test/poll-mshot-overflow.c b/test/poll-mshot-overflow.c
index 0dc8ee5..041e872 100644
--- a/test/poll-mshot-overflow.c
+++ b/test/poll-mshot-overflow.c
@@ -12,7 +12,7 @@
 #include "liburing.h"
 #include "helpers.h"
 
-int check_final_cqe(struct io_uring *ring)
+static int check_final_cqe(struct io_uring *ring)
 {
 	struct io_uring_cqe *cqe;
 	int count = 0;
diff --git a/test/read-before-exit.c b/test/read-before-exit.c
index be36bd4..d57cdd2 100644
--- a/test/read-before-exit.c
+++ b/test/read-before-exit.c
@@ -22,7 +22,7 @@ struct data {
         uint64_t buf2;
 };
 
-void *submit(void *data)
+static void *submit(void *data)
 {
 	struct io_uring_sqe *sqe;
 	struct data *d = data;
diff --git a/test/ring-leak2.c b/test/ring-leak2.c
index a8c03fe..6e76717 100644
--- a/test/ring-leak2.c
+++ b/test/ring-leak2.c
@@ -46,7 +46,7 @@ static struct io_uring *client_ring;
 
 static int client_eventfd = -1;
 
-int setup_io_uring(struct io_uring *ring)
+static int setup_io_uring(struct io_uring *ring)
 {
 	struct io_uring_params p = { };
 	int ret;
diff --git a/test/sq-poll-kthread.c b/test/sq-poll-kthread.c
index 3f4a07b..bf5122a 100644
--- a/test/sq-poll-kthread.c
+++ b/test/sq-poll-kthread.c
@@ -117,7 +117,7 @@ err_pipe:
 	return ret;
 }
 
-int test_sq_poll_kthread_stopped(bool do_exit)
+static int test_sq_poll_kthread_stopped(bool do_exit)
 {
 	pid_t pid;
 	int status = 0;
diff --git a/test/sqpoll-cancel-hang.c b/test/sqpoll-cancel-hang.c
index ef62272..cd1c309 100644
--- a/test/sqpoll-cancel-hang.c
+++ b/test/sqpoll-cancel-hang.c
@@ -92,7 +92,7 @@ SIZEOF_IO_URING_CQE + 63) & ~63;
 }
 
 
-void trigger_bug(void)
+static void trigger_bug(void)
 {
     intptr_t res = 0;
     *(uint32_t*)0x20000204 = 0;
diff --git a/test/symlink.c b/test/symlink.c
index cf4aa96..7edb514 100644
--- a/test/symlink.c
+++ b/test/symlink.c
@@ -43,7 +43,8 @@ err:
 	return 1;
 }
 
-int test_link_contents(const char* linkname, const char *expected_contents)
+static int test_link_contents(const char* linkname,
+			      const char *expected_contents)
 {
 	char buf[128];
 	int ret = readlink(linkname, buf, 127);
diff --git a/test/timeout-new.c b/test/timeout-new.c
index 6efcfb4..8640678 100644
--- a/test/timeout-new.c
+++ b/test/timeout-new.c
@@ -111,7 +111,8 @@ static int test_return_after_timeout(struct io_uring *ring)
 	return 0;
 }
 
-int __reap_thread_fn(void *data) {
+static int __reap_thread_fn(void *data)
+{
 	struct io_uring *ring = (struct io_uring *)data;
 	struct io_uring_cqe *cqe;
 	struct __kernel_timespec ts;
@@ -123,12 +124,14 @@ int __reap_thread_fn(void *data) {
 	return io_uring_wait_cqe_timeout(ring, &cqe, &ts);
 }
 
-void *reap_thread_fn0(void *data) {
+static void *reap_thread_fn0(void *data)
+{
 	thread_ret0 = __reap_thread_fn(data);
 	return NULL;
 }
 
-void *reap_thread_fn1(void *data) {
+static void *reap_thread_fn1(void *data)
+{
 	thread_ret1 = __reap_thread_fn(data);
 	return NULL;
 }
@@ -137,7 +140,8 @@ void *reap_thread_fn1(void *data) {
  * This is to test issuing a sqe in main thread and reaping it in two child-thread
  * at the same time. To see if timeout feature works or not.
  */
-int test_multi_threads_timeout() {
+static int test_multi_threads_timeout(void)
+{
 	struct io_uring ring;
 	int ret;
 	bool both_wait = false;
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH liburing v2 6/8] ucontext-cp: Mark a non-exported function as static
  2022-12-12 12:48 ` [PATCH] test/sendmsg_fs_cve: Fix the wrong SPDX-License-Identifier Ammar Faizi
                   ` (5 preceding siblings ...)
  (?)
@ 2022-11-24 16:28 ` Ammar Faizi
  -1 siblings, 0 replies; 11+ messages in thread
From: Ammar Faizi @ 2022-11-24 16:28 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Ammar Faizi, Dylan Yudaken, Pavel Begunkov, Gilang Fachrezy,
	Muhammad Rizki, Alviro Iskandar Setiawan, VNLX Kernel Department,
	io-uring Mailing List, GNU/Weeb Mailing List

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Functions that are not used outside the translation unit should be
static.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 examples/ucontext-cp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/examples/ucontext-cp.c b/examples/ucontext-cp.c
index ed3b342..d17aae7 100644
--- a/examples/ucontext-cp.c
+++ b/examples/ucontext-cp.c
@@ -68,7 +68,7 @@ DEFINE_AWAIT_OP(readv)
 DEFINE_AWAIT_OP(writev)
 #undef DEFINE_AWAIT_OP
 
-int await_delay(async_context *pctx, time_t seconds)
+static int await_delay(async_context *pctx, time_t seconds)
 {
 	struct io_uring_sqe *sqe = io_uring_get_sqe(pctx->ring);
 	struct io_uring_cqe *cqe;
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH liburing v2 7/8] test/Makefile: Omit `-Wmissing-prototypes` from the C++ compiler flags
  2022-12-12 12:48 ` [PATCH] test/sendmsg_fs_cve: Fix the wrong SPDX-License-Identifier Ammar Faizi
                   ` (6 preceding siblings ...)
  (?)
@ 2022-11-24 16:29 ` Ammar Faizi
  -1 siblings, 0 replies; 11+ messages in thread
From: Ammar Faizi @ 2022-11-24 16:29 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Ammar Faizi, Dylan Yudaken, Pavel Begunkov, Gilang Fachrezy,
	Muhammad Rizki, Alviro Iskandar Setiawan, VNLX Kernel Department,
	io-uring Mailing List, GNU/Weeb Mailing List

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

This is a preparation patch to integrate -Wmissing-prototypes to the
CI test robot. Clang++ is not happy with -Wmissing-prototypes:

    cc1plus: warning: command-line option '-Wmissing-prototypes' \
    is valid for C/ObjC but not for C++

Omit this flag when we are compiling a C++ source file.

Using -Wmissing-prototypes ensures we mark functions and variables as
static if we don't use them outside the translation unit.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/Makefile | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/test/Makefile b/test/Makefile
index 8ad9964..87b1fe1 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -214,8 +214,17 @@ helpers.o: helpers.c
 %.t: %.c $(helpers) helpers.h ../src/liburing.a
 	$(QUIET_CC)$(CC) $(CPPFLAGS) $(CFLAGS) -o $@ $< $(helpers) $(LDFLAGS)
 
+#
+# Clang++ is not happy with -Wmissing-prototypes:
+#
+#   cc1plus: warning: command-line option '-Wmissing-prototypes' \
+#   is valid for C/ObjC but not for C++
+#
 %.t: %.cc $(helpers) helpers.h ../src/liburing.a
-	$(QUIET_CXX)$(CXX) $(CPPFLAGS) $(CXXFLAGS) -o $@ $< $(helpers) $(LDFLAGS)
+	$(QUIET_CXX)$(CXX) \
+	$(patsubst -Wmissing-prototypes,,$(CPPFLAGS)) \
+	$(patsubst -Wmissing-prototypes,,$(CXXFLAGS)) \
+	-o $@ $< $(helpers) $(LDFLAGS)
 
 
 install: $(test_targets) runtests.sh runtests-loop.sh
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH liburing v2 8/8] github: Add `-Wmissing-prototypes` for GitHub CI bot
  2022-12-12 12:48 ` [PATCH] test/sendmsg_fs_cve: Fix the wrong SPDX-License-Identifier Ammar Faizi
                   ` (7 preceding siblings ...)
  (?)
@ 2022-11-24 16:29 ` Ammar Faizi
  -1 siblings, 0 replies; 11+ messages in thread
From: Ammar Faizi @ 2022-11-24 16:29 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Ammar Faizi, Dylan Yudaken, Pavel Begunkov, Gilang Fachrezy,
	Muhammad Rizki, Alviro Iskandar Setiawan, VNLX Kernel Department,
	io-uring Mailing List, GNU/Weeb Mailing List

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Using -Wmissing-prototypes ensures we mark functions and variables as
static if we don't use them outside the translation unit.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 .github/workflows/build.yml | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index b0e669d..4c0bd26 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -26,7 +26,8 @@ jobs:
             cxx_pkg: clang
             cc: clang
             cxx: clang++
-            extra_flags: -Wshorten-64-to-32
+            liburing_extra_flags: -Wshorten-64-to-32
+            extra_flags: -Wmissing-prototypes
 
           # x86 (32-bit) gcc
           - arch: i686
@@ -85,10 +86,10 @@ jobs:
             cxx: mips-linux-gnu-g++
 
     env:
-      FLAGS: -g -O3 -Wall -Wextra -Werror
+      FLAGS: -g -O3 -Wall -Wextra -Werror ${{matrix.extra_flags}}
 
       # Flags for building sources in src/ dir only.
-      LIBURING_CFLAGS: ${{matrix.extra_flags}}
+      LIBURING_CFLAGS: ${{matrix.liburing_extra_flags}}
 
     steps:
     - name: Checkout source
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static
  2022-12-12 12:48 ` [PATCH] test/sendmsg_fs_cve: Fix the wrong SPDX-License-Identifier Ammar Faizi
                   ` (8 preceding siblings ...)
  (?)
@ 2022-11-28 14:15 ` Jens Axboe
  -1 siblings, 0 replies; 11+ messages in thread
From: Jens Axboe @ 2022-11-28 14:15 UTC (permalink / raw)
  To: Ammar Faizi
  Cc: Gilang Fachrezy, Pavel Begunkov, io-uring Mailing List,
	VNLX Kernel Department, Dylan Yudaken, GNU/Weeb Mailing List,
	Muhammad Rizki, Alviro Iskandar Setiawan

On Thu, 24 Nov 2022 23:28:53 +0700, Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> 
> Hi Jens,
> 
> This is a v2 revision.
> 
> This series is a -Wmissing-prototypes enforcement. -Wmissing-prototypes
> is a clang C compiler flag that warns us if we have functions or
> variables that are not used outisde the translation unit, but not marked
> as static.
> 
> [...]

Applied, thanks!

[1/8] queue: Fix typo "entererd" -> "entered"
      (no commit info)
[2/8] queue: Mark `__io_uring_flush_sq()` as static
      (no commit info)
[3/8] test/io_uring_setup: Remove unused functions
      (no commit info)
[4/8] ucontext-cp: Remove an unused function
      (no commit info)
[5/8] tests: Mark non-exported functions as static
      (no commit info)
[6/8] ucontext-cp: Mark a non-exported function as static
      (no commit info)
[7/8] test/Makefile: Omit `-Wmissing-prototypes` from the C++ compiler flags
      (no commit info)
[8/8] github: Add `-Wmissing-prototypes` for GitHub CI bot
      (no commit info)

Best regards,
-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH] test/sendmsg_fs_cve: Fix the wrong SPDX-License-Identifier
@ 2022-12-12 12:48 ` Ammar Faizi
  0 siblings, 0 replies; 11+ messages in thread
From: Ammar Faizi @ 2022-12-12 12:48 UTC (permalink / raw)
  To: Jens Axboe
  Cc: io-uring Mailing List, Ammar Faizi, Nicolai Stange, Yuriy Chernyshov

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

This test program's license is GPL-2.0-or-later, but I put the wrong
SPDX-License-Identifier in commit:

   d36db0b72b9 ("test: Add missing SPDX-License-Identifier")

Fix it.

Cc: Nicolai Stange <nstange@suse.de>
Reported-by: Yuriy Chernyshov <georgthegreat@gmail.com>
Fixes: https://github.com/axboe/liburing/issues/753
Fixes: d36db0b72b9399623dee12b61069845d8e1bfa05 ("test: Add missing SPDX-License-Identifier")
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 test/sendmsg_fs_cve.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/sendmsg_fs_cve.c b/test/sendmsg_fs_cve.c
index 2ce3114..9e444d7 100644
--- a/test/sendmsg_fs_cve.c
+++ b/test/sendmsg_fs_cve.c
@@ -1,11 +1,11 @@
-/* SPDX-License-Identifier: MIT */
+/* SPDX-License-Identifier: GPL-2.0-or-later */
 /*
  * repro-CVE-2020-29373 -- Reproducer for CVE-2020-29373.
  *
  * Copyright (c) 2021 SUSE
  * Author: Nicolai Stange <nstange@suse.de>
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
  * as published by the Free Software Foundation; either version 2
  * of the License, or (at your option) any later version.

base-commit: 5f0cf68fc2cd28617a86976fc906447f737086d5
-- 
Ammar Faizi


^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-12-12 12:49 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-24 16:28 [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static Ammar Faizi
2022-12-12 12:48 ` [PATCH] test/sendmsg_fs_cve: Fix the wrong SPDX-License-Identifier Ammar Faizi
2022-11-24 16:28 ` [PATCH liburing v2 1/8] queue: Fix typo "entererd" -> "entered" Ammar Faizi
2022-11-24 16:28 ` [PATCH liburing v2 2/8] queue: Mark `__io_uring_flush_sq()` as static Ammar Faizi
2022-11-24 16:28 ` [PATCH liburing v2 3/8] test/io_uring_setup: Remove unused functions Ammar Faizi
2022-11-24 16:28 ` [PATCH liburing v2 4/8] ucontext-cp: Remove an unused function Ammar Faizi
2022-11-24 16:28 ` [PATCH liburing v2 5/8] tests: Mark non-exported functions as static Ammar Faizi
2022-11-24 16:28 ` [PATCH liburing v2 6/8] ucontext-cp: Mark a non-exported function " Ammar Faizi
2022-11-24 16:29 ` [PATCH liburing v2 7/8] test/Makefile: Omit `-Wmissing-prototypes` from the C++ compiler flags Ammar Faizi
2022-11-24 16:29 ` [PATCH liburing v2 8/8] github: Add `-Wmissing-prototypes` for GitHub CI bot Ammar Faizi
2022-11-28 14:15 ` [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.