C++ bitfield testing

Question

Is there a more compact way of comparing my bits than this (the only way I know):

#define BIT1 1
#define BIT2 2
#define BIT3 4
#define BIT4 8
#define BIT5 16
#define BIT6 32

// I declare this somewhere in a structure
unsigned char bits: 6;

// I want all of them to be 0 at first (000000)
bits = 0;

/* I do some bite setting here */

// I only want to know if the state of my bits == 000000
if(bits & (BIT1 | BIT2 | BIT3 | BIT4 | BIT5 | BIT6) == (BIT1 | BIT2 | BIT3 | BIT4 | BIT5 | BIT6))
{
    // All kinds of nasty stuff
}

I thought maybe something in the lines of bits & 0x00 == 0x00

Answer 1

If you want compact (as indicated in your comment) rather than fast, why not do something like:

#define BIT1 0x01
#define BIT2 0x02
#define BIT3 0x04
#define BIT4 0x08
#define BIT5 0x10
#define BIT6 0x20
#define BITS1THRU4 (BIT1|BIT2|BIT3|BIT4)
// or #define BITS1THRU6 0x0f

// I declare this somewhere in a structure
unsigned char bits: 6;

// I want all of them to be 0 at first (000000)
bits = 0;

/* I do some bit setting here */

// I only want to know if the state of my first four bits == 0000
if(bits & BITS1THRU4 == 0) ...

It probably won't be any faster since your original code would have been turned into that constant anyway but it may be more readable (which is often a good reason to do it).

If you have a need for other variations, just define them. If there's too many of them (63 defines, if you use them all, may be getting a bit on the high side), I'd start thinking about another solution.

But, to be honest, unless you're going to use more meaningful names for the defines, I'd just ditch them. The name BIT3 really adds nothing to 0x04 to those that understand bit patterns. If it was something like UART_READ_READY_BIT, that would be fine but what you have is only slightly better than:

#define THREE 3

(no offence intended, I'm just pointing out my views). I'd just work out the bit patterns and put them straight in the code (bits 1 thru 6 in your case being 0x3f).

And, just as an aside, for you particular case, I think bits will only be those six bits anyway so you may find it's enough to compare it to 0 (with no bit masking). I've left in the bit masking method in case you wanted a mode general solution for checking specific bits.

Answer 2

if( ~bits & (BIT1 | BIT2 | BIT3 | BIT4 | BIT5 | BIT6) == (BIT1 | BIT2 | BIT3 | BIT4 | BIT5 | BIT6))

And about speed (BIT1 | BIT2 | BIT3 | BIT4 | BIT5 | BIT6) is actualy compiled as constant and there are only few operation (like 2) for this if - one NOR and one compare (i am not sure if x386 supprts NOR, but i think it does)

Answer 3

Though this is just a note, operator == has higher precedence than &, the logical-and should be parenthesized as:

if((a & b) == c)

I interpreted that this is the intention of the questioner.

(I think this should be posted as just a comment, but it seems that I can't post a comment)

Answer 4

if I read your condition right (with corrected parentheses, as per @user547710), you check if all your bits are set, rather than zero.

Anyway, you can define a mask with bits 1-6 set more compactly by (1u << 6) - 1. This is compile-time constant expression, so you need not to worry about extra computing time. I'd do:

const unsigned char bitmask6 = 1u << 6) - 1;

if ((bits & bitmask6) == bitmask6)