Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mock function declaration swaps function argument array for pointer #422

Open
ljden opened this issue Jan 23, 2023 · 3 comments
Open

Mock function declaration swaps function argument array for pointer #422

ljden opened this issue Jan 23, 2023 · 3 comments

Comments

@ljden
Copy link

ljden commented Jan 23, 2023

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
Copy link

drdott commented Aug 25, 2023

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
Copy link
Member

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
Copy link

0xjakob commented Apr 25, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants