ThrowTheSwitch / CMock

CMock - Mock/stub generator for C
http://throwtheswitch.org
MIT License
653 stars 269 forks source link

Default CMock configuration issues invalid check for void* arguments #400

Open GlebPlekhotko opened 2 years ago

GlebPlekhotko commented 2 years ago

Hello.

Here is the case I met. I have a file named "Device.h". And there is a function:

uint32_t deviceTransactionCoupler(Device *this, DeviceCommand command,
                                  void *sendBuffer, uint16_t bytesToSend,
                                  void *receiveBuffer, uint16_t receiveBufferSize);

If I feed this header file to the CMock then generated mock function checks only first byte of the void pointers sendBuffer and receiveBuffer. Here is a snippet:

{
    UNITY_SET_DETAILS(CMockString_deviceTransactionCoupler,CMockString_sendBuffer);
    if (cmock_call_instance->Expected_sendBuffer == NULL)
      { UNITY_TEST_ASSERT_NULL(sendBuffer, cmock_line, CMockStringExpNULL); }
    else
      { UNITY_TEST_ASSERT_EQUAL_HEX8_ARRAY(cmock_call_instance->Expected_sendBuffer, sendBuffer, 1, cmock_line, CMockStringMismatch); }
 }

  {
    UNITY_SET_DETAILS(CMockString_deviceTransactionCoupler,CMockString_receiveBuffer);
    if (cmock_call_instance->Expected_receiveBuffer == NULL)
      { UNITY_TEST_ASSERT_NULL(receiveBuffer, cmock_line, CMockStringExpNULL); }
    else
      { UNITY_TEST_ASSERT_EQUAL_HEX8_ARRAY(cmock_call_instance->Expected_receiveBuffer, receiveBuffer, 1, cmock_line, CMockStringMismatch); }
 }

Argument number 3 in the _UNITY_TEST_ASSERT_EQUAL_HEX8ARRAY specifies number of elements to check. Let's check what hides behind it:

#define UNITY_TEST_ASSERT_EQUAL_HEX8_ARRAY(expected, actual, num_elements, line, message)        UnityAssertEqualIntArray((UNITY_INTERNAL_PTR)(expected), (UNITY_INTERNAL_PTR)(actual), (UNITY_UINT32)(num_elements), (message), (UNITY_LINE_TYPE)(line), UNITY_DISPLAY_STYLE_HEX8,    UNITY_ARRAY_TO_ARRAY)

Finally, in the UnityAssertEqualIntArray original void pointers are cast to the pointer to a byte:


case 1:
                expect_val = *(UNITY_PTR_ATTRIBUTE const UNITY_INT8*)expected;
                actual_val = *(UNITY_PTR_ATTRIBUTE const UNITY_INT8*)actual;
                increment  = sizeof(UNITY_INT8);
                break;

It is easy to fix it by using "_treatas" option:

:treat_as:
    void*:    PTR

But it is still quite confusing.

Thank you for your attention!

mvandervoord commented 3 months ago

You bring up a good point. I'm not sure what the best default is for void* (they all kinda stink), but I suspect it might be a pointer comparison. At least when that fails, it fails in a way that users can understand more easily without digging into code, right?