[PATCH liburing v2 0/3] Clean up clang `-Wshorten-64-to-32` warnings

[PATCH liburing v2 0/3] Clean up clang `-Wshorten-64-to-32` warnings

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Hi Jens,

This is a v2.

v2:
  - Fix Signed-off-by tag.
  - Make the cast in patch #1 consistent.

Please consider this small cleanup series before the release.

This series is a follow up of the clean up `-Wshorten-64-to-32`
warnings that I discussed with Dylan previously. It only addresses
the warnings in the src/ dir (main library). We ignore the tests
for now.

`-Wshorten-64-to-32` is a clang-specific flag, it currently doesn't
exist in GCC.

There are 3 patches in this series:

- Patch 1 is to clean up the warnings in the main library
  (this is based on a patch from Dylan).

  Taken from: https://github.com/DylanZA/liburing/commit/cdd6dfbb9019.patch

- Patch 2 is to introduce LIBURING_CFLAGS variable in the Makefile. We
  need this varaible to apply specific compiler flags to the main
  library only. Currently, this flag is only used by the GitHub bot.

- Patch 3 is to integrate `-Wshorten-64-to-32` flag to the GitHub bot.

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

Ammar Faizi (2):
  Makefile: Introduce `LIBURING_CFLAGS` variable
  github: Append `-Wshorten-64-to-32` flag for clang build

Dylan Yudaken (1):
  liburing: Clean up `-Wshorten-64-to-32` warnings from clang

 .github/workflows/build.yml |  4 ++++
 src/Makefile                |  5 ++++-
 src/arch/syscall-defs.h     |  4 ++--
 src/register.c              |  5 ++++-
 src/setup.c                 | 12 ++++++------
 src/syscall.h               |  4 ++--
 6 files changed, 22 insertions(+), 12 deletions(-)


base-commit: 3049530665ba6464f3c5f6f80b28e965719ca602
-- 
Ammar Faizi

[PATCH liburing v2 1/3] liburing: Clean up `-Wshorten-64-to-32` warnings from clang

[Ammar Faizi]

From: Dylan Yudaken <dylany@fb.com>

liburing has a couple of int shortening issues found by clang. Clean
them all. This cleanup is particularly useful for build systems that
include these files and run with that error enabled.

Link: https://lore.kernel.org/io-uring/20221019145042.446477-1-dylany@meta.com
Signed-off-by: Dylan Yudaken <dylany@fb.com>
Co-authored-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org>
---
 src/arch/syscall-defs.h |  4 ++--
 src/register.c          |  5 ++++-
 src/setup.c             | 12 ++++++------
 src/syscall.h           |  4 ++--
 4 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/src/arch/syscall-defs.h b/src/arch/syscall-defs.h
index 4afb2af..d351f8b 100644
--- a/src/arch/syscall-defs.h
+++ b/src/arch/syscall-defs.h
@@ -11,9 +11,9 @@ static inline int __sys_open(const char *pathname, int flags, mode_t mode)
 	 * Some architectures don't have __NR_open, but __NR_openat.
 	 */
 #ifdef __NR_open
-	return __do_syscall3(__NR_open, pathname, flags, mode);
+	return (int) __do_syscall3(__NR_open, pathname, flags, mode);
 #else
-	return __do_syscall4(__NR_openat, AT_FDCWD, pathname, flags, mode);
+	return (int) __do_syscall4(__NR_openat, AT_FDCWD, pathname, flags, mode);
 #endif
 }
 
diff --git a/src/register.c b/src/register.c
index 0a2e5af..e849825 100644
--- a/src/register.c
+++ b/src/register.c
@@ -277,8 +277,11 @@ int io_uring_enable_rings(struct io_uring *ring)
 int io_uring_register_iowq_aff(struct io_uring *ring, size_t cpusz,
 			       const cpu_set_t *mask)
 {
+	if (cpusz >= (1U << 31))
+		return -EINVAL;
+
 	return __sys_io_uring_register(ring->ring_fd, IORING_REGISTER_IOWQ_AFF,
-				       mask, cpusz);
+					mask, (int) cpusz);
 }
 
 int io_uring_unregister_iowq_aff(struct io_uring *ring)
diff --git a/src/setup.c b/src/setup.c
index 21283eb..d918f86 100644
--- a/src/setup.c
+++ b/src/setup.c
@@ -248,29 +248,29 @@ __cold void io_uring_free_probe(struct io_uring_probe *probe)
 	uring_free(probe);
 }
 
-static inline int __fls(int x)
+static inline int __fls(unsigned long x)
 {
 	if (!x)
 		return 0;
-	return 8 * sizeof(x) - __builtin_clz(x);
+	return 8 * sizeof(x) - __builtin_clzl(x);
 }
 
 static unsigned roundup_pow2(unsigned depth)
 {
-	return 1UL << __fls(depth - 1);
+	return 1U << __fls(depth - 1);
 }
 
