From: Nathan Chancellor <nathan@kernel.org>
To: Justin Stitt <jstitt007@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Nick Desaulniers <ndesaulniers@google.com>,
Tom Rix <trix@redhat.com>,
linux-kernel@vger.kernel.org, llvm@lists.linux.dev,
Al Viro <viro@zeniv.linux.org.uk>
Subject: Re: [PATCH v3] include/uapi/linux/swab.h: move explicit cast outside ternary
Date: Thu, 9 Jun 2022 13:31:40 -0700 [thread overview]
Message-ID: <YqJYrImC3Bk40H1H@dev-arch.thelio-3990X> (raw)
In-Reply-To: <20220608223539.470472-1-justinstitt@google.com>
On Wed, Jun 08, 2022 at 03:35:39PM -0700, Justin Stitt wrote:
> From: Justin Stitt <jstitt007@gmail.com>
>
> A cast inside __builtin_constant_p doesn't do anything since it should
> evaluate as constant at compile time irrespective of this cast. Instead,
> I moved this cast outside the ternary to ensure the return type is as
> expected.
>
> Additionally, if __HAVE_BUILTIN_BSWAP16__ was not defined then __swab16
> is actually returning an `int` not a `u16` due to integer promotion.
>
> As Al Viro notes:
> You *can't* get smaller-than-int out of ? :, same as you can't get it
> out of addition, etc.
>
> This also fixes some clang -Wformat warnings involving default
> argument promotion.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/378
> Suggested-by: Al Viro <viro@zeniv.linux.org.uk>
> Suggested-by: Nathan Chancellor <nathan@kernel.org>
> Suggested-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Justin Stitt <jstitt007@gmail.com>
Thanks for the patch and sticking with it through all the review!
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> ---
> diff from v2 -> v3:
> * re-insert respective (u16, u32, u64) cast to __builtin_bswap as per
> Nick's suggestion
> * added note from Al Viro
>
> include/uapi/linux/swab.h | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/linux/swab.h b/include/uapi/linux/swab.h
> index 7272f85d6d6a..0723a9cce747 100644
> --- a/include/uapi/linux/swab.h
> +++ b/include/uapi/linux/swab.h
> @@ -102,7 +102,7 @@ static inline __attribute_const__ __u32 __fswahb32(__u32 val)
> #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
> #else
> #define __swab16(x) \
> - (__builtin_constant_p((__u16)(x)) ? \
> + (__u16)(__builtin_constant_p(x) ? \
> ___constant_swab16(x) : \
> __fswab16(x))
> #endif
> @@ -115,7 +115,7 @@ static inline __attribute_const__ __u32 __fswahb32(__u32 val)
> #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
> #else
> #define __swab32(x) \
> - (__builtin_constant_p((__u32)(x)) ? \
> + (__u32)(__builtin_constant_p(x) ? \
> ___constant_swab32(x) : \
> __fswab32(x))
> #endif
> @@ -128,7 +128,7 @@ static inline __attribute_const__ __u32 __fswahb32(__u32 val)
> #define __swab64(x) (__u64)__builtin_bswap64((__u64)(x))
> #else
> #define __swab64(x) \
> - (__builtin_constant_p((__u64)(x)) ? \
> + (__u64)(__builtin_constant_p(x) ? \
> ___constant_swab64(x) : \
> __fswab64(x))
> #endif
> --
> 2.36.1.476.g0c4daa206d-goog
>
>
next prev parent reply other threads:[~2022-06-09 20:31 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-08 22:35 [PATCH v3] include/uapi/linux/swab.h: move explicit cast outside ternary Justin Stitt
2022-06-09 20:31 ` Nathan Chancellor [this message]
2022-07-18 18:12 ` Justin Stitt
2022-07-18 18:22 ` Nathan Chancellor
2022-07-18 18:25 ` Justin Stitt
2022-07-18 18:47 ` Nathan Chancellor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YqJYrImC3Bk40H1H@dev-arch.thelio-3990X \
--to=nathan@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=jstitt007@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=ndesaulniers@google.com \
--cc=trix@redhat.com \
--cc=viro@zeniv.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.