views:

163

answers:

3

Im trying to write a few simple macros to simplify the task of setting and clearing bits which should be a simple task however I cant seem to get them to work correctly.

#define SET_BIT(p,n) ((p) |= (1 << (n)))
#define CLR_BIT(p,n) ((p) &= (~(1) << (n)))
+6  A: 

One obvious issue is that ((p) &= (~(1) << (n))) should be ((p) &= ~(1 << (n))).

Apart from that, you do have to be careful with the width of your integer types. If you were using unsigned long you might need to use (e.g.) ((p) |= (1UL << (n)))

Charles Bailey
Yes your right that was the problem, Thanks.
volting
+1 for pointing out the integer sizing issues.
Matt B.
A: 

Ugh. Do you not have a set of functions locally to do this for you? That would hide any sort of magic that has to occur when skipping across word boundaries.

Failing that, how does the above fail? They look 'ok', but I'd still rather do this sort of thing by hand if functions aren't available. Macros just hide nasty bugs when doing this sort of thing. Passing signed vs unsigned, etc. Won't be caught with Macros.

Michael Dorgan
+4  A: 

Try

#define CLR_BIT(p,n) ((p) &= ~((1) << (n)))

However for various reasons of general macro evil I would advise not using a macro. Use an inline function and pass by reference, something like this:

static inline void set_bit(long *x, int bitNum) {
    *x |= (1L << bitNum);
}
Artelius
Thanks that worked. Iv heard all about the "evils" of macros and I generally restrict my use of them, but in this case Im just to the simplify the task of setting and clearing port pins on an AVR controller e.g CLR_BIT(PORTA, PIN3), I thought defines were acceptable in that context? but I guess using inline functions would eliminate any headaches..
volting
They're acceptable, there are certainly far worse ways to use the preprocessor: http://stackoverflow.com/questions/652788 but one day you'll have a strange bug that turns out to be macro-related.
Artelius
True, Ive seen plenty of those horrors of macros threads.. Its scary how easily people can butcher the c language... I think Ill stick to inline functions when I can as long as they as are as fast as macros theres nothing to be lost only gained.
volting
These *particular* macros are quite safe, since they evaluate each argument only once.
caf
Agreed, they are pretty safe, but if you accidentally give them a pointer argument or some such, the errors will be more cryptic than usual. Macros with side effects are too easy to get wrong, though.
Artelius
Does this work when sizeof(int) < sizeof(long)? I would have thought not, since 1 << bitNum would overflow.
Paul Hankin
Quite right, Paul. I'm too accustomed to my 32-bit processors...
Artelius