Search code examples
c++gccc++17compiler-warningsstrncpy

-Wstringop-overflow warning when length allocated to destination string is equal to source


I am using GCC 10.2.0 with C++17, and I get the following error:

ioapi.c: In function ‘file_build_ioposix’:
ioapi.c:125:5: warning: ‘strncpy’ specified bound depends on the length of the source argument [-Wstringop-overflow=]
  125 |     strncpy(ioposix->filename, filename, ioposix->filenameLength);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
ioapi.c:123:31: note: length computed here
  123 |     ioposix->filenameLength = strlen(filename) + 1;

And this is my code:

static voidpf file_build_ioposix(FILE *file, const char *filename)
{
    FILE_IOPOSIX *ioposix = NULL;
    if (file == NULL)
        return NULL;
    ioposix = (FILE_IOPOSIX*)malloc(sizeof(FILE_IOPOSIX));
    ioposix->file = file;
    ioposix->filenameLength = strlen(filename) + 1;
    ioposix->filename = (char*)malloc(ioposix->filenameLength * sizeof(char));
    strncpy(ioposix->filename, filename, ioposix->filenameLength);
    return (voidpf)ioposix;
}

Problem here is this should be considered okay, since destination is set to source length + 1, but it still provides an error.

Tried checking for failed malloc, but doesn't change anything.


Solution

  • The warning is kind of a false positive. The warning tries to catch cases such as strncpy(dest, src, strlen(src) + 1) where the destination buffer is written to, past its end.

    ioposix->filenameLength = strlen(filename) + 1;
    ioposix->filename = (char*)malloc(ioposix->filenameLength * sizeof(char));
    strncpy(ioposix->filename, filename, ioposix->filenameLength);
    

    In this particular case, the size of the destination buffer is equal to the size of the source buffer, since the destination buffer was allocated with malloc to have exactly the right size. strncpy is safe as used, and the warning is a false positive.

    However, strncpy is also pointless because this could have been written as:

    ioposix->filenameLength = strlen(filename) + 1;
    ioposix->filename = (char*)malloc(ioposix->filenameLength); // sizeof(char) == 1
    memcpy(ioposix->filename, filename, ioposix->filenameLength);
    

    Or using memdup (from POSIX, or written yourself for convenience)

    ioposix->filenameLength = strlen(filename) + 1;
    ioposix->filename = (char*)memdup(filename, ioposix->filenameLength);