[PATCH liburing v1 0/7] Ensure we mark internal functions and variables as static

[PATCH liburing v1 0/7] Ensure we mark internal functions and variables as static

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Hi Jens,

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. This enforcement is good because it hints the compiler to do
escape analysis and optimization better.

There are 7 patches in this series.

- Patch 1 is a core library change. Export __io_uring_flush_sq().
- Patch 2 to 6 are cleanups preparation before enforcing
  -Wmissing-prototypes.
- Patch 7 is to add `-Wmissing-prototypes` for GitHub CI bot.

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

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

Ammar Faizi (7):
  liburing.h: Export `__io_uring_flush_sq()` function
  test/io_uring_setup: Remove unused functions
  ucontext-cp: Remove an unused function
  tests: Mark internal functions as static
  ucontext-cp: Mark internal functions 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/include/liburing.h      |  1 +
 src/liburing.map            |  5 +++
 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/io_uring_setup.c       | 83 ++-----------------------------------
 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 ++++--
 23 files changed, 70 insertions(+), 136 deletions(-)


base-commit: b90a28636e5b5efe6dc1383acc90aec61814d9ba
-- 
Ammar Faizi

[PATCH liburing v1 1/7] liburing.h: Export `__io_uring_flush_sq()` function

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

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)

This function is used by test/iopoll.c, therefore, it can't be static.
Export it.

Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 src/include/liburing.h | 1 +
 src/liburing.map       | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/src/include/liburing.h b/src/include/liburing.h
index 12a703f..c1d8edb 100644
--- a/src/include/liburing.h
+++ b/src/include/liburing.h
@@ -237,6 +237,7 @@ int io_uring_register_file_alloc_range(struct io_uring *ring,
 
 int io_uring_get_events(struct io_uring *ring);
 int io_uring_submit_and_get_events(struct io_uring *ring);
+unsigned __io_uring_flush_sq(struct io_uring *ring);
 
 /*
  * io_uring syscalls.
diff --git a/src/liburing.map b/src/liburing.map
index 06c64f8..6b2f4b2 100644
--- a/src/liburing.map
+++ b/src/liburing.map
@@ -67,3 +67,8 @@ LIBURING_2.3 {
 		io_uring_get_events;
 		io_uring_submit_and_get_events;
 } LIBURING_2.2;
+
+LIBURING_2.4 {
+	global:
+		__io_uring_flush_sq;
+} LIBURING_2.3;
-- 
Ammar Faizi

[PATCH liburing v1 2/7] test/io_uring_setup: Remove unused functions

[Ammar Faizi]

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

[PATCH liburing v1 3/7] ucontext-cp: Remove an unused function

[Ammar Faizi]

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

[PATCH liburing v1 4/7] tests: Mark internal functions as static

[Ammar Faizi]

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 431a337..97d1cbe 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

[PATCH liburing v1 5/7] ucontext-cp: Mark internal functions as static

[Ammar Faizi]

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

[PATCH liburing v1 6/7] test/Makefile: Omit `-Wmissing-prototypes` from the C++ compiler flags

[Ammar Faizi]

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

[PATCH liburing v1 7/7] github: Add `-Wmissing-prototypes` for GitHub CI bot

[Ammar Faizi]

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. This
enforcement is good because it hints the compiler to do escape analysis
and optimization better.

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

Re: [PATCH liburing v1 1/7] liburing.h: Export `__io_uring_flush_sq()` function

[Dylan Yudaken]

On Thu, 2022-11-24 at 15:00 +0700, Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> 
> 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)
> 
> This function is used by test/iopoll.c, therefore, it can't be
> static.
> Export it.
> 
> Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> ---
>  src/include/liburing.h | 1 +
>  src/liburing.map       | 5 +++++
>  2 files changed, 6 insertions(+)
> 

I think changing the tests to use the public API is probably better
than exporting this function. I don't believe it has much general use?

Dylan

Re: [PATCH liburing v1 1/7] liburing.h: Export `__io_uring_flush_sq()` function

[Ammar Faizi]

On 11/24/22 5:14 PM, Dylan Yudaken wrote:
> I think changing the tests to use the public API is probably better
> than exporting this function. I don't believe it has much general use?

But there is no public API that does the same thing. I'll mark it
as static and create a copy of that function in iopoll.c (in v2).

Something like this, what do you think?

  src/queue.c   |  2 +-
  test/iopoll.c | 33 ++++++++++++++++++++++++++++++++-
  2 files changed, 33 insertions(+), 2 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/iopoll.c b/test/iopoll.c
index 20f91c7..5edd5c3 100644
--- a/test/iopoll.c
+++ b/test/iopoll.c
@@ -201,7 +201,38 @@ err:
  	return 1;
  }
  
-extern unsigned __io_uring_flush_sq(struct io_uring *ring);
+/*
+ * 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.
+ */
+static 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;
+}
  
  /*
   * if we are polling io_uring_submit needs to always enter the


-- 
Ammar Faizi

Re: [PATCH liburing v1 1/7] liburing.h: Export `__io_uring_flush_sq()` function

[Jens Axboe]

On 11/24/22 4:47 AM, Ammar Faizi wrote:
> On 11/24/22 5:14 PM, Dylan Yudaken wrote:
>> I think changing the tests to use the public API is probably better
>> than exporting this function. I don't believe it has much general use?
> 
> But there is no public API that does the same thing. I'll mark it
> as static and create a copy of that function in iopoll.c (in v2).
> 
> Something like this, what do you think?

Yeah I think this is better for now, and then we can work on fixing
the test. Sanity of the core parts of the library is a lot more
important than some test case.

-- 
Jens Axboe