GNU/Weeb Mailing List <[email protected]>
 help / color / mirror / Atom feed
* [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static
@ 2022-11-24 16:28 Ammar Faizi
  2022-11-24 16:28 ` [PATCH liburing v2 1/8] queue: Fix typo "entererd" -> "entered" Ammar Faizi
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ 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 <[email protected]>

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 <[email protected]>
---

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] 10+ messages in thread

* [PATCH liburing v2 1/8] queue: Fix typo "entererd" -> "entered"
  2022-11-24 16:28 [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static Ammar Faizi
@ 2022-11-24 16:28 ` Ammar Faizi
  2022-11-24 16:28 ` [PATCH liburing v2 2/8] queue: Mark `__io_uring_flush_sq()` as static Ammar Faizi
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ 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 <[email protected]>

  s/entererd/entered/

Signed-off-by: Ammar Faizi <[email protected]>
---
 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] 10+ messages in thread

* [PATCH liburing v2 2/8] queue: Mark `__io_uring_flush_sq()` as static
  2022-11-24 16:28 [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static Ammar Faizi
  2022-11-24 16:28 ` [PATCH liburing v2 1/8] queue: Fix typo "entererd" -> "entered" Ammar Faizi
@ 2022-11-24 16:28 ` Ammar Faizi
  2022-11-24 16:28 ` [PATCH liburing v2 3/8] test/io_uring_setup: Remove unused functions Ammar Faizi
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ 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 <[email protected]>

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 <[email protected]>
Cc: Jens Axboe <[email protected]>
Link: https://lore.kernel.org/io-uring/[email protected]
Signed-off-by: Ammar Faizi <[email protected]>
---
 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] 10+ messages in thread

* [PATCH liburing v2 3/8] test/io_uring_setup: Remove unused functions
  2022-11-24 16:28 [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static 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 ` Ammar Faizi
  2022-11-24 16:28 ` [PATCH liburing v2 4/8] ucontext-cp: Remove an unused function Ammar Faizi
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ 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 <[email protected]>

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 <[email protected]>
---
 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] 10+ messages in thread

* [PATCH liburing v2 4/8] ucontext-cp: Remove an unused function
  2022-11-24 16:28 [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static Ammar Faizi
                   ` (2 preceding siblings ...)
  2022-11-24 16:28 ` [PATCH liburing v2 3/8] test/io_uring_setup: Remove unused functions Ammar Faizi
@ 2022-11-24 16:28 ` Ammar Faizi
  2022-11-24 16:28 ` [PATCH liburing v2 5/8] tests: Mark non-exported functions as static Ammar Faizi
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ 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 <[email protected]>

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 <[email protected]>
---
 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] 10+ messages in thread

* [PATCH liburing v2 5/8] tests: Mark non-exported functions as static
  2022-11-24 16:28 [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static Ammar Faizi
                   ` (3 preceding siblings ...)
  2022-11-24 16:28 ` [PATCH liburing v2 4/8] ucontext-cp: Remove an unused function Ammar Faizi
@ 2022-11-24 16:28 ` Ammar Faizi
  2022-11-24 16:28 ` [PATCH liburing v2 6/8] ucontext-cp: Mark a non-exported function " Ammar Faizi
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ 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 <[email protected]>

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

Signed-off-by: Ammar Faizi <[email protected]>
---
 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] 10+ messages in thread

* [PATCH liburing v2 6/8] ucontext-cp: Mark a non-exported function as static
  2022-11-24 16:28 [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static Ammar Faizi
                   ` (4 preceding siblings ...)
  2022-11-24 16:28 ` [PATCH liburing v2 5/8] tests: Mark non-exported functions as static Ammar Faizi
@ 2022-11-24 16:28 ` Ammar Faizi
  2022-11-24 16:29 ` [PATCH liburing v2 7/8] test/Makefile: Omit `-Wmissing-prototypes` from the C++ compiler flags Ammar Faizi
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ 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 <[email protected]>

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

Signed-off-by: Ammar Faizi <[email protected]>
---
 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] 10+ messages in thread

* [PATCH liburing v2 7/8] test/Makefile: Omit `-Wmissing-prototypes` from the C++ compiler flags
  2022-11-24 16:28 [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static Ammar Faizi
                   ` (5 preceding siblings ...)
  2022-11-24 16:28 ` [PATCH liburing v2 6/8] ucontext-cp: Mark a non-exported function " Ammar Faizi
@ 2022-11-24 16:29 ` 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
  8 siblings, 0 replies; 10+ 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 <[email protected]>

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 <[email protected]>
---
 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] 10+ messages in thread

* [PATCH liburing v2 8/8] github: Add `-Wmissing-prototypes` for GitHub CI bot
  2022-11-24 16:28 [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static Ammar Faizi
                   ` (6 preceding siblings ...)
  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 ` Ammar Faizi
  2022-11-28 14:15 ` [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static Jens Axboe
  8 siblings, 0 replies; 10+ 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 <[email protected]>

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 <[email protected]>
---
 .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] 10+ messages in thread

* Re: [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static
  2022-11-24 16:28 [PATCH liburing v2 0/8] Ensure we mark non-exported functions and variables as static Ammar Faizi
                   ` (7 preceding siblings ...)
  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 ` Jens Axboe
  8 siblings, 0 replies; 10+ 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 <[email protected]>
> 
> 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] 10+ messages in thread

end of thread, other threads:[~2022-11-28 14:15 UTC | newest]

Thread overview: 10+ 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-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox