ThrowTheSwitch / CMock

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

Mock function declaration swaps function argument array for pointer #422

Open ljden opened 1 year ago

ljden commented 1 year ago

I am trying to mock an esp-idf component with CMock and the build is failing due to a mismatch in function definition from a const uint8_t[6] in the original header to a const uint8_t[6] in the mock header. Have I missed something or does CMock not handle this case correctly?

[7/11] Building C object esp-idf/esp_wifi/CMakeFiles/__idf_esp_wifi.dir/mocks/Mockesp_wifi.c.oFAILED: esp-idf/esp_wifi/CMakeFiles/__idf_esp_wifi.dir/mocks/Mockesp_wifi.c.o 
/usr/bin/cc -DUNITY_INCLUDE_CONFIG_H -I/home/vbox/wifi_manager/host_test/build/config -I/home/vbox/esp/esp-idf/components/esp_wifi/include -I/home/vbox/esp/esp-idf/tools/mocks/esp_wifi/mocks/include -I/home/vbox/wifi_manager/host_test/build/esp-idf/esp_wifi/mocks -I/home/vbox/esp/esp-idf/tools/mocks/freertos/include -I/home/vbox/esp/esp-idf/components/freertos/FreeRTOS-Kernel/include -I/home/vbox/esp/esp-idf/components/freertos/esp_additions/include -I/home/vbox/esp/esp-idf/components/freertos/esp_additions/include/freertos -I/home/vbox/esp/esp-idf/components/freertos/FreeRTOS-Kernel/include/freertos -I/home/vbox/esp/esp-idf/components/freertos/FreeRTOS-Kernel/portable/linux/include -I/home/vbox/wifi_manager/host_test/build/esp-idf/freertos/mocks -I/home/vbox/esp/esp-idf/components/log/include -I/home/vbox/esp/esp-idf/components/esp_rom/include -I/home/vbox/esp/esp-idf/components/esp_rom/include/linux -I/home/vbox/esp/esp-idf/tools/mocks/soc/include -I/home/vbox/esp/esp-idf/components/esp_common/include -I/home/vbox/esp/esp-idf/components/cmock/CMock/src -I/home/vbox/esp/esp-idf/components/unity/include -I/home/vbox/esp/esp-idf/components/unity/unity/src -I/home/vbox/esp/esp-idf/components/esp_hw_support/include -I/home/vbox/wifi_manager/host_test/build/esp-idf/esp_hw_support/mocks -I/home/vbox/esp/esp-idf/components/esp_event/include -I/home/vbox/esp/esp-idf/components/linux/include -I/home/vbox/esp/esp-idf/components/esp_netif/include -ffunction-sections -fdata-sections -Wall -Werror=all -Wno-error=unused-function -Wno-error=unused-variable -Wno-error=deprecated-declarations -Wextra -Wno-unused-parameter -Wno-sign-compare -Wno-enum-conversion -gdwarf-4 -ggdb -Og -fmacro-prefix-map=/home/vbox/wifi_manager/host_test=. -fmacro-prefix-map=/home/vbox/esp/esp-idf=/IDF -fstrict-volatile-bitfields -Wno-error=unused-but-set-variable -fno-jump-tables -fno-tree-switch-conversion -I./mocks/include/ -std=gnu17 -Wno-old-style-declaration -D_GNU_SOURCE -DIDF_VER=\"v5.0-165-ge5675848e3\" -DconfigTICK_RATE_HZ=1000 -DESP_PLATFORM -MD -MT esp-idf/esp_wifi/CMakeFiles/__idf_esp_wifi.dir/mocks/Mockesp_wifi.c.o -MF esp-idf/esp_wifi/CMakeFiles/__idf_esp_wifi.dir/mocks/Mockesp_wifi.c.o.d -o esp-idf/esp_wifi/CMakeFiles/__idf_esp_wifi.dir/mocks/Mockesp_wifi.c.o -c /home/vbox/wifi_manager/host_test/build/esp-idf/esp_wifi/mocks/Mockesp_wifi.c
/home/vbox/wifi_manager/host_test/build/esp-idf/esp_wifi/mocks/Mockesp_wifi.c:5679:65: error: argument 2 of type ‘const uint8_t *’ {aka ‘const unsigned char *’} declared as a pointer [-Werror=array-parameter=]
 5679 | esp_err_t esp_wifi_set_mac(wifi_interface_t ifx, const uint8_t* mac)
      |                                                  ~~~~~~~~~~~~~~~^~~
In file included from /home/vbox/wifi_manager/host_test/build/esp-idf/esp_wifi/mocks/Mockesp_wifi.h:6,
                 from /home/vbox/wifi_manager/host_test/build/esp-idf/esp_wifi/mocks/Mockesp_wifi.c:6:
/home/vbox/esp/esp-idf/components/esp_wifi/include/esp_wifi.h:673:64: note: previously declared as an array ‘const uint8_t[6]’ {aka ‘const unsigned char[6]’}
  673 | esp_err_t esp_wifi_set_mac(wifi_interface_t ifx, const uint8_t mac[6]);
      |                                                  ~~~~~~~~~~~~~~^~~~~~
/home/vbox/wifi_manager/host_test/build/esp-idf/esp_wifi/mocks/Mockesp_wifi.c:5830:59: error: argument 2 of type ‘uint8_t *’ {aka ‘unsigned char *’} declared as a pointer [-Werror=array-parameter=]
 5830 | esp_err_t esp_wifi_get_mac(wifi_interface_t ifx, uint8_t* mac)
      |                                                  ~~~~~~~~~^~~
