On 29/08/2017 18:46, David Hildenbrand wrote: >> +#define BYTE_MASK(access) \ >> + ((1 & (access) ? 2 : 0) | \ >> + (2 & (access) ? 4 : 0) | \ >> + (3 & (access) ? 8 : 0) | \ >> + (4 & (access) ? 16 : 0) | \ >> + (5 & (access) ? 32 : 0) | \ >> + (6 & (access) ? 64 : 0) | \ >> + (7 & (access) ? 128 : 0)) >> + > Hmm, I wonder if > > #define BYTE_MASK(access) \ > ((1 & (access) ? (1 << 1) : 0) | \ > (2 & (access) ? (1 << 2) : 0) | \ > ... > > would be easier to get > Yeah, you have a point. Another way to write it is: (1 & (access) ? 0xAA : 0) | \ (2 & (access) ? 0xCC : 0) | \ (4 & (access) ? 0xF0 : 0) but I think yours is the best. >> >> + >> + const u8 x = BYTE_MASK(ACC_EXEC_MASK); >> + const u8 w = BYTE_MASK(ACC_WRITE_MASK); >> + const u8 u = BYTE_MASK(ACC_USER_MASK); >> + >> + bool cr4_smep = kvm_read_cr4_bits(vcpu, X86_CR4_SMEP) != 0; >> + bool cr4_smap = kvm_read_cr4_bits(vcpu, X86_CR4_SMAP) != 0; >> + bool cr0_wp = is_write_protection(vcpu); > > all three can be turned const. (and I'd drop the empty lines in between ..) I am using const to identify a compile-time constant here, so more like "static const" (but I was not sure if C optimizes away static const, so I just used "const"). This explains also the empty line! Paolo
