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

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

[Ammar Faizi]

From: Ammar Faizi <ammarfaizi2@gnuweeb.org>

Hi Jens,

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: 2950c0c1b23ef6bc3cfa3646adbfeffc23fc59cf
-- 
Ammar Faizi

[PATCH liburing v1 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..7660574 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..912851a 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..1885731 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..c61d572 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 v1 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.

Co-authored-by: Dylan Yudaken <dylany@fb.com>
Signed-off-by: Dylan Yudaken <dylany@fb.com>
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 v1 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.

Co-authored-by: Dylan Yudaken <dylany@fb.com>
Signed-off-by: Dylan Yudaken <dylany@fb.com>
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 v1 1/3] liburing: Clean up `-Wshorten-64-to-32` warnings from clang

[Ammar Faizi]

On 10/20/22 6:52 PM, Ammar Faizi wrote:
> 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>

BTW, before it's too late. I think we should be consistent on the cast
style:

    (type) a

or

    (type)a

What do you think?

-- 
Ammar Faizi

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

[Jens Axboe]

On 10/20/22 5:12 AM, Ammar Faizi wrote:
> On 10/20/22 6:52 PM, Ammar Faizi wrote:
>> 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>
> 
> BTW, before it's too late. I think we should be consistent on the cast
> style:
> 
>    (type) a
> 
> or
> 
>    (type)a
> 
> What do you think?

I tend to prefer (type) a, but sometimes if you have multiple casts
or the lines become too long, I'll do (type)a. I don't think it's
super important in terms of style, as it isn't that prevalent.

-- 
Jens Axboe

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

[Dylan Yudaken]

On Thu, 2022-10-20 at 18:52 +0700, Ammar Faizi wrote:
> From: Ammar Faizi <ammarfaizi2@gnuweeb.org>
> 
> - 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.


I think better to take my co-authored and signed-off tags from patch
2/3, as its not really similar to what I had done

Thanks,
Dylan

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

[Ammar Faizi]

On 10/20/22 7:53 PM, Dylan Yudaken wrote:
> I think better to take my co-authored and signed-off tags from patch
> 2/3, as its not really similar to what I had done

I'll send v2 with that fixed.

-- 
Ammar Faizi