| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Z8q6LlkQDG95Amgw@visitorckw-System-Product-Name>
Date: Fri, 7 Mar 2025 17:19:42 +0800
From: Kuan-Wei Chiu <visitorckw@...il.com>
To: "H. Peter Anvin" <hpa@...or.com>, longman@...hat.com, llong@...hat.com
Cc: tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
dave.hansen@...ux.intel.com, x86@...nel.org, jk@...abs.org,
joel@....id.au, eajames@...ux.ibm.com, andrzej.hajda@...el.com,
neil.armstrong@...aro.org, rfoss@...nel.org,
maarten.lankhorst@...ux.intel.com, mripard@...nel.org,
tzimmermann@...e.de, airlied@...il.com, simona@...ll.ch,
dmitry.torokhov@...il.com, mchehab@...nel.org,
awalls@...metrocast.net, hverkuil@...all.nl,
miquel.raynal@...tlin.com, richard@....at, vigneshr@...com,
louis.peens@...igine.com, andrew+netdev@...n.ch,
davem@...emloft.net, edumazet@...gle.com, pabeni@...hat.com,
parthiban.veerasooran@...rochip.com, arend.vanspriel@...adcom.com,
johannes@...solutions.net, gregkh@...uxfoundation.org,
jirislaby@...nel.org, yury.norov@...il.com,
akpm@...ux-foundation.org, alistair@...ple.id.au,
linux@...musvillemoes.dk, Laurent.pinchart@...asonboard.com,
jonas@...boo.se, jernej.skrabec@...il.com, kuba@...nel.org,
linux-kernel@...r.kernel.org, linux-fsi@...ts.ozlabs.org,
dri-devel@...ts.freedesktop.org, linux-input@...r.kernel.org,
linux-media@...r.kernel.org, linux-mtd@...ts.infradead.org,
oss-drivers@...igine.com, netdev@...r.kernel.org,
linux-wireless@...r.kernel.org, brcm80211@...ts.linux.dev,
brcm80211-dev-list.pdl@...adcom.com, linux-serial@...r.kernel.org,
bpf@...r.kernel.org, jserv@...s.ncku.edu.tw,
Yu-Chun Lin <eleanor15x@...il.com>
Subject: Re: [PATCH v3 00/16] Introduce and use generic parity16/32/64 helper
+Cc Waiman Long for bool cast to int discussion
Hi Peter,
On Thu, Mar 06, 2025 at 07:14:13PM -0800, H. Peter Anvin wrote:
> On March 6, 2025 8:25:25 AM PST, Kuan-Wei Chiu <visitorckw@...il.com> wrote:
> >Several parts of the kernel contain redundant implementations of parity
> >calculations for 16/32/64-bit values. Introduces generic
> >parity16/32/64() helpers in bitops.h, providing a standardized
> >and optimized implementation.
> >
> >Subsequent patches refactor various kernel components to replace
> >open-coded parity calculations with the new helpers, reducing code
> >duplication and improving maintainability.
> >
> >Co-developed-by: Yu-Chun Lin <eleanor15x@...il.com>
> >Signed-off-by: Yu-Chun Lin <eleanor15x@...il.com>
> >Signed-off-by: Kuan-Wei Chiu <visitorckw@...il.com>
> >---
> >In v3, I use parityXX() instead of the parity() macro since the
> >parity() macro may generate suboptimal code and requires special hacks
> >to make GCC happy. If anyone still prefers a single parity() macro,
> >please let me know.
> >
> >Additionally, I changed parityXX() << y users to !!parityXX() << y
> >because, unlike C++, C does not guarantee that true casts to int as 1.
> >
> >Changes in v3:
> >- Avoid using __builtin_parity.
> >- Change return type to bool.
> >- Drop parity() macro.
> >- Change parityXX() << y to !!parityXX() << y.
> >
> >
> >Changes in v2:
> >- Provide fallback functions for __builtin_parity() when the compiler
> > decides not to inline it
> >- Use __builtin_parity() when no architecture-specific implementation
> > is available
> >- Optimize for constant folding when val is a compile-time constant
> >- Add a generic parity() macro
> >- Drop the x86 bootflag conversion patch since it has been merged into
> > the tip tree
> >
> >v1: https://lore.kernel.org/lkml/20250223164217.2139331-1-visitorckw@gmail.com/
> >v2: https://lore.kernel.org/lkml/20250301142409.2513835-1-visitorckw@gmail.com/
> >
> >Kuan-Wei Chiu (16):
> > bitops: Change parity8() return type to bool
> > bitops: Add parity16(), parity32(), and parity64() helpers
> > media: media/test_drivers: Replace open-coded parity calculation with
> > parity8()
> > media: pci: cx18-av-vbi: Replace open-coded parity calculation with
> > parity8()
> > media: saa7115: Replace open-coded parity calculation with parity8()
> > serial: max3100: Replace open-coded parity calculation with parity8()
> > lib/bch: Replace open-coded parity calculation with parity32()
> > Input: joystick - Replace open-coded parity calculation with
> > parity32()
> > net: ethernet: oa_tc6: Replace open-coded parity calculation with
> > parity32()
> > wifi: brcm80211: Replace open-coded parity calculation with parity32()
> > drm/bridge: dw-hdmi: Replace open-coded parity calculation with
> > parity32()
> > mtd: ssfdc: Replace open-coded parity calculation with parity32()
> > fsi: i2cr: Replace open-coded parity calculation with parity32()
> > fsi: i2cr: Replace open-coded parity calculation with parity64()
> > Input: joystick - Replace open-coded parity calculation with
> > parity64()
> > nfp: bpf: Replace open-coded parity calculation with parity64()
> >
> > drivers/fsi/fsi-master-i2cr.c | 18 ++-----
> > .../drm/bridge/synopsys/dw-hdmi-ahb-audio.c | 8 +--
> > drivers/input/joystick/grip_mp.c | 17 +-----
> > drivers/input/joystick/sidewinder.c | 24 ++-------
> > drivers/media/i2c/saa7115.c | 12 +----
> > drivers/media/pci/cx18/cx18-av-vbi.c | 12 +----
> > .../media/test-drivers/vivid/vivid-vbi-gen.c | 8 +--
> > drivers/mtd/ssfdc.c | 20 ++-----
> > drivers/net/ethernet/netronome/nfp/nfp_asm.c | 7 +--
> > drivers/net/ethernet/oa_tc6.c | 19 ++-----
> > .../broadcom/brcm80211/brcmsmac/dma.c | 16 +-----
> > drivers/tty/serial/max3100.c | 3 +-
> > include/linux/bitops.h | 52 +++++++++++++++++--
> > lib/bch.c | 14 +----
> > 14 files changed, 77 insertions(+), 153 deletions(-)
> >
>
> !!x is used with a value that is not necessary booleanized already, and is exactly equivalent to (x ? true : false). It is totally redundant on a value known to be bool.
>
I used to believe that casting a boolean variable to int would always
result in 0 or 1 until a few months ago when Waiman Long explicitly
pointed out during a review that C does not guarantee this.
So I revisited the C11 standard, which states that casting to _Bool
always results in 0 or 1 [1]. Another section specifies that bool,
true, and false are macros defined in <stdbool.h>, with true expanding
to 1 and false to 0. However, these macros can be #undef and redefined
to other values [2]. I'm not sure if this is sufficient to conclude
that casting bool to int will always result in 0 or 1, but if the
consensus is that it does, I'll remove the !! hack in the next version.
> If (int)true wasn't inherently 1, then !! wouldn't work either.
>
The C standard guarantees that the ! operator returns an int, either 0
or 1. So regardless of how true casts, using !! should work. Right?
> There was a time when some code would use as a temporary hack:
>
> typedef enum { false, true } bool;
>
> ... when compiling on pre-C99 compilers; in that case a (bool) case wouldn't necessarily work as expected, whereas !! would. Furthermore, unlike (bool), !! works in the preprocessor.
I'm not entirely sure how !! works in the preprocessor. I always
thought it was handled by the compiler. Could you elaborate on this?
Regards,
Kuan-Wei
[1]: 6.3.1.2 Boolean type
1 When any scalar value is converted to _Bool, the result is 0 if the value compares equal
to 0; otherwise, the result is 1.59)
[2]: 7.18 Boolean type and values <stdbool.h>
1 The header <stdbool.h> defines four macros.
2 The macro
bool
expands to _Bool.
3 The remaining three macros are suitable for use in #if preprocessing directives. They
are
true
which expands to the integer constant 1,
false
which expands to the integer constant 0, and
_ _bool_true_false_are_defined
which expands to the integer constant 1.
4 Notwithstanding the provisions of 7.1.3, a program may undefine and perhaps then
redefine the macros bool, true, and false.
259)
Powered by blists - more mailing lists