A more correct bitWrite() macro in wiring.h?

CoreyCoop

Active member
I have not checked this issue with other versions of the wiring.h header, but in the hardware/teensy/avr/cores/teensy3 version I found the following issue:

Code:
// my original code, which does not work properly, (although it compiles with no errors):
bitWrite(CheckBits, i2, (digitalRead(ports[1][i2])==HIGH)?1:0);

// but this does work (added parenthesis):
bitWrite(CheckBits, i2, ((digitalRead(ports[1][i2])==HIGH)?1:0));

//  the code in wiring.h is 
#define bitWrite(value, bit, bitvalue) (bitvalue ? bitSet(value, bit) : bitClear(value, bit))

// I think to fix my problem  it would be more correct to be written:
#define bitWrite(value, bit, bitvalue) ((bitvalue) ? bitSet(value, bit) : bitClear(value, bit))

// and while it might be redundant, since "bitRead" for instance, does put parenthesis around "bit", one could argue that this could be even more correct:
#define bitWrite(value, bit, bitvalue) ((bitvalue) ? bitSet(value, (bit)) : bitClear(value, (bit)))

And yes, after writing the code, and finding the bug, I checked to be sure that HIGH and LOW are defined such that I didn't need the trinary comparison, but the example still points out the problem.

Corey Cooper
 
Back
Top