In file included from /home/vbox/wifi_manager/host_test/build/esp-idf/esp_wifi/mocks/Mockesp_wifi.h:6,
                 from /home/vbox/wifi_manager/host_test/build/esp-idf/esp_wifi/mocks/Mockesp_wifi.c:6:
/home/vbox/esp/esp-idf/components/esp_wifi/include/esp_wifi.h:687:58: note: previously declared as an array ‘uint8_t[6]’ {aka ‘unsigned char[6]’}
  687 | esp_err_t esp_wifi_get_mac(wifi_interface_t ifx, uint8_t mac[6]);
      |                                                  ~~~~~~~~^~~~~~
/home/vbox/wifi_manager/host_test/build/esp-idf/esp_wifi/mocks/Mockesp_wifi.c:7448:50: error: argument 1 of type ‘const uint8_t *’ {aka ‘const unsigned char *’} declared as a pointer [-Werror=array-parameter=]
 7448 | esp_err_t esp_wifi_ap_get_sta_aid(const uint8_t* mac, uint16_t* aid)
      |                                   ~~~~~~~~~~~~~~~^~~
In file included from /home/vbox/wifi_manager/host_test/build/esp-idf/esp_wifi/mocks/Mockesp_wifi.h:6,
                 from /home/vbox/wifi_manager/host_test/build/esp-idf/esp_wifi/mocks/Mockesp_wifi.c:6:
/home/vbox/esp/esp-idf/components/esp_wifi/include/esp_wifi.h:855:49: note: previously declared as an array ‘const uint8_t[6]’ {aka ‘const unsigned char[6]’}
  855 | esp_err_t esp_wifi_ap_get_sta_aid(const uint8_t mac[6], uint16_t *aid);
      |                                   ~~~~~~~~~~~~~~^~~~~~
cc1: some warnings being treated as errors
vbox@vbox:~/esp/esp-idf/tools/mocks/esp_wifi$ tree
.
├── CMakeLists.txt
├── mock
│   └── mock_config.yaml
└── mocks
    └── include
        └── machine
            └── endian.h

4 directories, 3 files
vbox@vbox:~/esp/esp-idf/tools/mocks/esp_wifi$ cat mock/mock_config.yaml 
        :cmock:
          :plugins:
            - expect
            - expect_any_args
            - return_thru_ptr
            - array
            - ignore
            - ignore_arg
            - callback
vbox@vbox:~/esp/esp-idf/tools/mocks/esp_wifi$ cat CMakeLists.txt 
# NOTE: This kind of mocking currently works on Linux targets only.
#       On Espressif chips, too many dependencies are missing at the moment.
message(STATUS "building ESP WIFI MOCKS")

idf_component_get_property(original_esp_wifi_dir esp_wifi COMPONENT_OVERRIDEN_DIR)

idf_component_mock(INCLUDE_DIRS "${original_esp_wifi_dir}/include" "./mocks/include"
                   REQUIRES esp_hw_support esp_event esp_netif
                   MOCK_HEADER_FILES ${original_esp_wifi_dir}/include/esp_wifi.h
    )

I'm not 100% sure if this is an issue on the CMock or esp-idf side. If you think it is not CMock, let me know and I'll open a ticket on esp-idf

drdott commented 10 months ago

I've just been hit with this as well, and it's definately something that cmock does. A simple example to replicate is attached and written below:

foo.zip contains foo.h:

#ifndef FOO_H
#define FOO_H
int foo(int bar[4]);
#endif

On a command line run:

ruby <path to>\cmock.rb foo.h

And you will get this generated in "Mockfoo.c":

int foo(int* bar)
{
  [...]
}

Which then causes a compiler warning with gcc.

It seems to be done in CMock/lib/cmock_header_parser.rb, line 479-480, but I assume there would be many knock on effects if you changed it away from converting arrays to pointers. I haven't tried, I don't know enough ruby.:

      # magically turn brackets into asterisks, also match for parentheses that come from macros
      arg_list.gsub!(/(\w+)(?:\s*\[[^\[\]]*\])+/, '*\1')

This already seems to be reported in #69, but that specifically mentions multidemensional arrays but I assume it happens for any arrays.

mvandervoord commented 3 months ago

Hi. I'm going to admit something here. We took a shortcut WAaaaaaaaaay back when we wrote CMock. We said "internally, arrays and pointers are the same thing... and internally multidimensional arrays are just single dimension arrays accessed differently." It was a hack and it was meant to get to version 1 and then get fixed.

Then it didn't get fixed.

For a long time.

And I feel embarrassed and I deeply apologize for how long we've let that slide. This is an issue that should have been fixed a long long time ago.

It's not a hard problem to solve. It IS a nuanced problem. That means that I know that when I start working on it, it's going to require a time investment to do it right. And it's worth doing it right. So far that's kept me from tackling it in my slivers of free time.

I intend to change that pattern soon. Hopefully I'm not adding words here that I will further regret. ;)

0xjakob commented 2 months ago

Note that in this particular case, you can try -Wno-array-parameter in GCC to suppress the warning (and error with -Werror) to work around that limitation, which we did in IDF to enable mocking of that component. However, it does not seem to work with re-declarations of two-dimensional arrays, e.g. esp_err_t esp_efuse_write_keys(const esp_efuse_purpose_t* purposes, uint8_t* keys, unsigned number_of_keys) (generated) vs esp_err_t esp_efuse_write_keys(const esp_efuse_purpose_t purposes[], uint8_t keys[][32], unsigned number_of_keys); (original).