-static size_t npages(size_t size, unsigned page_size)
+static size_t npages(size_t size, long page_size)
 {
 	size--;
 	size /= page_size;
-	return __fls(size);
+	return __fls((int) size);
 }
 
 #define KRING_SIZE	320
 
 static size_t rings_size(struct io_uring_params *p, unsigned entries,
-			 unsigned cq_entries, unsigned page_size)
+			 unsigned cq_entries, long page_size)
 {
 	size_t pages, sq_size, cq_size;
 
diff --git a/src/syscall.h b/src/syscall.h
index f750782..8f03dfc 100644
--- a/src/syscall.h
+++ b/src/syscall.h
@@ -23,9 +23,9 @@ static inline void *ERR_PTR(intptr_t n)
 	return (void *) n;
 }
 
-static inline intptr_t PTR_ERR(const void *ptr)
+static inline int PTR_ERR(const void *ptr)
 {
-	return (intptr_t) ptr;
+	return (int) (intptr_t) ptr;
 }
 
 static inline bool IS_ERR(const void *ptr)
-- 
Ammar Faizi

[PATCH liburing v2 2/3] Makefile: Introduce `LIBURING_CFLAGS` variable

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

`LIBURING_CFLAGS` will be appended to `CFLAGS` but it only applies
to files in the `src/` directory (the main library). The first use
case of this variable is for appending a clang-specific flag,
`-Wshorten-64-to-32` in the GitHub bot.

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

diff --git a/src/Makefile b/src/Makefile
index 73a98ba..09617fb 100644
--- a/src/Makefile
+++ b/src/Makefile
@@ -5,11 +5,14 @@ includedir ?= $(prefix)/include
 libdir ?= $(prefix)/lib
 libdevdir ?= $(prefix)/lib
 
+LIBURING_CFLAGS ?=
 CPPFLAGS ?=
 override CPPFLAGS += -D_GNU_SOURCE \
 	-Iinclude/ -include ../config-host.h
 CFLAGS ?= -g -O3 -Wall -Wextra -fno-stack-protector
-override CFLAGS += -Wno-unused-parameter -Wno-sign-compare -DLIBURING_INTERNAL
+override CFLAGS += -Wno-unused-parameter -Wno-sign-compare \
+	-DLIBURING_INTERNAL \
+	$(LIBURING_CFLAGS)
 SO_CFLAGS=-fPIC $(CFLAGS)
 L_CFLAGS=$(CFLAGS)
 LINK_FLAGS=
-- 
Ammar Faizi

[PATCH liburing v2 3/3] github: Append `-Wshorten-64-to-32` flag for clang build

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

liburing has a couple of int shortening issues found by clang. A
previous commit has cleaned them up. Integrate -Wshorten-64-to-32
flag to the GitHub bot to spot the same issue in the future.

This flag is clang-specific, it currently doesn't exist in GCC.

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

diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml
index 2608644..b0e669d 100644
--- a/.github/workflows/build.yml
+++ b/.github/workflows/build.yml
@@ -26,6 +26,7 @@ jobs:
             cxx_pkg: clang
             cc: clang
             cxx: clang++
+            extra_flags: -Wshorten-64-to-32
 
           # x86 (32-bit) gcc
           - arch: i686
@@ -86,6 +87,9 @@ jobs:
     env:
       FLAGS: -g -O3 -Wall -Wextra -Werror
 
+      # Flags for building sources in src/ dir only.
+      LIBURING_CFLAGS: ${{matrix.extra_flags}}
+
     steps:
     - name: Checkout source
       uses: actions/checkout@v2
-- 
Ammar Faizi

Re: [PATCH liburing v2 0/3] Clean up clang `-Wshorten-64-to-32` warnings

[Jens Axboe]

On Thu, 20 Oct 2022 20:14:52 +0700, Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> 
> Hi Jens,
> 
> This is a v2.
> 
> v2:
>   - Fix Signed-off-by tag.
>   - Make the cast in patch #1 consistent.
> 
> [...]

Applied, thanks!

[1/3] liburing: Clean up `-Wshorten-64-to-32` warnings from clang
      commit: b9df4750ce9335437a34fd93c76863d871b70416
[2/3] Makefile: Introduce `LIBURING_CFLAGS` variable
      commit: 1755faf1737d0d84775aa46dd0a2b4b2e3474d28
[3/3] github: Append `-Wshorten-64-to-32` flag for clang build
      commit: 80817b7cd56b96a095c3cb4583a357d16fb867f1

Best regards,
-- 
Jens Axboe