| Message ID | c818d685-6609-450b-a695-44b5c57518a7@p183 (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/percpu: fix cast in __pcpu_cast_* macros | expand |
Hi Alexey, On Tue, May 28, 2024 at 01:43:27PM -0700, Christoph Lameter (Ampere) wrote: > On Mon, 27 May 2024, Alexey Dobriyan wrote: > > > this_cpu_add(*p, x = 1); > > A side effect in a this_cpu_op? Shudder.... > At the end of the day this is Thomas' code, but I can't imagine a single use case where this is a good idea. Especially with how much fun percpu macros end up being. Thanks, Dennis
On Mon, May 27 2024 at 08:51, Alexey Dobriyan wrote: > Fix compile failure > > this_cpu_add(*p, x = 1); > > kernel/test.c:7:29: error: lvalue required as left operand of assignment > 7 | this_cpu_add(*p, x = 1); > | ^ > arch/x86/include/asm/percpu.h:134:51: note: in definition of macro '__pcpu_cast_1' > 134 | #define __pcpu_cast_1(val) ((u8)(((unsigned long) val) & 0xff)) > > > This pattern is almost always wrong: > > #define M(x) ((T)x) > > also because it can't be used with assignment expressions: cast makes > left part of it non-lvalue and assignment to non-lvalue has to break > compilation. In principle I agree with the change itself, but I really don't see the point for this_cpu_add(*p, x = 1); Even if C supports it, it's just nasty and tasteless. Thanks, tglx
--- a/arch/x86/include/asm/percpu.h +++ b/arch/x86/include/asm/percpu.h @@ -119,9 +119,9 @@ #define __pcpu_type_4 u32 #define __pcpu_type_8 u64 -#define __pcpu_cast_1(val) ((u8)(((unsigned long) val) & 0xff)) -#define __pcpu_cast_2(val) ((u16)(((unsigned long) val) & 0xffff)) -#define __pcpu_cast_4(val) ((u32)(((unsigned long) val) & 0xffffffff)) +#define __pcpu_cast_1(val) ((u8)((unsigned long)(val) & 0xff)) +#define __pcpu_cast_2(val) ((u16)((unsigned long)(val) & 0xffff)) +#define __pcpu_cast_4(val) ((u32)((unsigned long)(val) & 0xffffffff)) #define __pcpu_cast_8(val) ((u64)(val)) #define __pcpu_op1_1(op, dst) op "b " dst
Fix compile failure this_cpu_add(*p, x = 1); kernel/test.c:7:29: error: lvalue required as left operand of assignment 7 | this_cpu_add(*p, x = 1); | ^ arch/x86/include/asm/percpu.h:134:51: note: in definition of macro '__pcpu_cast_1' 134 | #define __pcpu_cast_1(val) ((u8)(((unsigned long) val) & 0xff)) This pattern is almost always wrong: #define M(x) ((T)x) also because it can't be used with assignment expressions: cast makes left part of it non-lvalue and assignment to non-lvalue has to break compilation. Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com> --- arch/x86/include/asm/percpu.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)