CODEWITHSUNDEEP
c++gccc++17compiler-warningsstrncpy
ChrisAB
ChrisAB

Reputation: 11

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

Upvotes: 0

Views: 201

Answers (2)

Jan Schultke
Jan Schultke

Reputation: 39444

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

Upvotes: 1

Abstraction
Abstraction

Reputation: 1572

strncpy(dest, src, count) copies characters from src until it encounters '\0' or copies count symbols, whichever comes first. The warning reflects that allocating count bytes and passing the same count to strncpy() is generally dangerous: you may end up with a non-null-terminated string. You won't in this specific case since src[count-1] is guaranteed to be '\0' but seemingly warning system misses that.

Notice that a "warning" isn't an "error" and may be issued for a perfectly correct code.

Upvotes: 0

Related Questions