Bosch Sensortec Community

    cancel
    Showing results for 
    Search instead for 
    Did you mean: 
    SOLVED

    Hazardous use of nested UINT16_C macros in BMI270 API

    Hazardous use of nested UINT16_C macros in BMI270 API

    sebmadgwick
    Established Member

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

     

    2 REPLIES 2

    o_o
    Contributor

    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.

    o_o
    Contributor
    Thank you sir!

    We will address this issue.
    Icon--AD-black-48x48Icon--address-consumer-data-black-48x48Icon--appointment-black-48x48Icon--back-left-black-48x48Icon--calendar-black-48x48Icon--center-alignedIcon--Checkbox-checkIcon--clock-black-48x48Icon--close-black-48x48Icon--compare-black-48x48Icon--confirmation-black-48x48Icon--dealer-details-black-48x48Icon--delete-black-48x48Icon--delivery-black-48x48Icon--down-black-48x48Icon--download-black-48x48Ic-OverlayAlertIcon--externallink-black-48x48Icon-Filledforward-right_adjustedIcon--grid-view-black-48x48IC_gd_Check-Circle170821_Icons_Community170823_Bosch_Icons170823_Bosch_Icons170821_Icons_CommunityIC-logout170821_Icons_Community170825_Bosch_Icons170821_Icons_CommunityIC-shopping-cart2170821_Icons_CommunityIC-upIC_UserIcon--imageIcon--info-i-black-48x48Icon--left-alignedIcon--Less-minimize-black-48x48Icon-FilledIcon--List-Check-grennIcon--List-Check-blackIcon--List-Cross-blackIcon--list-view-mobile-black-48x48Icon--list-view-black-48x48Icon--More-Maximize-black-48x48Icon--my-product-black-48x48Icon--newsletter-black-48x48Icon--payment-black-48x48Icon--print-black-48x48Icon--promotion-black-48x48Icon--registration-black-48x48Icon--Reset-black-48x48Icon--right-alignedshare-circle1Icon--share-black-48x48Icon--shopping-bag-black-48x48Icon-shopping-cartIcon--start-play-black-48x48Icon--store-locator-black-48x48Ic-OverlayAlertIcon--summary-black-48x48tumblrIcon-FilledvineIc-OverlayAlertwhishlist