Hi, Il giorno mar 20 giu 2023 alle ore 15:00 Jan Beulich <jbeul...@suse.com> ha scritto:
> On 20.06.2023 12:35, Simone Ballarin wrote: > > --- a/xen/include/public/io/ring.h > > +++ b/xen/include/public/io/ring.h > > @@ -36,11 +36,11 @@ > > typedef unsigned int RING_IDX; > > > > /* Round a 32-bit unsigned constant down to the nearest power of two. */ > > -#define __RD2(_x) (((_x) & 0x00000002) ? 0x2 : ((_x) > & 0x1)) > > -#define __RD4(_x) (((_x) & 0x0000000c) ? __RD2((_x)>>2)<<2 : > __RD2(_x)) > > -#define __RD8(_x) (((_x) & 0x000000f0) ? __RD4((_x)>>4)<<4 : > __RD4(_x)) > > -#define __RD16(_x) (((_x) & 0x0000ff00) ? __RD8((_x)>>8)<<8 : > __RD8(_x)) > > -#define __RD32(_x) (((_x) & 0xffff0000) ? __RD16((_x)>>16)<<16 : > __RD16(_x)) > > +#define __RD2(_x) (((_x) & 0x00000002U) ? 0x2 : ((_x) > & 0x1)) > > +#define __RD4(_x) (((_x) & 0x0000000cU) ? __RD2((_x)>>2)<<2 : > __RD2(_x)) > > +#define __RD8(_x) (((_x) & 0x000000f0U) ? __RD4((_x)>>4)<<4 : > __RD4(_x)) > > +#define __RD16(_x) (((_x) & 0x0000ff00U) ? __RD8((_x)>>8)<<8 : > __RD8(_x)) > > +#define __RD32(_x) (((_x) & 0xffff0000U) ? __RD16((_x)>>16)<<16 : > __RD16(_x)) > > While I don't mind the suffixes being added, I'm wondering how > the tool would have spotted the single violation here. Iirc we > don't use this header anywhere in the hypervisor. > In the analyzed build it is used: aarch64-linux-gnu-gcc-12 -MMD -MP -MF common/.vm_event.o.d -DBUILD_ID -fno-strict-aliasing -std=gnu99 -Wall -Wstrict-prototypes -Wdeclaration-after-statement -Wno-unused-but-set-variable -Wno-unused-local-typedefs -O1 -fno-omit-frame-pointer -nostdinc -fno-builtin -fno-common -Werror -Wredundant-decls -Wno-pointer-arith -Wvla -pipe -D__XEN__ -include ./include/xen/config.h "-Wa,--strip-local-absolute" -g -mcpu=generic -mgeneral-regs-only -mno-outline-atomics -I./include -I./arch/arm/include -fno-pie -fno-stack-protector -fno-exceptions -fno-asynchronous-unwind-tables -Wnested-externs -fprofile-arcs -ftest-coverage -c common/vm_event.c -o common/vm_event.o xen/common/vm_event.c: 85 FRONT_RING_INIT(&ved->front_ring, 86 (vm_event_sring_t *)ved->ring_page, 87 PAGE_SIZE); The expansions that lead to __RD32 are: FRONT_RING_INIT -> FRONT_RING_ATTACH -> __RING_SIZE -> __RD32 > > Furthermore, if I recall correctly Misra also mandates single > evaluation of macro arguments. While I don't immediately see how > to address that without resorting to compiler extensions, I don't > think it makes sense to address one violation here but not he > other (the more when the code in question doesn't affect the > hypervisor build). > If not clearly connected I suggest discussing one rule at a time. > > Jan > -- Simone Ballarin, M.Sc. Field Application Engineer, BUGSENG (https://bugseng.com <http://bugseng.com>)
