4

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)))
volting
  • 16,773
  • 7
  • 36
  • 54
  • 2
    It would help if you said how, exactly, the macros aren't working. – CB Bailey Jun 04 '10 at 23:36
  • 1
    sorry about neglecting that... I thought the error would be obvious to some of the better versed c programmers here, rather than me *trying* to explain... – volting Jun 04 '10 at 23:54
  • I could see two potential issues but without any context it's impossible to tell what applies to your situation and what doesn't. – CB Bailey Jun 04 '10 at 23:58

3 Answers3

11

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
  • 48,337
  • 13
  • 89
  • 105
  • 1
    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 Jun 04 '10 at 23:51
  • 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 Jun 05 '10 at 00:15
  • 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 Jun 05 '10 at 00:28
  • 2
    These *particular* macros are quite safe, since they evaluate each argument only once. – caf Jun 05 '10 at 03:28
  • 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 Jun 05 '10 at 05:40
  • 3
    Does this work when sizeof(int) < sizeof(long)? I would have thought not, since 1 << bitNum would overflow. –  Jun 05 '10 at 12:38
  • Why should it be static? I wrote myself a header with functions like this, I didn't use static but it worked like it should. – JuSchu Jan 19 '15 at 09:45
8

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)))

CB Bailey
  • 755,051
  • 104
  • 632
  • 656
0

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
  • 12,453
  • 3
  • 31
  • 61