nxp-mcuxpresso / rpmsg-lite

RPMsg implementation for small MCUs
BSD 3-Clause "New" or "Revised" License
241 stars 74 forks source link

Macro expansion bug in RL_ASSERT #2

Closed G33KatWork closed 7 years ago

G33KatWork commented 7 years ago

The RL_ASSERT macro is defined as

#define RL_ASSERT(x)  \
    do                \
    {                 \
        if (!x)       \
            while (1) \
                ;     \
    } while (0);

in rpmsg_default_config.h.

When calling this macro with an expression as an argument like you do in https://github.com/NXPmicro/rpmsg-lite/blob/master/lib/rpmsg_lite/rpmsg_lite.c#L160, it gets expanded in the wrong way.

Instead of if(!(rpmsg_lite_dev != NULL)) we get if(!rpmsg_lite_dev != NULL) which is something different. I only caught this, because I got compiler warnings like this:

In file included from /project/rpu_firmware/libs/rpmsg_lite/rpmsg-lite/lib/include/rpmsg_env.h:83:0,                                          
                 from /project/rpu_firmware/libs/rpmsg_lite/rpmsg-lite/lib/include/virtqueue.h:36,                                            
                 from /project/rpu_firmware/libs/rpmsg_lite/rpmsg-lite/lib/include/rpmsg_lite.h:41,                                           
                 from /project/rpu_firmware/libs/rpmsg_lite/rpmsg-lite/lib/rpmsg_lite/rpmsg_lite.c:33:                                        
/project/rpu_firmware/libs/rpmsg_lite/rpmsg-lite/lib/rpmsg_lite/rpmsg_lite.c: In function 'rpmsg_lite_rx_callback':                           
/project/rpu_firmware/libs/rpmsg_lite/rpmsg-lite/lib/rpmsg_lite/rpmsg_lite.c:160:30: warning: comparison between pointer and integer          
     RL_ASSERT(rpmsg_lite_dev != NULL);                                
                              ^
/project/rpu_firmware/libs/rpmsg_lite/rpmsg-lite/lib/include/rpmsg_default_config.h:76:14: note: in definition of macro 'RL_ASSERT'           
         if (!x)       \                                               
              ^
/project/rpu_firmware/libs/rpmsg_lite/rpmsg-lite/lib/rpmsg_lite/rpmsg_lite.c: In function 'rpmsg_lite_tx_callback':                           
/project/rpu_firmware/libs/rpmsg_lite/rpmsg-lite/lib/rpmsg_lite/rpmsg_lite.c:201:30: warning: comparison between pointer and integer          
     RL_ASSERT(rpmsg_lite_dev != NULL);                                
                              ^
/project/rpu_firmware/libs/rpmsg_lite/rpmsg-lite/lib/include/rpmsg_default_config.h:76:14: note: in definition of macro 'RL_ASSERT'           
         if (!x)       \ 

An easy fix is to just wrap the x in if(!x) with a pair of parentheses like this: if(!(x)).

MarekNovakNXP commented 7 years ago

Hello G33KatWork,

Thank you for your findings and for using RPMsg-Lite! Patch to fix this is on its way...

Regards, Marek