06-23-2019 06:02 PM
Lines 362 to 369 of bmi2.c use nested UINT16_C macros. This results in compiler warnings (may be compiler specific) and presents a hazard that I recommend is fixed.
The follow steps describe the macro expansion of one line (362) to demonstrate the hazard:
#define ACC_2G_MAX_NOISE_LIMIT UINT16_C(ACC_FOC_2G_REF + 255)
↓
#define ACC_2G_MAX_NOISE_LIMIT UINT16_C(UINT16_C(16384) + 255)
↓
#define ACC_2G_MAX_NOISE_LIMIT 16384 ## U + 255 ## U
↓
#define ACC_2G_MAX_NOISE_LIMIT 16384U + 255U
This crates an operator precedence hazard. For example, 2 * ACC_2G_MAX_NOISE_LIMIT would expand to 2 * 16384U + 255U, and not 2 * (16384U + 255U).
This hazard does not currently result in bugs because the only use of these definitions is with '<' and '>' operators which have lower precedence and '+' and '-'. However, relying on such circumstance is poor practise and risks bugs appearing in the future. I suggest that lines 362 to 369 are updated to become:
#define ACC_2G_MAX_NOISE_LIMIT (ACC_FOC_2G_REF + UINT16_C(255))
#define ACC_2G_MIN_NOISE_LIMIT (ACC_FOC_2G_REF - UINT16_C(255))
#define ACC_4G_MAX_NOISE_LIMIT (ACC_FOC_4G_REF + UINT16_C(255))
#define ACC_4G_MIN_NOISE_LIMIT (ACC_FOC_4G_REF - UINT16_C(255))
#define ACC_8G_MAX_NOISE_LIMIT (ACC_FOC_8G_REF + UINT16_C(255))
#define ACC_8G_MIN_NOISE_LIMIT (ACC_FOC_8G_REF - UINT16_C(255))
#define ACC_16G_MAX_NOISE_LIMIT (ACC_FOC_16G_REF + UINT16_C(255))
#define ACC_16G_MIN_NOISE_LIMIT (ACC_FOC_16G_REF - UINT16_C(255))
Solved! Go to Solution.
06-24-2019 11:39 AM - edited 06-24-2019 11:40 AM
Thanks for the feedback !
Could you place a pull request directly on GitHub ?
Please link this message in your comment on GitHub so we know the history.
06-25-2019 07:47 AM