ubxlib icon indicating copy to clipboard operation
ubxlib copied to clipboard

strncpy faills to compile in ESP-IDF

Open jcradavelli opened this issue 1 year ago • 4 comments

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
  1. Get all submodules recursively
git submodule update --init --recursive
  1. Open the project at: 'port/platform/esp-idf/mcu/esp32/runner'

  2. 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")
  1. 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.

jcradavelli avatar May 22 '24 20:05 jcradavelli

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.

mazgch avatar May 22 '24 21:05 mazgch

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 avatar May 23 '24 11:05 RobMeades

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

RobMeades avatar May 23 '24 12:05 RobMeades

Hopefully fixed in b645c901a44ecc2aec125aaacdd5e9a0511e27f9.

RobMeades avatar May 23 '24 15:05 RobMeades

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

RobMeades avatar May 28 '24 10:05 RobMeades