u-blox / ubxlib

Portable C libraries which provide APIs to build applications with u-blox products and services. Delivered as add-on to existing microcontroller and RTOS SDKs.
Apache License 2.0
313 stars 98 forks source link

strncpy faills to compile in ESP-IDF #239

Closed jcradavelli closed 6 months ago

jcradavelli commented 6 months ago

Hello, this is my first contribution here and I hope I'm doing it properly.

Description

The esp32 (ESP-IDF) build for the "ubxlib_runner" fails with the following error messages

/ubxlib/gnss/test/u_gnss_private_test.c:598:13: error: 'strncpy' specified bound 9 equals destination size [-Werror=stringop-truncation]      
  598 |             strncpy(talkerSentenceBuffer, gNmeaTestMessage[x].pTalkerSentenceStr, sizeof(talkerSentenceBuffer));
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/ubxlib/common/sock/test/u_sock_test.c:845:13: error: 'strncpy' specified bound 64 equals destination size [-Werror=stringop-truncation]      
  845 |             strncpy(buffer, gTestAddressList[x].pAddressString,
      |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  846 |                     sizeof(buffer));
      |                     ~~~~~~~~~~~~~~~
/ubxlib/common/sock/test/u_sock_test.c:890:9: error: 'strncpy' specified bound 64 equals destination size [-Werror=stringop-truncation]       
  890 |         strncpy(buffer, gTestAddressPortRemoval[x].pAddressStringOriginal,
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  891 |                 sizeof(buffer));
      |                 ~~~~~~~~~~~~~~~

Steps to reproduce the failure

  1. Get the repository (5e32f48)

    git clone https://github.com/u-blox/ubxlib
  2. Get all submodules recursively

    git submodule update --init --recursive
  3. Open the project at: 'port/platform/esp-idf/mcu/esp32/runner'

  4. As I am using vscode and the esp-idf extension, so I changed line 31 of the CMakeLists.txt file to look like this (probably there is a bether way to do this)

    set(TEST_COMPONENTS "ubxlib_runner" CACHE STRING "Component to test")
  5. I compiled using the extension commands (cilinder like icon in the footbar)

Workaround

Apparently the strncpy function may not be able to add the '\0' terminator if the size of the input string is equal to the size of the output buffer, resulting in a string not terminated by '\0'. To work around this error, I subtracted 1 from the value entered in the third argument of the strncpy function in the following locations

/ubxlib/gnss/test/u_gnss_private_test.c:598
            strncpy(talkerSentenceBuffer, gNmeaTestMessage[x].pTalkerSentenceStr, sizeof(talkerSentenceBuffer)-1);
/ubxlib/common/sock/test/u_sock_test.c:845:

            strncpy(buffer, gTestAddressList[x].pAddressString,
                    sizeof(buffer)-1);
/ubxlib/common/sock/test/u_sock_test.c:890
        strncpy(buffer, gTestAddressPortRemoval[x].pAddressStringOriginal,
                sizeof(buffer)-1);

With these modifications the compilation ended successfully. Despite this, I haven't been able to carry out any further tests yet, as I'm waiting for the delivery of the devboard I purchased this week.

Hope this helps, greetings to the whole team.

mazgch commented 6 months ago

You may need to add a buffer[sizeof(buffer)-1] = '\0' after as strncpy will not write beyond the size and add the zero termination if it does exceed.

That last char may have not been set before calling strncpy.

RobMeades commented 6 months ago

Hi, and thanks for posting. This is another one of those extremely irritating and quite pointless errors/warnings that GCC 8 introduced (the other one I'm aware of is with snprintf()): as you will see from the code, the buffer sizes already include room for a null terminator, there is no actual problem here, making this an error simply causes us to add more code (the buffer[sizeof(buffer)-1] = '\0' that Michael mentions) for no reason.

Let me see what is the simplest way to shut it up.

RobMeades commented 6 months ago

Actually, could you just try replacing strncpy with memcpy in these three locations? That should fix it.

RobMeades commented 6 months ago

Hopefully fixed in b645c901a44ecc2aec125aaacdd5e9a0511e27f9.

RobMeades commented 6 months ago

I will close this one now: please feel free to re-open, or open a new issue, if there is more to discuss.